From cdb6bce1515e50ba4d7e33e6e6ab3c65afcc7b11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 16:54:58 +0100 Subject: [PATCH 1/9] Calling flag.Parse() twice confuses some flags. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flag.Parse() was called twice previously, first to get -config.file value to parse the config file, and second to parse remaining command line parameters – overwriting values from config file. Unfortunately that confuses command line parameters that accept multiple values. Here we get -config.file via a separate FlagSet to avoid calling flag.Parse() (on default FlagSet) twice. Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 41 ++++++++++++++++++++++++------------- pkg/util/flagext/ignored.go | 22 ++++++++++++++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) create mode 100644 pkg/util/flagext/ignored.go diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index b4a5ae28cb1..523980cc9a1 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -9,6 +9,7 @@ import ( "runtime" "time" + "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/go-kit/kit/log/level" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -18,22 +19,25 @@ import ( "github.com/cortexproject/cortex/pkg/cortex" "github.com/cortexproject/cortex/pkg/util" - "github.com/cortexproject/cortex/pkg/util/flagext" ) func init() { prometheus.MustRegister(version.NewCollector("cortex")) } +const configFileOption = "config.file" + func main() { var ( - cfg cortex.Config - configFile = "" eventSampleRate int ballastBytes int mutexProfileFraction int ) - flag.StringVar(&configFile, "config.file", "", "Configuration file to load.") + + cfg := parseAndLoadConfigFile() + + // Ignore -config.file here, since it was already parsed, but it's still present on command line. + flagext.IgnoredFlag(flag.CommandLine, configFileOption, "Configuration file to load.") flag.IntVar(&eventSampleRate, "event.sample-rate", 0, "How often to sample observability events (0 = never).") flag.IntVar(&ballastBytes, "mem-ballast-size-bytes", 0, "Size of memory ballast to allocate.") flag.IntVar(&mutexProfileFraction, "debug.mutex-profile-fraction", 0, "Fraction at which mutex profile vents will be reported, 0 to disable") @@ -45,16 +49,6 @@ func main() { flagext.RegisterFlags(&cfg) flag.Parse() - if configFile != "" { - if err := LoadConfig(configFile, &cfg); err != nil { - fmt.Printf("error loading config from %s: %v\n", configFile, err) - os.Exit(1) - } - } - - // Parse a second time, as command line flags should take precedent over the config file. - flag.Parse() - // Validate the config once both the config file has been loaded // and CLI flags parsed. err := cfg.Validate() @@ -90,6 +84,25 @@ func main() { util.CheckFatal("initializing cortex", err) } +// Parse -config.file option via separate flag set, to avoid polluting default one and calling flag.Parse on it twice. +func parseAndLoadConfigFile() cortex.Config { + var configFile = "" + fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError) // ignore unknown flags here. + fs.SetOutput(ioutil.Discard) // eat all error messages for unknown flags, and default Usage output + fs.StringVar(&configFile, configFileOption, "", "") // usage not used in this function. + _ = fs.Parse(os.Args[1:]) + + var cfg cortex.Config + if configFile != "" { + if err := LoadConfig(configFile, &cfg); err != nil { + fmt.Printf("error loading config from %s: %v\n", configFile, err) + os.Exit(1) + } + } + + return cfg +} + // LoadConfig read YAML-formatted config from filename into cfg. func LoadConfig(filename string, cfg *cortex.Config) error { buf, err := ioutil.ReadFile(filename) diff --git a/pkg/util/flagext/ignored.go b/pkg/util/flagext/ignored.go new file mode 100644 index 00000000000..2dd49a87ebd --- /dev/null +++ b/pkg/util/flagext/ignored.go @@ -0,0 +1,22 @@ +package flagext + +import ( + "flag" +) + +type ignoredFlag struct { + name string +} + +func (ignoredFlag) String() string { + return "ignored" +} + +func (d ignoredFlag) Set(string) error { + return nil +} + +// IgnoredFlag ignores set value, without any warning +func IgnoredFlag(f *flag.FlagSet, name, message string) { + f.Var(ignoredFlag{name}, name, message) +} From b6c7b5973ae21a78d44da40da8c661c099f2bbeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 17:15:04 +0100 Subject: [PATCH 2/9] Reverted unintended import order change. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 523980cc9a1..f9de9d1becb 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -9,7 +9,6 @@ import ( "runtime" "time" - "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/go-kit/kit/log/level" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -19,6 +18,7 @@ import ( "github.com/cortexproject/cortex/pkg/cortex" "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/flagext" ) func init() { From cb8a8df3686a3ef23dd081a57064b43aa61b67a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 20:37:12 +0100 Subject: [PATCH 3/9] Call RegisterFlags(&cfg) to set defaults before parsing config file. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index f9de9d1becb..624ab559712 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -29,12 +29,27 @@ const configFileOption = "config.file" func main() { var ( + cfg cortex.Config eventSampleRate int ballastBytes int mutexProfileFraction int ) - cfg := parseAndLoadConfigFile() + configFile := parseConfigFileParameter() + + // This sets default values from flags to the config. + // It needs to be called before parsing the config file! + flagext.RegisterFlags(&cfg) + + if configFile != "" { + if err := LoadConfig(configFile, &cfg); err != nil { + fmt.Printf("error loading config from %s: %v\n", configFile, err) + if exitOnError { + os.Exit(1) + } + return + } + } // Ignore -config.file here, since it was already parsed, but it's still present on command line. flagext.IgnoredFlag(flag.CommandLine, configFileOption, "Configuration file to load.") @@ -46,7 +61,6 @@ func main() { runtime.SetMutexProfileFraction(mutexProfileFraction) } - flagext.RegisterFlags(&cfg) flag.Parse() // Validate the config once both the config file has been loaded @@ -54,7 +68,10 @@ func main() { err := cfg.Validate() if err != nil { fmt.Printf("error validating config: %v\n", err) - os.Exit(1) + if exitOnError { + os.Exit(1) + } + return } // Allocate a block of memory to alter GC behaviour. See https://github.com/golang/go/issues/23044 @@ -85,22 +102,14 @@ func main() { } // Parse -config.file option via separate flag set, to avoid polluting default one and calling flag.Parse on it twice. -func parseAndLoadConfigFile() cortex.Config { +func parseConfigFileParameter() string { var configFile = "" fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError) // ignore unknown flags here. fs.SetOutput(ioutil.Discard) // eat all error messages for unknown flags, and default Usage output fs.StringVar(&configFile, configFileOption, "", "") // usage not used in this function. _ = fs.Parse(os.Args[1:]) - var cfg cortex.Config - if configFile != "" { - if err := LoadConfig(configFile, &cfg); err != nil { - fmt.Printf("error loading config from %s: %v\n", configFile, err) - os.Exit(1) - } - } - - return cfg + return configFile } // LoadConfig read YAML-formatted config from filename into cfg. From 44cfb20a291c80397431f09263d5a7ddbe53c324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 20:38:22 +0100 Subject: [PATCH 4/9] Added some unit tests for parsing config. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 16 ++++- cmd/cortex/main_test.go | 136 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 cmd/cortex/main_test.go diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 624ab559712..aff633a6a51 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -27,6 +27,8 @@ func init() { const configFileOption = "config.file" +var exitOnError = true + func main() { var ( cfg cortex.Config @@ -61,7 +63,19 @@ func main() { runtime.SetMutexProfileFraction(mutexProfileFraction) } - flag.Parse() + // parse flags. We don't use flag.Parse() because it either exits on errors, or ignores them. + // We want to either exit or return on error (for tests). + if !exitOnError { + flag.CommandLine.Init(flag.CommandLine.Name(), flag.ContinueOnError) + } + if err := flag.CommandLine.Parse(os.Args[1:]); err != nil { + if exitOnError { + os.Exit(2) + } + + fmt.Fprintln(os.Stderr, err) + return + } // Validate the config once both the config file has been loaded // and CLI flags parsed. diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go new file mode 100644 index 00000000000..659916257c8 --- /dev/null +++ b/cmd/cortex/main_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "bytes" + "flag" + "io" + "io/ioutil" + "os" + "sync" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestName(t *testing.T) { + for name, tc := range map[string]struct { + arguments []string + yaml string + message string + }{ + "-h": { + arguments: []string{"-h"}, + message: configFileOption, + }, + + // check that config file is used + "-config.file": { + yaml: "target: unknown", + message: "unrecognised module name: unknown", + }, + + "unknown flag": { + arguments: []string{"-unknown.flag"}, + message: "unknown.flag", + }, + + "-config.file + argument override": { + yaml: "target: ingester", + arguments: []string{"-target=unknown"}, + message: "unrecognised module name: unknown", + }, + + // we cannot test the happy path, as cortex would then fully start + } { + t.Run(name, func(t *testing.T) { + testSingle(t, tc.arguments, tc.yaml, []byte(tc.message)) + }) + } +} + +func testSingle(t *testing.T, arguments []string, yaml string, message []byte) { + oldArgs, oldStdout, oldStderr := os.Args, os.Stdout, os.Stderr + defer func() { + os.Stdout = oldStdout + os.Stderr = oldStderr + os.Args = oldArgs + }() + + if yaml != "" { + tempFile, err := ioutil.TempFile("", "test") + require.NoError(t, err) + + defer func() { + require.NoError(t, tempFile.Close()) + require.NoError(t, os.Remove(tempFile.Name())) + }() + + _, err = tempFile.WriteString(yaml) + require.NoError(t, err) + + arguments = append([]string{"-" + configFileOption, tempFile.Name()}, arguments...) + } + + arguments = append([]string{"./cortex"}, arguments...) + + exitOnError = false + os.Args = arguments + co := captureOutput(t) + + // reset default flags + flag.CommandLine = flag.NewFlagSet(arguments[0], flag.ExitOnError) + + main() + + stdout, stderr := co.Done() + require.True(t, bytes.Contains(stdout, message) || bytes.Contains(stderr, message)) +} + +type capturedOutput struct { + stdoutBuf bytes.Buffer + stderrBuf bytes.Buffer + + wg sync.WaitGroup + stdoutReader, stdoutWriter *os.File + stderrReader, stderrWriter *os.File +} + +func captureOutput(t *testing.T) *capturedOutput { + stdoutR, stdoutW, err := os.Pipe() + require.NoError(t, err) + os.Stdout = stdoutW + + stderrR, stderrW, err := os.Pipe() + require.NoError(t, err) + os.Stderr = stderrW + + co := &capturedOutput{ + stdoutReader: stdoutR, + stdoutWriter: stdoutW, + stderrReader: stderrR, + stderrWriter: stderrW, + } + co.wg.Add(1) + go func() { + defer co.wg.Done() + _, _ = io.Copy(&co.stdoutBuf, stdoutR) + }() + + co.wg.Add(1) + go func() { + defer co.wg.Done() + _, _ = io.Copy(&co.stderrBuf, stderrR) + }() + + return co +} + +func (co *capturedOutput) Done() (stdout []byte, stderr []byte) { + // we need to close writers for readers to stop + _ = co.stdoutWriter.Close() + _ = co.stderrWriter.Close() + + co.wg.Wait() + + return co.stdoutBuf.Bytes(), co.stderrBuf.Bytes() +} From 9535e401e32b500c076d1527084fad46e6bcd13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 21:04:00 +0100 Subject: [PATCH 5/9] Test mode in main simply dumps the config YAML if no error occurs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added tests for happy path. Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 45 ++++++++++++++++--------------- cmd/cortex/main_test.go | 25 +++++++++++++---- pkg/cortex/modules.go | 4 +++ pkg/ring/kv/multi.go | 2 +- pkg/util/runtimeconfig/manager.go | 2 +- 5 files changed, 50 insertions(+), 28 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index aff633a6a51..5d2da65bc22 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -27,7 +27,7 @@ func init() { const configFileOption = "config.file" -var exitOnError = true +var testMode = false func main() { var ( @@ -46,10 +46,10 @@ func main() { if configFile != "" { if err := LoadConfig(configFile, &cfg); err != nil { fmt.Printf("error loading config from %s: %v\n", configFile, err) - if exitOnError { - os.Exit(1) + if testMode { + return } - return + os.Exit(1) } } @@ -63,29 +63,22 @@ func main() { runtime.SetMutexProfileFraction(mutexProfileFraction) } - // parse flags. We don't use flag.Parse() because it either exits on errors, or ignores them. - // We want to either exit or return on error (for tests). - if !exitOnError { + if testMode { + // Don't exit on error in test mode. Just parse parameters, dump config and stop. flag.CommandLine.Init(flag.CommandLine.Name(), flag.ContinueOnError) - } - if err := flag.CommandLine.Parse(os.Args[1:]); err != nil { - if exitOnError { - os.Exit(2) - } - - fmt.Fprintln(os.Stderr, err) + flag.Parse() + DumpYaml(&cfg) return } + flag.Parse() + // Validate the config once both the config file has been loaded // and CLI flags parsed. err := cfg.Validate() if err != nil { fmt.Printf("error validating config: %v\n", err) - if exitOnError { - os.Exit(1) - } - return + os.Exit(1) } // Allocate a block of memory to alter GC behaviour. See https://github.com/golang/go/issues/23044 @@ -118,9 +111,10 @@ func main() { // Parse -config.file option via separate flag set, to avoid polluting default one and calling flag.Parse on it twice. func parseConfigFileParameter() string { var configFile = "" - fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError) // ignore unknown flags here. - fs.SetOutput(ioutil.Discard) // eat all error messages for unknown flags, and default Usage output - fs.StringVar(&configFile, configFileOption, "", "") // usage not used in this function. + // ignore errors and any output here. Any flag errors will be reported by main flag.Parse() call. + fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError) + fs.SetOutput(ioutil.Discard) + fs.StringVar(&configFile, configFileOption, "", "") // usage not used in this function. _ = fs.Parse(os.Args[1:]) return configFile @@ -140,3 +134,12 @@ func LoadConfig(filename string, cfg *cortex.Config) error { return nil } + +func DumpYaml(cfg *cortex.Config) { + out, err := yaml.Marshal(cfg) + if err != nil { + fmt.Fprintln(os.Stderr, err) + } else { + fmt.Printf("%s\n", out) + } +} diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index 659916257c8..03e476c895c 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -18,13 +18,13 @@ func TestName(t *testing.T) { yaml string message string }{ - "-h": { + "help": { arguments: []string{"-h"}, message: configFileOption, }, // check that config file is used - "-config.file": { + "config with unknown target": { yaml: "target: unknown", message: "unrecognised module name: unknown", }, @@ -34,12 +34,27 @@ func TestName(t *testing.T) { message: "unknown.flag", }, - "-config.file + argument override": { + "config with wrong argument override": { yaml: "target: ingester", arguments: []string{"-target=unknown"}, message: "unrecognised module name: unknown", }, + "default values": { + message: "target: all\n", + }, + + "config": { + yaml: "target: ingester", + message: "target: ingester\n", + }, + + "config with arguments override": { + yaml: "target: ingester", + arguments: []string{"-target=distributor"}, + message: "target: distributor\n", + }, + // we cannot test the happy path, as cortex would then fully start } { t.Run(name, func(t *testing.T) { @@ -73,7 +88,7 @@ func testSingle(t *testing.T, arguments []string, yaml string, message []byte) { arguments = append([]string{"./cortex"}, arguments...) - exitOnError = false + testMode = true os.Args = arguments co := captureOutput(t) @@ -83,7 +98,7 @@ func testSingle(t *testing.T, arguments []string, yaml string, message []byte) { main() stdout, stderr := co.Done() - require.True(t, bytes.Contains(stdout, message) || bytes.Contains(stderr, message)) + require.True(t, bytes.Contains(stdout, message) || bytes.Contains(stderr, message), "Message expected in output: %q, got stdout: %q and stderr: %q", message, stdout, stderr) } type capturedOutput struct { diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index a512d92f87c..27675823b0b 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -137,6 +137,10 @@ func (m *moduleName) Set(s string) error { } } +func (m moduleName) MarshalYAML() (interface{}, error) { + return m.String(), nil +} + func (m *moduleName) UnmarshalYAML(unmarshal func(interface{}) error) error { var s string if err := unmarshal(&s); err != nil { diff --git a/pkg/ring/kv/multi.go b/pkg/ring/kv/multi.go index 8831540cd0f..73c14194791 100644 --- a/pkg/ring/kv/multi.go +++ b/pkg/ring/kv/multi.go @@ -50,7 +50,7 @@ type MultiConfig struct { MirrorTimeout time.Duration `yaml:"mirror_timeout"` // ConfigProvider returns channel with MultiRuntimeConfig updates. - ConfigProvider func() <-chan MultiRuntimeConfig + ConfigProvider func() <-chan MultiRuntimeConfig `yaml:"-"` } // RegisterFlagsWithPrefix registers flags with prefix. diff --git a/pkg/util/runtimeconfig/manager.go b/pkg/util/runtimeconfig/manager.go index a6b21857ea9..3f3a8bd6758 100644 --- a/pkg/util/runtimeconfig/manager.go +++ b/pkg/util/runtimeconfig/manager.go @@ -24,7 +24,7 @@ type Loader func(filename string) (interface{}, error) type ManagerConfig struct { ReloadPeriod time.Duration `yaml:"period"` LoadPath string `yaml:"file"` - Loader Loader + Loader Loader `yaml:"-"` } // RegisterFlags registers flags. From f742d9e91e282bc7066df1745bc94ba3ce14c6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Fri, 17 Jan 2020 21:21:48 +0100 Subject: [PATCH 6/9] Separate stdout/stderr expected messages and checks. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 2 +- cmd/cortex/main_test.go | 53 +++++++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 5d2da65bc22..728ce0b51b3 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -45,7 +45,7 @@ func main() { if configFile != "" { if err := LoadConfig(configFile, &cfg); err != nil { - fmt.Printf("error loading config from %s: %v\n", configFile, err) + fmt.Fprintf(os.Stderr, "error loading config from %s: %v\n", configFile, err) if testMode { return } diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index 03e476c895c..e9aa843dc8d 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -14,56 +14,62 @@ import ( func TestName(t *testing.T) { for name, tc := range map[string]struct { - arguments []string - yaml string - message string + arguments []string + yaml string + stdoutMessage string // string that must be included in stdout + stderrMessage string // string that must be included in stderr }{ "help": { - arguments: []string{"-h"}, - message: configFileOption, + arguments: []string{"-h"}, + stderrMessage: configFileOption, }, // check that config file is used "config with unknown target": { - yaml: "target: unknown", - message: "unrecognised module name: unknown", + yaml: "target: unknown", + stderrMessage: "unrecognised module name: unknown", + }, + + "argument with unknown target": { + arguments: []string{"-target=unknown"}, + stderrMessage: "unrecognised module name: unknown", }, "unknown flag": { - arguments: []string{"-unknown.flag"}, - message: "unknown.flag", + arguments: []string{"-unknown.flag"}, + stderrMessage: "-unknown.flag", }, "config with wrong argument override": { - yaml: "target: ingester", - arguments: []string{"-target=unknown"}, - message: "unrecognised module name: unknown", + yaml: "target: ingester", + arguments: []string{"-target=unknown"}, + stderrMessage: "unrecognised module name: unknown", }, "default values": { - message: "target: all\n", + stdoutMessage: "target: all\n", }, "config": { - yaml: "target: ingester", - message: "target: ingester\n", + yaml: "target: ingester", + stdoutMessage: "target: ingester\n", }, "config with arguments override": { - yaml: "target: ingester", - arguments: []string{"-target=distributor"}, - message: "target: distributor\n", + yaml: "target: ingester", + arguments: []string{"-target=distributor"}, + stdoutMessage: "target: distributor\n", }, // we cannot test the happy path, as cortex would then fully start } { t.Run(name, func(t *testing.T) { - testSingle(t, tc.arguments, tc.yaml, []byte(tc.message)) + testSingle(t, tc.arguments, tc.yaml, []byte(tc.stdoutMessage), []byte(tc.stderrMessage)) }) } } -func testSingle(t *testing.T, arguments []string, yaml string, message []byte) { +func testSingle(t *testing.T, arguments []string, yaml string, stdoutMessage, stderrMessage []byte) { oldArgs, oldStdout, oldStderr := os.Args, os.Stdout, os.Stderr defer func() { os.Stdout = oldStdout @@ -98,7 +104,12 @@ func testSingle(t *testing.T, arguments []string, yaml string, message []byte) { main() stdout, stderr := co.Done() - require.True(t, bytes.Contains(stdout, message) || bytes.Contains(stderr, message), "Message expected in output: %q, got stdout: %q and stderr: %q", message, stdout, stderr) + if !bytes.Contains(stdout, stdoutMessage) { + t.Errorf("Expected on stdout: %q, stdout: %s\n", stdoutMessage, stdout) + } + if !bytes.Contains(stderr, stderrMessage) { + t.Errorf("Expected on stderr: %q, stderr: %s\n", stderrMessage, stderr) + } } type capturedOutput struct { From 85636f98736746fb06794b199889882aeb669c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 21 Jan 2020 09:45:22 +0100 Subject: [PATCH 7/9] Use mutexProfileFraction only after flags are parsed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 728ce0b51b3..0503298329f 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -59,10 +59,6 @@ func main() { flag.IntVar(&ballastBytes, "mem-ballast-size-bytes", 0, "Size of memory ballast to allocate.") flag.IntVar(&mutexProfileFraction, "debug.mutex-profile-fraction", 0, "Fraction at which mutex profile vents will be reported, 0 to disable") - if mutexProfileFraction > 0 { - runtime.SetMutexProfileFraction(mutexProfileFraction) - } - if testMode { // Don't exit on error in test mode. Just parse parameters, dump config and stop. flag.CommandLine.Init(flag.CommandLine.Name(), flag.ContinueOnError) @@ -73,6 +69,10 @@ func main() { flag.Parse() + if mutexProfileFraction > 0 { + runtime.SetMutexProfileFraction(mutexProfileFraction) + } + // Validate the config once both the config file has been loaded // and CLI flags parsed. err := cfg.Validate() From f0db5a002383a0fca2fae0008bcf3a729480823a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 21 Jan 2020 09:52:26 +0100 Subject: [PATCH 8/9] Renamed default test name. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index e9aa843dc8d..dfe1e41f997 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestName(t *testing.T) { +func TestFlagParsing(t *testing.T) { for name, tc := range map[string]struct { arguments []string yaml string From 9efeaf0e5917f364358bce4f38a7f7d0c56f9b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20S=CC=8Ctibrany=CC=81?= Date: Tue, 21 Jan 2020 10:37:37 +0100 Subject: [PATCH 9/9] Reset testMode to original value at the end of test. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Peter Štibraný --- cmd/cortex/main_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index dfe1e41f997..17d3ab39a10 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -70,11 +70,12 @@ func TestFlagParsing(t *testing.T) { } func testSingle(t *testing.T, arguments []string, yaml string, stdoutMessage, stderrMessage []byte) { - oldArgs, oldStdout, oldStderr := os.Args, os.Stdout, os.Stderr + oldArgs, oldStdout, oldStderr, oldTestMode := os.Args, os.Stdout, os.Stderr, testMode defer func() { os.Stdout = oldStdout os.Stderr = oldStderr os.Args = oldArgs + testMode = oldTestMode }() if yaml != "" {