Ensure commands validate thresholds

This commit ensures that impacted commands (run, archive, cloud)
validate thresholds before running.

cmd/archive_test.go

@@ -32,6 +32,36 @@ func TestArchiveThresholds(t *testing.T) {
 			testFilename: "testdata/thresholds/malformed_expression.js",
 			wantErr:      false,
 		},
+		{
+			name:         "run should fail with exit status 104 on a threshold applied to a non existing metric",
+			noThresholds: false,
+			testFilename: "testdata/thresholds/non_existing_metric.js",
+			wantErr:      true,
+		},
+		{
+			name:         "run should succeed on a threshold applied to a non existing metric with the --no-thresholds flag set",
+			noThresholds: true,
+			testFilename: "testdata/thresholds/non_existing_metric.js",
+			wantErr:      false,
+		},
+		{
+			name:         "run should succeed on a threshold applied to a non existing submetric with the --no-thresholds flag set",
+			noThresholds: true,
+			testFilename: "testdata/thresholds/non_existing_metric.js",
+			wantErr:      false,
+		},
+		{
+			name:         "run should fail with exit status 104 on a threshold applying an unsupported aggregation method to a metric",
+			noThresholds: false,
+			testFilename: "testdata/thresholds/unsupported_aggregation_method.js",
+			wantErr:      true,
+		},
+		{
+			name:         "run should succeed on a threshold applying an unsupported aggregation method to a metric with the --no-thresholds flag set",
+			noThresholds: true,
+			testFilename: "testdata/thresholds/unsupported_aggregation_method.js",
+			wantErr:      false,
+		},
 	}
 
 	for _, testCase := range testCases {

cmd/run_test.go

@@ -167,6 +167,22 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
 			expErr:       "ReferenceError: someUndefinedVar is not defined",
 			expExitCode:  exitcodes.ScriptException,
 		},
+		{
+			testFilename: "thresholds/non_existing_metric.js",
+			name:         "run should fail with exit status 104 on a threshold applied to a non existing metric",
+			expErr:       "invalid threshold",
+			expExitCode:  exitcodes.InvalidConfig,
+		},
+		{
+			testFilename: "thresholds/non_existing_metric.js",
+			name:         "run should succeed on a threshold applied to a non existing metric with the --no-thresholds flag set",
+			extraArgs:    []string{"--no-thresholds"},
+		},
+		{
+			testFilename: "thresholds/non_existing_metric.js",
+			name:         "run should succeed on a threshold applied to a non existing submetric with the --no-thresholds flag set",
+			extraArgs:    []string{"--no-thresholds"},
+		},
 		{
 			testFilename: "thresholds/malformed_expression.js",
 			name:         "run should fail with exit status 104 on a malformed threshold expression",
@@ -179,6 +195,17 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
 			extraArgs:    []string{"--no-thresholds"},
 			// we don't expect an error
 		},
+		{
+			testFilename: "thresholds/unsupported_aggregation_method.js",
+			name:         "run should fail with exit status 104 on a threshold applying an unsupported aggregation method to a metric",
+			expErr:       "invalid threshold",
+			expExitCode:  exitcodes.InvalidConfig,
+		},
+		{
+			testFilename: "thresholds/unsupported_aggregation_method.js",
+			name:         "run should succeed on a threshold applying an unsupported aggregation method to a metric with the --no-thresholds flag set",
+			extraArgs:    []string{"--no-thresholds"},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -212,3 +239,48 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
 		})
 	}
 }
+
+func TestInvalidOptionsThresholdErrExitCode(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name         string
+		testFilename string
+		expExitCode  errext.ExitCode
+		extraArgs    []string
+	}{
+		{
+			name:         "run should fail with exit status 104 on a malformed threshold expression",
+			testFilename: "thresholds/malformed_expression.js",
+			expExitCode:  exitcodes.InvalidConfig,
+		},
+		{
+			name:         "run should fail with exit status 104 on a threshold applied to a non existing metric",
+			testFilename: "thresholds/non_existing_metric.js",
+			expExitCode:  exitcodes.InvalidConfig,
+		},
+		{
+			name:         "run should fail with exit status 104 on a threshold method being unsupported by the metric",
+			testFilename: "thresholds/unsupported_aggregation_method.js",
+			expExitCode:  exitcodes.InvalidConfig,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			testScript, err := ioutil.ReadFile(path.Join("testdata", tc.testFilename))
+			require.NoError(t, err)
+
+			testState := newGlobalTestState(t)
+			require.NoError(t, afero.WriteFile(testState.fs, filepath.Join(testState.cwd, tc.testFilename), testScript, 0o644))
+			testState.args = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...)
+
+			testState.expectedExitCode = int(tc.expExitCode)
+			newRootCommand(testState.globalState).execute()
+		})
+	}
+}

cmd/test_load.go

@@ -184,8 +184,13 @@ func (lt *loadedTest) consolidateDeriveAndValidateConfig(
 	// If parsing the threshold expressions failed, consider it as an
 	// invalid configuration error.
 	if !lt.runtimeOptions.NoThresholds.Bool {
-		for _, thresholds := range consolidatedConfig.Options.Thresholds {
-			err = thresholds.Parse()
+		for metricName, thresholdsDefinition := range consolidatedConfig.Options.Thresholds {
+			err = thresholdsDefinition.Parse()
+			if err != nil {
+				return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
+			}
+
+			err = thresholdsDefinition.Validate(metricName, lt.metricsRegistry)
 			if err != nil {
 				return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
 			}

cmd/testdata/thresholds/non_existing_metric.js

@@ -0,0 +1,13 @@
+export const options = {
+	thresholds: {
+		// non existing is neither registered, nor a builtin metric.
+		// k6 should catch that.
+		"non existing": ["rate>0"],
+	},
+};
+
+export default function () {
+	console.log(
+		"asserting that a threshold over a non-existing metric fails with exit code 104 (Invalid config)"
+	);
+}

cmd/testdata/thresholds/non_existing_submetric.js

@@ -0,0 +1,13 @@
+export const options = {
+	thresholds: {
+		// http_reqs being a builtin metric has no foo:bar
+		// tag definition. As such k6 should detect it and
+		"http_reqs{foo:bar}": ["count>0"],
+	},
+};
+
+export default function () {
+	console.log(
+		"asserting that a threshold applying a method over a metric not supporting it fails with exit code 104 (Invalid config)"
+	);
+}

cmd/testdata/thresholds/unsupported_aggregation_method.js

@@ -0,0 +1,15 @@
+export const options = {
+	thresholds: {
+		// http_reqs is a Counter metric. As such, it supports
+		// only the 'count' and 'rate' operations. Thus, 'value'
+		// being a Gauge's metric aggregation method, the threshold
+		// configuration evaluation should fail.
+		http_reqs: ["value>0"],
+	},
+};
+
+export default function () {
+	console.log(
+		"asserting that a threshold applying a method over a metric not supporting it fails with exit code 104 (Invalid config)"
+	);
+}