From 5265061c83ccb2583fd38cd3b2e7cbeb8b66199a Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Fri, 7 Jun 2024 21:14:27 +0200 Subject: [PATCH 1/7] set correct exit code when running a script with an invalid option Closes #3759 --- cmd/tests/cmd_run_test.go | 15 +++++++++++++++ js/bundle.go | 7 ++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/cmd/tests/cmd_run_test.go b/cmd/tests/cmd_run_test.go index bbc372690ea..6cfce0ebbab 100644 --- a/cmd/tests/cmd_run_test.go +++ b/cmd/tests/cmd_run_test.go @@ -2027,6 +2027,21 @@ func TestEventSystemError(t *testing.T) { }, expExitCode: exitcodes.ScriptException, }, + { + name: "invalid", + script: ` + export const options = { + vus: 'this is an invalid type', + } + + export default function () {} + `, + expLog: []string{ + "got event Exit with data '&{Error:could not initialize '-': could not load JS test 'file:///-': strconv.ParseInt: parsing \"this is an invalid type\": invalid syntax}'", + "could not initialize '-': could not load JS test 'file:///-': strconv.ParseInt: parsing \"this is an invalid type\": invalid syntax", + }, + expExitCode: exitcodes.InvalidConfig, + }, { name: "throw", script: ` diff --git a/js/bundle.go b/js/bundle.go index 0f694339722..bdfc489f246 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -15,6 +15,8 @@ import ( "github.com/sirupsen/logrus" "gopkg.in/guregu/null.v3" + "go.k6.io/k6/errext" + "go.k6.io/k6/errext/exitcodes" "go.k6.io/k6/event" "go.k6.io/k6/js/common" "go.k6.io/k6/js/compiler" @@ -208,7 +210,10 @@ func (b *Bundle) populateExports(updateOptions bool, exports *goja.Object) error dec.DisallowUnknownFields() if err := dec.Decode(&b.Options); err != nil { if uerr := json.Unmarshal(data, &b.Options); uerr != nil { - return uerr + return errext.WithAbortReasonIfNone( + errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig), + errext.AbortedByScriptError, + ) } b.preInitState.Logger.WithError(err).Warn("There were unknown fields in the options exported in the script") } From af7cdea8ed0a3cc289243e02efea3934575899ab Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Mon, 17 Jun 2024 18:33:51 +0200 Subject: [PATCH 2/7] fix typo --- js/bundle.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/bundle.go b/js/bundle.go index bdfc489f246..9c9941da4f1 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -211,7 +211,7 @@ func (b *Bundle) populateExports(updateOptions bool, exports *goja.Object) error if err := dec.Decode(&b.Options); err != nil { if uerr := json.Unmarshal(data, &b.Options); uerr != nil { return errext.WithAbortReasonIfNone( - errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig), + errext.WithExitCodeIfNone(uerr, exitcodes.InvalidConfig), errext.AbortedByScriptError, ) } From 20d745797c7fa6f5b820cace23fe14abc6076e23 Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Thu, 20 Jun 2024 19:46:51 +0200 Subject: [PATCH 3/7] move test to a better place --- cmd/run_test.go | 6 ++++++ cmd/testdata/invalid_option.js | 5 +++++ cmd/tests/cmd_run_test.go | 15 --------------- 3 files changed, 11 insertions(+), 15 deletions(-) create mode 100644 cmd/testdata/invalid_option.js diff --git a/cmd/run_test.go b/cmd/run_test.go index aa3661f2301..4266c221771 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -146,6 +146,12 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expErr: "ReferenceError: someUndefinedVar is not defined", expExitCode: exitcodes.ScriptException, }, + { + testFilename: "invalid_option.js", + name: "run should fail with exit status 104 if an invalid option value exists", + expErr: "this is an invalid type", + expExitCode: exitcodes.InvalidConfig, + }, { testFilename: "thresholds/non_existing_metric.js", name: "run should fail with exit status 104 on a threshold applied to a non existing metric", diff --git a/cmd/testdata/invalid_option.js b/cmd/testdata/invalid_option.js new file mode 100644 index 00000000000..d1ee341ecf2 --- /dev/null +++ b/cmd/testdata/invalid_option.js @@ -0,0 +1,5 @@ +export const options = { + vus: 'this is an invalid type', +} + +export default function () {} diff --git a/cmd/tests/cmd_run_test.go b/cmd/tests/cmd_run_test.go index 6cfce0ebbab..bbc372690ea 100644 --- a/cmd/tests/cmd_run_test.go +++ b/cmd/tests/cmd_run_test.go @@ -2027,21 +2027,6 @@ func TestEventSystemError(t *testing.T) { }, expExitCode: exitcodes.ScriptException, }, - { - name: "invalid", - script: ` - export const options = { - vus: 'this is an invalid type', - } - - export default function () {} - `, - expLog: []string{ - "got event Exit with data '&{Error:could not initialize '-': could not load JS test 'file:///-': strconv.ParseInt: parsing \"this is an invalid type\": invalid syntax}'", - "could not initialize '-': could not load JS test 'file:///-': strconv.ParseInt: parsing \"this is an invalid type\": invalid syntax", - }, - expExitCode: exitcodes.InvalidConfig, - }, { name: "throw", script: ` From 0573ddad19ad7d6c64ec99743e1519f162f8b846 Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Fri, 21 Jun 2024 19:35:14 +0200 Subject: [PATCH 4/7] add fix and test case for env and k6 variables --- cmd/config.go | 4 +--- cmd/run_test.go | 21 +++++++++++++++++++++ cmd/testdata/option_env.js | 10 ++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 cmd/testdata/option_env.js diff --git a/cmd/config.go b/cmd/config.go index 3aa6237450c..b68daecf0e6 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -164,15 +164,13 @@ func readEnvConfig(envMap map[string]string) (Config, error) { // TODO: add better validation, more explicit default values and improve consistency between formats // TODO: accumulate all errors and differentiate between the layers? func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib.Options) (conf Config, err error) { - // TODO: use errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) where it makes sense? - fileConf, err := readDiskConfig(gs) if err != nil { return conf, err } envConf, err := readEnvConfig(gs.Env) if err != nil { - return conf, err + return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } conf = cliConf.Apply(fileConf) diff --git a/cmd/run_test.go b/cmd/run_test.go index 4266c221771..0f5b44ef139 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -119,6 +119,7 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expErr, expLogOutput string expExitCode exitcodes.ExitCode extraArgs []string + envVars map[string]string }{ { testFilename: "abort.js", @@ -152,6 +153,22 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expErr: "this is an invalid type", expExitCode: exitcodes.InvalidConfig, }, + { + testFilename: "option_env.js", + name: "run should fail with exit status 104 if an invalid option is set through env variable", + expErr: "envconfig.Process", + expExitCode: exitcodes.InvalidConfig, + envVars: map[string]string{ + "K6_DURATION": "fails", + }, + }, + { + testFilename: "option_env.js", + name: "run should fail with exit status 104 if an invalid option is set through k6 variable", + expErr: "invalid duration", + expExitCode: exitcodes.InvalidConfig, + extraArgs: []string{"--env", "DURATION=fails"}, + }, { testFilename: "thresholds/non_existing_metric.js", name: "run should fail with exit status 104 on a threshold applied to a non existing metric", @@ -209,6 +226,10 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.testFilename), testScript, 0o644)) ts.CmdArgs = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...) + if tc.envVars != nil { + ts.Env = tc.envVars + } + ts.ExpectedExitCode = int(tc.expExitCode) newRootCommand(ts.GlobalState).execute() diff --git a/cmd/testdata/option_env.js b/cmd/testdata/option_env.js new file mode 100644 index 00000000000..79fdd0ff815 --- /dev/null +++ b/cmd/testdata/option_env.js @@ -0,0 +1,10 @@ +import http from 'k6/http'; + +export const options = { + iterations: 1, + duration: __ENV.DURATION, +}; + +export default function () { + const res = http.get('https://test.k6.io'); +} From 0834bf835f59830ba5e4f65e2f18bd43e2422d6a Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Fri, 21 Jun 2024 19:47:02 +0200 Subject: [PATCH 5/7] add fix and test case for option using config file --- cmd/config.go | 2 +- cmd/run_test.go | 14 ++++++++++++++ cmd/testdata/config/invalid.json | 3 +++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 cmd/testdata/config/invalid.json diff --git a/cmd/config.go b/cmd/config.go index b68daecf0e6..4fc50d83dd9 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -166,7 +166,7 @@ func readEnvConfig(envMap map[string]string) (Config, error) { func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib.Options) (conf Config, err error) { fileConf, err := readDiskConfig(gs) if err != nil { - return conf, err + return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } envConf, err := readEnvConfig(gs.Env) if err != nil { diff --git a/cmd/run_test.go b/cmd/run_test.go index 0f5b44ef139..fc7e8365337 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -118,6 +118,7 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { testFilename, name string expErr, expLogOutput string expExitCode exitcodes.ExitCode + configFilename string extraArgs []string envVars map[string]string }{ @@ -169,6 +170,13 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expExitCode: exitcodes.InvalidConfig, extraArgs: []string{"--env", "DURATION=fails"}, }, + { + testFilename: "option_env.js", + name: "run should fail with exit status 104 if an invalid option is set in a config file", + expErr: "invalid duration", + expExitCode: exitcodes.InvalidConfig, + configFilename: "invalid.json", + }, { testFilename: "thresholds/non_existing_metric.js", name: "run should fail with exit status 104 on a threshold applied to a non existing metric", @@ -226,6 +234,12 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.testFilename), testScript, 0o644)) ts.CmdArgs = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...) + if tc.configFilename != "" { + configFile, err := os.ReadFile(path.Join("testdata", "config", tc.configFilename)) //nolint:forbidigo + require.NoError(t, err) + require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.configFilename), configFile, 0o644)) + ts.Flags.ConfigFilePath = path.Join(ts.Cwd, tc.configFilename) + } if tc.envVars != nil { ts.Env = tc.envVars } diff --git a/cmd/testdata/config/invalid.json b/cmd/testdata/config/invalid.json new file mode 100644 index 00000000000..ab405b20f3c --- /dev/null +++ b/cmd/testdata/config/invalid.json @@ -0,0 +1,3 @@ +{ + "duration": "fails" +} From d34e646da263d2587a0a480e117c5fae0a65ecd9 Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Fri, 21 Jun 2024 19:53:12 +0200 Subject: [PATCH 6/7] add test case for invalid scenario --- cmd/run_test.go | 6 ++++++ cmd/testdata/invalid_scenario.js | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 cmd/testdata/invalid_scenario.js diff --git a/cmd/run_test.go b/cmd/run_test.go index fc7e8365337..4f47cce70be 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -177,6 +177,12 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expExitCode: exitcodes.InvalidConfig, configFilename: "invalid.json", }, + { + testFilename: "invalid_scenario.js", + name: "run should fail with exit status 104 if an invalid scenario exists", + expErr: "specified executor type", + expExitCode: exitcodes.InvalidConfig, + }, { testFilename: "thresholds/non_existing_metric.js", name: "run should fail with exit status 104 on a threshold applied to a non existing metric", diff --git a/cmd/testdata/invalid_scenario.js b/cmd/testdata/invalid_scenario.js new file mode 100644 index 00000000000..35bb2c12b12 --- /dev/null +++ b/cmd/testdata/invalid_scenario.js @@ -0,0 +1,22 @@ +export const options = { + scenarios: { + example_scenario: { + // name of the executor to use + executor: 'shared-iterations', + // common scenario configuration + startTime: '10s', + gracefulStop: '5s', + env: { EXAMPLEVAR: 'testing' }, + tags: { example_tag: 'testing' }, + + // executor-specific configuration + vus: 10, + iterations: 200, + maxDuration: '10s', + }, + another_scenario: { + /*...*/ + }, + }, + }; + \ No newline at end of file From 9ebe2b51b9c8450ad3836488da7aa8ba0ac2236d Mon Sep 17 00:00:00 2001 From: Ismael Arias Date: Thu, 27 Jun 2024 19:25:44 +0200 Subject: [PATCH 7/7] move all testdata files to dedicated "invalidconfig" dir --- cmd/run_test.go | 14 +++++++------- .../{config => invalidconfig}/invalid.json | 0 cmd/testdata/{ => invalidconfig}/invalid_option.js | 0 .../{ => invalidconfig}/invalid_scenario.js | 0 cmd/testdata/{ => invalidconfig}/option_env.js | 0 5 files changed, 7 insertions(+), 7 deletions(-) rename cmd/testdata/{config => invalidconfig}/invalid.json (100%) rename cmd/testdata/{ => invalidconfig}/invalid_option.js (100%) rename cmd/testdata/{ => invalidconfig}/invalid_scenario.js (100%) rename cmd/testdata/{ => invalidconfig}/option_env.js (100%) diff --git a/cmd/run_test.go b/cmd/run_test.go index 4f47cce70be..4b562359ccf 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -149,13 +149,13 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { expExitCode: exitcodes.ScriptException, }, { - testFilename: "invalid_option.js", + testFilename: "invalidconfig/invalid_option.js", name: "run should fail with exit status 104 if an invalid option value exists", expErr: "this is an invalid type", expExitCode: exitcodes.InvalidConfig, }, { - testFilename: "option_env.js", + testFilename: "invalidconfig/option_env.js", name: "run should fail with exit status 104 if an invalid option is set through env variable", expErr: "envconfig.Process", expExitCode: exitcodes.InvalidConfig, @@ -164,21 +164,21 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { }, }, { - testFilename: "option_env.js", + testFilename: "invalidconfig/option_env.js", name: "run should fail with exit status 104 if an invalid option is set through k6 variable", expErr: "invalid duration", expExitCode: exitcodes.InvalidConfig, extraArgs: []string{"--env", "DURATION=fails"}, }, { - testFilename: "option_env.js", + testFilename: "invalidconfig/option_env.js", name: "run should fail with exit status 104 if an invalid option is set in a config file", expErr: "invalid duration", expExitCode: exitcodes.InvalidConfig, - configFilename: "invalid.json", + configFilename: "invalidconfig/invalid.json", }, { - testFilename: "invalid_scenario.js", + testFilename: "invalidconfig/invalid_scenario.js", name: "run should fail with exit status 104 if an invalid scenario exists", expErr: "specified executor type", expExitCode: exitcodes.InvalidConfig, @@ -241,7 +241,7 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { ts.CmdArgs = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...) if tc.configFilename != "" { - configFile, err := os.ReadFile(path.Join("testdata", "config", tc.configFilename)) //nolint:forbidigo + configFile, err := os.ReadFile(path.Join("testdata", tc.configFilename)) //nolint:forbidigo require.NoError(t, err) require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.configFilename), configFile, 0o644)) ts.Flags.ConfigFilePath = path.Join(ts.Cwd, tc.configFilename) diff --git a/cmd/testdata/config/invalid.json b/cmd/testdata/invalidconfig/invalid.json similarity index 100% rename from cmd/testdata/config/invalid.json rename to cmd/testdata/invalidconfig/invalid.json diff --git a/cmd/testdata/invalid_option.js b/cmd/testdata/invalidconfig/invalid_option.js similarity index 100% rename from cmd/testdata/invalid_option.js rename to cmd/testdata/invalidconfig/invalid_option.js diff --git a/cmd/testdata/invalid_scenario.js b/cmd/testdata/invalidconfig/invalid_scenario.js similarity index 100% rename from cmd/testdata/invalid_scenario.js rename to cmd/testdata/invalidconfig/invalid_scenario.js diff --git a/cmd/testdata/option_env.js b/cmd/testdata/invalidconfig/option_env.js similarity index 100% rename from cmd/testdata/option_env.js rename to cmd/testdata/invalidconfig/option_env.js