From 518226a47d594e603922eb672db4ff7129747e9d Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Fri, 1 Oct 2021 01:22:24 +0100 Subject: [PATCH 1/7] Support camelCase csv output argument for file_name and save_interval. This change adds support for camelCased versions of file_name and save_interval CSV output arguments, writing a warning to the logs informing users the older snake cased argument will be deprecated in time. --- output/csv/config.go | 13 ++++++++++--- output/csv/config_test.go | 19 ++++++++++++++++++- output/csv/output.go | 10 +++++----- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/output/csv/config.go b/output/csv/config.go index 6d696e46d7e9..5b1356d9932c 100644 --- a/output/csv/config.go +++ b/output/csv/config.go @@ -29,6 +29,7 @@ import ( "gopkg.in/guregu/null.v3" "github.com/kelseyhightower/envconfig" + "github.com/sirupsen/logrus" "go.k6.io/k6/lib/types" ) @@ -59,7 +60,7 @@ func (c Config) Apply(cfg Config) Config { } // ParseArg takes an arg string and converts it to a config -func ParseArg(arg string) (Config, error) { +func ParseArg(arg string, logger *logrus.Logger) (Config, error) { c := Config{} if !strings.Contains(arg, "=") { @@ -76,11 +77,17 @@ func ParseArg(arg string) (Config, error) { } switch r[0] { case "save_interval": + logger.Warnf("CSV output argument '%s' will soon be deprecated, please use 'saveInterval' instead.", r[0]) + fallthrough + case "saveInterval": err := c.SaveInterval.UnmarshalText([]byte(r[1])) if err != nil { return c, err } case "file_name": + logger.Warnf("CSV output argument '%s' will soon be deprecated, please use 'fileName' instead.", r[0]) + fallthrough + case "fileName": c.FileName = null.StringFrom(r[1]) default: return c, fmt.Errorf("unknown key %q as argument for csv output", r[0]) @@ -92,7 +99,7 @@ func ParseArg(arg string) (Config, error) { // GetConsolidatedConfig combines {default config values + JSON config + // environment vars + arg config values}, and returns the final result. -func GetConsolidatedConfig(jsonRawConf json.RawMessage, env map[string]string, arg string) (Config, error) { +func GetConsolidatedConfig(jsonRawConf json.RawMessage, env map[string]string, arg string, logger *logrus.Logger) (Config, error) { result := NewConfig() if jsonRawConf != nil { jsonConf := Config{} @@ -110,7 +117,7 @@ func GetConsolidatedConfig(jsonRawConf json.RawMessage, env map[string]string, a result = result.Apply(envConfig) if arg != "" { - urlConf, err := ParseArg(arg) + urlConf, err := ParseArg(arg, logger) if err != nil { return result, err } diff --git a/output/csv/config_test.go b/output/csv/config_test.go index fd6e66479036..1b5d254cdfe4 100644 --- a/output/csv/config_test.go +++ b/output/csv/config_test.go @@ -24,8 +24,11 @@ import ( "testing" "time" + "go.k6.io/k6/lib/testutils" + "gopkg.in/guregu/null.v3" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "go.k6.io/k6/lib/types" @@ -91,23 +94,37 @@ func TestParseArg(t *testing.T) { SaveInterval: types.NullDurationFrom(5 * time.Second), }, }, + "saveInterval=5s": { + config: Config{ + SaveInterval: types.NullDurationFrom(5 * time.Second), + }, + }, "file_name=test.csv,save_interval=5s": { config: Config{ FileName: null.StringFrom("test.csv"), SaveInterval: types.NullDurationFrom(5 * time.Second), }, }, + "fileName=test.csv,save_interval=5s": { + config: Config{ + FileName: null.StringFrom("test.csv"), + SaveInterval: types.NullDurationFrom(5 * time.Second), + }, + }, "filename=test.csv,save_interval=5s": { expectedErr: true, }, } + testLog := logrus.New() + testLog.SetOutput(testutils.NewTestOutput(t)) + for arg, testCase := range cases { arg := arg testCase := testCase t.Run(arg, func(t *testing.T) { - config, err := ParseArg(arg) + config, err := ParseArg(arg, testLog) if testCase.expectedErr { assert.Error(t, err) diff --git a/output/csv/output.go b/output/csv/output.go index be5c4573fd8a..005abe819f61 100644 --- a/output/csv/output.go +++ b/output/csv/output.go @@ -76,7 +76,11 @@ func newOutput(params output.Params) (*Output, error) { sort.Strings(resTags) sort.Strings(ignoredTags) - config, err := GetConsolidatedConfig(params.JSONConfig, params.Environment, params.ConfigArgument) + logger := params.Logger.WithFields(logrus.Fields{ + "output": "csv", + "filename": params.ConfigArgument, + }) + config, err := GetConsolidatedConfig(params.JSONConfig, params.Environment, params.ConfigArgument, logger.Logger) if err != nil { return nil, err } @@ -84,10 +88,6 @@ func newOutput(params output.Params) (*Output, error) { saveInterval := time.Duration(config.SaveInterval.Duration) fname := config.FileName.String - logger := params.Logger.WithFields(logrus.Fields{ - "output": "csv", - "filename": params.ConfigArgument, - }) if fname == "" || fname == "-" { stdoutWriter := csv.NewWriter(os.Stdout) return &Output{ From 10d78a52f99986d06840a1aae684aa22e8ddc1da Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Sat, 2 Oct 2021 23:01:28 +0100 Subject: [PATCH 2/7] Address linting error --- output/csv/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/output/csv/config.go b/output/csv/config.go index 5b1356d9932c..8c6f2d121fb0 100644 --- a/output/csv/config.go +++ b/output/csv/config.go @@ -99,6 +99,7 @@ func ParseArg(arg string, logger *logrus.Logger) (Config, error) { // GetConsolidatedConfig combines {default config values + JSON config + // environment vars + arg config values}, and returns the final result. +//nolint: lll func GetConsolidatedConfig(jsonRawConf json.RawMessage, env map[string]string, arg string, logger *logrus.Logger) (Config, error) { result := NewConfig() if jsonRawConf != nil { From 4ed547fc9e53ad2e1490e56508531611e8695cdf Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Tue, 5 Oct 2021 22:05:39 +0100 Subject: [PATCH 3/7] Update output/csv/config.go Co-authored-by: Mihail Stoykov --- output/csv/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/output/csv/config.go b/output/csv/config.go index 8c6f2d121fb0..c9f259862be7 100644 --- a/output/csv/config.go +++ b/output/csv/config.go @@ -77,7 +77,7 @@ func ParseArg(arg string, logger *logrus.Logger) (Config, error) { } switch r[0] { case "save_interval": - logger.Warnf("CSV output argument '%s' will soon be deprecated, please use 'saveInterval' instead.", r[0]) + logger.Warnf("CSV output argument '%s' is deprecated, please use 'saveInterval' instead.", r[0]) fallthrough case "saveInterval": err := c.SaveInterval.UnmarshalText([]byte(r[1])) From 0e93149f63fb5f34d547e9a3c85f27f84e857954 Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 6 Oct 2021 00:33:45 +0100 Subject: [PATCH 4/7] Correct csv output warning to indicate field is already deprecated --- output/csv/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/output/csv/config.go b/output/csv/config.go index c9f259862be7..3e14c1732255 100644 --- a/output/csv/config.go +++ b/output/csv/config.go @@ -85,7 +85,7 @@ func ParseArg(arg string, logger *logrus.Logger) (Config, error) { return c, err } case "file_name": - logger.Warnf("CSV output argument '%s' will soon be deprecated, please use 'fileName' instead.", r[0]) + logger.Warnf("CSV output argument '%s' is deprecated, please use 'fileName' instead.", r[0]) fallthrough case "fileName": c.FileName = null.StringFrom(r[1]) From ac736b0f9e7c19d1ad089c42fca2d434e73c64ba Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 6 Oct 2021 00:37:17 +0100 Subject: [PATCH 5/7] Remove nolint comment in favour of formatting long function accordingly --- output/csv/config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/output/csv/config.go b/output/csv/config.go index 3e14c1732255..18e5011e211d 100644 --- a/output/csv/config.go +++ b/output/csv/config.go @@ -99,8 +99,9 @@ func ParseArg(arg string, logger *logrus.Logger) (Config, error) { // GetConsolidatedConfig combines {default config values + JSON config + // environment vars + arg config values}, and returns the final result. -//nolint: lll -func GetConsolidatedConfig(jsonRawConf json.RawMessage, env map[string]string, arg string, logger *logrus.Logger) (Config, error) { +func GetConsolidatedConfig( + jsonRawConf json.RawMessage, env map[string]string, arg string, logger *logrus.Logger, +) (Config, error) { result := NewConfig() if jsonRawConf != nil { jsonConf := Config{} From 199248a32ea15a954c3db2d760e89435893a05f4 Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 6 Oct 2021 00:38:03 +0100 Subject: [PATCH 6/7] Add test over csv output deprecation log warning --- output/csv/config_test.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/output/csv/config_test.go b/output/csv/config_test.go index 1b5d254cdfe4..6106af2f60ed 100644 --- a/output/csv/config_test.go +++ b/output/csv/config_test.go @@ -24,13 +24,12 @@ import ( "testing" "time" - "go.k6.io/k6/lib/testutils" - "gopkg.in/guregu/null.v3" - "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" + "go.k6.io/k6/lib/testutils" "go.k6.io/k6/lib/types" ) @@ -80,8 +79,9 @@ func TestApply(t *testing.T) { func TestParseArg(t *testing.T) { cases := map[string]struct { - config Config - expectedErr bool + config Config + expectedLogEntries []string + expectedErr bool }{ "test_file.csv": { config: Config{ @@ -93,6 +93,9 @@ func TestParseArg(t *testing.T) { config: Config{ SaveInterval: types.NullDurationFrom(5 * time.Second), }, + expectedLogEntries: []string{ + "CSV output argument 'save_interval' is deprecated, please use 'saveInterval' instead.", + }, }, "saveInterval=5s": { config: Config{ @@ -104,27 +107,34 @@ func TestParseArg(t *testing.T) { FileName: null.StringFrom("test.csv"), SaveInterval: types.NullDurationFrom(5 * time.Second), }, + expectedLogEntries: []string{ + "CSV output argument 'file_name' is deprecated, please use 'fileName' instead.", + "CSV output argument 'save_interval' is deprecated, please use 'saveInterval' instead.", + }, }, "fileName=test.csv,save_interval=5s": { config: Config{ FileName: null.StringFrom("test.csv"), SaveInterval: types.NullDurationFrom(5 * time.Second), }, + expectedLogEntries: []string{ + "CSV output argument 'save_interval' is deprecated, please use 'saveInterval' instead.", + }, }, "filename=test.csv,save_interval=5s": { expectedErr: true, }, } - testLog := logrus.New() - testLog.SetOutput(testutils.NewTestOutput(t)) - for arg, testCase := range cases { arg := arg testCase := testCase + testLogger, hook := test.NewNullLogger() + testLogger.SetOutput(testutils.NewTestOutput(t)) + t.Run(arg, func(t *testing.T) { - config, err := ParseArg(arg, testLog) + config, err := ParseArg(arg, testLogger) if testCase.expectedErr { assert.Error(t, err) @@ -133,6 +143,12 @@ func TestParseArg(t *testing.T) { } assert.Equal(t, testCase.config.FileName.String, config.FileName.String) assert.Equal(t, testCase.config.SaveInterval.String(), config.SaveInterval.String()) + + var entries []string + for _, v := range hook.AllEntries() { + entries = append(entries, v.Message) + } + assert.ElementsMatch(t, entries, testCase.expectedLogEntries) }) } } From 83070295cf9a707ab4ab8b3c7d0b3f68660a9c00 Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 6 Oct 2021 00:50:23 +0100 Subject: [PATCH 7/7] Add assertion to log level --- output/csv/config_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/output/csv/config_test.go b/output/csv/config_test.go index 6106af2f60ed..cb53cd7c81eb 100644 --- a/output/csv/config_test.go +++ b/output/csv/config_test.go @@ -24,6 +24,8 @@ import ( "testing" "time" + "github.com/sirupsen/logrus" + "gopkg.in/guregu/null.v3" "github.com/sirupsen/logrus/hooks/test" @@ -146,6 +148,7 @@ func TestParseArg(t *testing.T) { var entries []string for _, v := range hook.AllEntries() { + assert.Equal(t, v.Level, logrus.WarnLevel) entries = append(entries, v.Message) } assert.ElementsMatch(t, entries, testCase.expectedLogEntries)