From 902fafb8e7216d0f5064667452b49dbc0d267e99 Mon Sep 17 00:00:00 2001 From: "Fuente, Pablo Andres" Date: Sun, 9 Jul 2017 01:34:50 -0300 Subject: [PATCH] Fixing tests for Windows Fixing the config/config_test, the discovery/file/file_test and the promql/promql_test tests for Windows. For most of the tests, the fix involved correct handling of path separators. In the case of the promql tests, the issue was related to the removal of the temporal directories used by the storage. The issue is that the RemoveAll() call returns an error when it tries to remove a directory which is not empty, which seems to be true due to some kind of process that is still running after closing the storage. To fix it I added some retries to the remove of the temporal directories. Adding tags file from Universal Ctags to .gitignore --- .gitignore | 1 + config/config_non_windows_test.go | 27 +++++++++++ config/config_test.go | 45 ++++++++++++++----- config/config_windows_test.go | 25 +++++++++++ config/testdata/conf.good.yml | 1 - config/testdata/rules_abs_path.good.yml | 4 ++ .../testdata/rules_abs_path_windows.good.yml | 4 ++ discovery/file/file_test.go | 5 ++- util/testutil/directory.go | 14 ++++-- 9 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 config/config_non_windows_test.go create mode 100644 config/config_windows_test.go create mode 100644 config/testdata/rules_abs_path.good.yml create mode 100644 config/testdata/rules_abs_path_windows.good.yml diff --git a/.gitignore b/.gitignore index 2fe870e942..4d417605a1 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ .*.swo *.iml .idea +tags /prometheus /promtool diff --git a/config/config_non_windows_test.go b/config/config_non_windows_test.go new file mode 100644 index 0000000000..8e0be72439 --- /dev/null +++ b/config/config_non_windows_test.go @@ -0,0 +1,27 @@ +// Copyright 2015 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: "", +} diff --git a/config/config_test.go b/config/config_test.go index 0282deb0e5..8071d72588 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -17,6 +17,7 @@ import ( "encoding/json" "io/ioutil" "net/url" + "path/filepath" "reflect" "regexp" "strings" @@ -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{ @@ -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{ @@ -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, }, }, @@ -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", @@ -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"), }, }, }, @@ -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 diff --git a/config/config_windows_test.go b/config/config_windows_test.go new file mode 100644 index 0000000000..cb8dcebfb0 --- /dev/null +++ b/config/config_windows_test.go @@ -0,0 +1,25 @@ +// Copyright 2015 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: "", +} diff --git a/config/testdata/conf.good.yml b/config/testdata/conf.good.yml index 69c9cdc880..34da77d4ed 100644 --- a/config/testdata/conf.good.yml +++ b/config/testdata/conf.good.yml @@ -10,7 +10,6 @@ global: rule_files: - "first.rules" -- "/absolute/second.rules" - "my/*.rules" remote_write: diff --git a/config/testdata/rules_abs_path.good.yml b/config/testdata/rules_abs_path.good.yml new file mode 100644 index 0000000000..e9d3cf7f96 --- /dev/null +++ b/config/testdata/rules_abs_path.good.yml @@ -0,0 +1,4 @@ +rule_files: + - 'first.rules' + - 'rules/second.rules' + - '/absolute/third.rules' diff --git a/config/testdata/rules_abs_path_windows.good.yml b/config/testdata/rules_abs_path_windows.good.yml new file mode 100644 index 0000000000..667411ff56 --- /dev/null +++ b/config/testdata/rules_abs_path_windows.good.yml @@ -0,0 +1,4 @@ +rule_files: + - 'first.rules' + - 'rules\second.rules' + - 'c:\absolute\third.rules' diff --git a/discovery/file/file_test.go b/discovery/file/file_test.go index f977e3b228..65c53c23b8 100644 --- a/discovery/file/file_test.go +++ b/discovery/file/file_test.go @@ -17,6 +17,7 @@ import ( "fmt" "io" "os" + "path/filepath" "testing" "time" @@ -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 diff --git a/util/testutil/directory.go b/util/testutil/directory.go index 1672c5b015..d3c9c926f1 100644 --- a/util/testutil/directory.go +++ b/util/testutil/directory.go @@ -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 ( @@ -84,15 +87,20 @@ func NewCallbackCloser(fn func()) Closer { } func (t temporaryDirectory) Close() { + retries := temporaryDirectoryRemoveRetries 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 {