Skip to content
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

set correct exit code when running a script with an invalid option #3783

Merged
merged 7 commits into from
Jul 1, 2024
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
6 changes: 2 additions & 4 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}
envConf, err := readEnvConfig(gs.Env)
if err != nil {
return conf, err
return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}

conf = cliConf.Apply(fileConf)
Expand Down
47 changes: 47 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
testFilename, name string
expErr, expLogOutput string
expExitCode exitcodes.ExitCode
configFilename string
extraArgs []string
envVars map[string]string
}{
{
testFilename: "abort.js",
Expand Down Expand Up @@ -146,6 +148,41 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
expErr: "ReferenceError: someUndefinedVar is not defined",
expExitCode: exitcodes.ScriptException,
},
{
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: "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,
envVars: map[string]string{
"K6_DURATION": "fails",
},
},
{
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: "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: "invalidconfig/invalid.json",
},
{
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,
},
{
testFilename: "thresholds/non_existing_metric.js",
name: "run should fail with exit status 104 on a threshold applied to a non existing metric",
Expand Down Expand Up @@ -203,6 +240,16 @@ 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", 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
}

ts.ExpectedExitCode = int(tc.expExitCode)
newRootCommand(ts.GlobalState).execute()

Expand Down
3 changes: 3 additions & 0 deletions cmd/testdata/invalidconfig/invalid.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"duration": "fails"
}
5 changes: 5 additions & 0 deletions cmd/testdata/invalidconfig/invalid_option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const options = {
vus: 'this is an invalid type',
}

export default function () {}
22 changes: 22 additions & 0 deletions cmd/testdata/invalidconfig/invalid_scenario.js
Original file line number Diff line number Diff line change
@@ -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: {
/*...*/
},
},
};

10 changes: 10 additions & 0 deletions cmd/testdata/invalidconfig/option_env.js
Original file line number Diff line number Diff line change
@@ -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');
}
7 changes: 6 additions & 1 deletion js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(uerr, exitcodes.InvalidConfig),
errext.AbortedByScriptError,
)
}
b.preInitState.Logger.WithError(err).Warn("There were unknown fields in the options exported in the script")
}
Expand Down