8000 Fixing tests for Windows by pafuent · Pull Request #2927 · prometheus/prometheus · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fixing tests for Windows #2927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
.*.swo
*.iml
.idea
tags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for me 😉, I use Exuberant CTags during development to navigate the code. I can remove this if you like.


/prometheus
/promtool
Expand Down
28 changes: 28 additions & 0 deletions config/config_default_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2017 The Prometheus Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// +build !windows

package config

const ruleFilesConfigFile = "testdata/rules_abs_path.good.yml"

var ruleFilesExpectedConf = &Config{
GlobalConfig: DefaultGlobalConfig,
RuleFiles: []string{
"testdata/first.rules",
"testdata/rules/second.rules",
"/absolute/third.rules",
},
original: "",
}
45 changes: 34 additions & 11 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"encoding/json"
"io/ioutil"
"net/url"
"path/filepath"
"reflect"
"regexp"
"strings"
Expand Down Expand Up @@ -48,9 +49,8 @@ var expectedConf = &Config{
},

RuleFiles: []string{
"testdata/first.rules",
"/absolute/second.rules",
"testdata/my/*.rules",
filepath.FromSlash("testdata/first.rules"),
filepath.FromSlash("testdata/my/*.rules"),
},

RemoteWriteConfigs: []*RemoteWriteConfig{
Expand Down Expand Up @@ -85,7 +85,7 @@ var expectedConf = &Config{
Scheme: DefaultScrapeConfig.Scheme,

HTTPClientConfig: HTTPClientConfig{
BearerTokenFile: "testdata/valid_token_file",
BearerTokenFile: filepath.FromSlash("testdata/valid_token_file"),
},

ServiceDiscoveryConfig: ServiceDiscoveryConfig{
Expand Down Expand Up @@ -252,9 +252,9 @@ var expectedConf = &Config{
TagSeparator: DefaultConsulSDConfig.TagSeparator,
Scheme: "https",
TLSConfig: TLSConfig{
CertFile: "testdata/valid_cert_file",
KeyFile: "testdata/valid_key_file",
CAFile: "testdata/valid_ca_file",
CertFile: filepath.FromSlash("testdata/valid_cert_file"),
KeyFile: filepath.FromSlash("testdata/valid_key_file"),
CAFile: filepath.FromSlash("testdata/valid_ca_file"),
InsecureSkipVerify: false,
},
},
Expand Down Expand Up @@ -283,8 +283,8 @@ var expectedConf = &Config{

HTTPClientConfig: HTTPClientConfig{
TLSConfig: TLSConfig{
CertFile: "testdata/valid_cert_file",
KeyFile: "testdata/valid_key_file",
CertFile: filepath.FromSlash("testdata/valid_cert_file"),
KeyFile: filepath.FromSlash("testdata/valid_key_file"),
},

BearerToken: "mysecret",
Expand Down Expand Up @@ -354,8 +354,8 @@ var expectedConf = &Config{
Timeout: model.Duration(30 * time.Second),
RefreshInterval: model.Duration(30 * time.Second),
TLSConfig: TLSConfig{
CertFile: "testdata/valid_cert_file",
KeyFile: "testdata/valid_key_file",
CertFile: filepath.FromSlash("testdata/valid_cert_file"),
KeyFile: filepath.FromSlash("testdata/valid_key_file"),
},
},
},
Expand Down Expand Up @@ -549,6 +549,29 @@ func TestLoadConfig(t *testing.T) {

}

func TestLoadConfigRuleFilesAbsolutePath(t *testing.T) {
// Parse a valid file that sets a rule files with an absolute path
c, err := LoadFile(ruleFilesConfigFile)
if err != nil {
t.Errorf("Error parsing %s: %s", ruleFilesConfigFile, err)
}

bgot, err := yaml.Marshal(c)
if err != nil {
t.Fatalf("%s", err)
}

bexp, err := yaml.Marshal(ruleFilesExpectedConf)
if err != nil {
t.Fatalf("%s", err)
}
ruleFilesExpectedConf.original = c.original

if !reflect.DeepEqual(c, ruleFilesExpectedConf) {
t.Fatalf("%s: unexpected config result: \n\n%s\n expected\n\n%s", ruleFilesConfigFile, bgot, bexp)
}
}

var expectedErrors = []struct {
filename string
errMsg string
Expand Down
26 changes: 26 additions & 0 deletions config/config_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2017 The Prometheus Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package config

const ruleFilesConfigFile = "testdata/rules_abs_path_windows.good.yml"

var ruleFilesExpectedConf = &Config{
GlobalConfig: DefaultGlobalConfig,
RuleFiles: []string{
"testdata\\first.rules",
"testdata\\rules\\second.rules",
"c:\\absolute\\third.rules",
},
original: "",
}
1 change: 0 additions & 1 deletion config/testdata/conf.good.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ global:

rule_files:
- "first.rules"
- "/absolute/second.rules"
- "my/*.rules"

remote_write:
Expand Down
4 changes: 4 additions & 0 deletions config/testdata/rules_abs_path.good.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rule_files:
- 'first.rules'
- 'rules/second.rules'
- '/absolute/third.rules'
4 changes: 4 additions & 0 deletions config/testdata/rules_abs_path_windows.good.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rule_files:
- 'first.rules'
- 'rules\second.rules'
- 'c:\absolute\third.rules'
5 changes: 3 additions & 2 deletions discovery/file/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -89,12 +90,12 @@ retry:
if _, ok := tg.Labels["foo"]; !ok {
t.Fatalf("Label not parsed")
}
if tg.String() != fmt.Sprintf("fixtures/_test%s:0", ext) {
if tg.String() != filepath.FromSlash(fmt.Sprintf("fixtures/_test%s:0", ext)) {
t.Fatalf("Unexpected target group %s", tg)
}

tg = tgs[1]
if tg.String() != fmt.Sprintf("fixtures/_test%s:1", ext) {
if tg.String() != filepath.FromSlash(fmt.Sprintf("fixtures/_test%s:1", ext)) {
t.Fatalf("Unexpected target groups %s", tg)
}
break retry
Expand Down
14 changes: 11 additions & 3 deletions util/testutil/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const (

// NilCloser is a no-op Closer.
NilCloser = nilCloser(true)

// The number of times that a TemporaryDirectory will retry its removal
temporaryDirectoryRemoveRetries = 2
)

type (
Expand Down Expand Up @@ -84,15 +87,20 @@ func NewCallbackCloser(fn func()) Closer {
}

func (t temporaryDirectory) Close() {
retries := temporaryDirectoryRemoveRetries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the retrying actually help in practice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it helps. In a Windows machine, the promql tests always fail due to RemoveAll() can't remove a directory because is not empty. The strange thing is that if you go to the offending directory after the tests finished, the directory is actually empty.
My first approach to fix this, was to check if the Storage was properly close before trying to delete the storage directory, but I couldn't find any issue in that (all the proper channels seems to be wait before returning the control to the calling function). Then I checked, if was LevelDB, but I didn't have any luck founding the issue. So, maybe this is a bug of RemoveAll() in Windows, maybe Prometheus is giving back the control before all the things are properly close or maybe LevelDB is the one that is giving back the control before it should.

err := os.RemoveAll(t.path)
if err != nil {
for err != nil && retries > 0 {
switch {
case os.IsNotExist(err):
return
err = nil
default:
t.tester.Fatal(err)
retries--
err = os.RemoveAll(t.path)
}
}
if err != nil {
t.tester.Fatal(err)
}
}

func (t temporaryDirectory) Path() string {
Expand Down
0