Remove the unnecessary execution.Scheduler.Init() (#2885)

This is the first commit from #2815, plus an extra test that checks the exit code when aborting a `k6 run --paused` test with Ctrl+C. 

Unfortunately, it turned out that 19dd505 from #2813 introduced a regression. Interrupting the k6 process with a signal should result in a `105` exit code (`ExternalAbort`), but with that commit k6 exited with `103` if k6 was `--paused`. The good news is that the fix was already done in the first commit of #2815, which was entirely self-sufficient and can be moved to this separate PR. As a bonus, this makes reviewing everything even easier... 

So, some details about the commit itself... In short, there is no need to have 2 separate `Run()` and `Init()` methods in the `execution.Scheduler`. Instead, Run() can start the VU initialization itself. As a bonus, this immediately makes the error handling around init errors much more in line with other error handling, allowing us to resolve the bug, but also to respect `--linger` and to try and execute `handleSummary()` if there were problems. Except the first init that is used to get the exported options, of course, that one is still special.

cmd/tests/cmd_run_test.go

@@ -982,11 +982,18 @@ func TestAbortedByTestAbortInNonFirstInitCode(t *testing.T) {
 
 		export default function () {};
 
-		// Should not be called, since error is in the init context
 		export function handleSummary() { return {stdout: '\n\n\nbogus summary\n\n\n'};}
 	`
 
-	testAbortedByScriptTestAbort(t, false, script, runTestWithNoLinger)
+	t.Run("noLinger", func(t *testing.T) {
+		t.Parallel()
+		testAbortedByScriptTestAbort(t, script, runTestWithNoLinger)
+	})
+
+	t.Run("withLinger", func(t *testing.T) {
+		t.Parallel()
+		testAbortedByScriptTestAbort(t, script, runTestWithLinger)
+	})
 }
 
 func TestAbortedByScriptAbortInVUCode(t *testing.T) {
@@ -1001,12 +1008,12 @@ func TestAbortedByScriptAbortInVUCode(t *testing.T) {
 
 	t.Run("noLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithNoLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithNoLinger)
 	})
 
 	t.Run("withLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithLinger)
 	})
 }
 
@@ -1025,12 +1032,12 @@ func TestAbortedByScriptAbortInVUCodeInGroup(t *testing.T) {
 
 	t.Run("noLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithNoLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithNoLinger)
 	})
 
 	t.Run("withLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithLinger)
 	})
 }
 
@@ -1047,12 +1054,12 @@ func TestAbortedByScriptAbortInSetup(t *testing.T) {
 
 	t.Run("noLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithNoLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithNoLinger)
 	})
 
 	t.Run("withLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithLinger)
 	})
 }
 
@@ -1069,18 +1076,16 @@ func TestAbortedByScriptAbortInTeardown(t *testing.T) {
 
 	t.Run("noLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithNoLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithNoLinger)
 	})
 
 	t.Run("withLinger", func(t *testing.T) {
 		t.Parallel()
-		testAbortedByScriptTestAbort(t, true, script, runTestWithLinger)
+		testAbortedByScriptTestAbort(t, script, runTestWithLinger)
 	})
 }
 
-func testAbortedByScriptTestAbort(
-	t *testing.T, shouldHaveMetrics bool, script string, runTest func(*testing.T, *GlobalTestState),
-) *GlobalTestState { //nolint:unparam
+func testAbortedByScriptTestAbort(t *testing.T, script string, runTest func(*testing.T, *GlobalTestState)) {
 	ts := getSimpleCloudOutputTestState(
 		t, script, nil, cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ScriptAborted,
 	)
@@ -1093,13 +1098,8 @@ func testAbortedByScriptTestAbort(
 	assert.Contains(t, stdout, "test aborted: foo")
 	assert.Contains(t, stdout, `level=debug msg="Sending test finished" output=cloud ref=111 run_status=5 tainted=false`)
 	assert.Contains(t, stdout, `level=debug msg="Metrics emission of VUs and VUsMax metrics stopped"`)
-	if shouldHaveMetrics {
-		assert.Contains(t, stdout, `level=debug msg="Metrics processing finished!"`)
-		assert.Contains(t, stdout, "bogus summary")
-	} else {
-		assert.NotContains(t, stdout, "bogus summary")
-	}
-	return ts
+	assert.Contains(t, stdout, `level=debug msg="Metrics processing finished!"`)
+	assert.Contains(t, stdout, "bogus summary")
 }
 
 func TestAbortedByInterruptDuringVUInit(t *testing.T) {
@@ -1118,14 +1118,8 @@ func TestAbortedByInterruptDuringVUInit(t *testing.T) {
 
 		export default function () {};
 	`
-
-	// TODO: fix this to exect lib.RunStatusAbortedUser and
-	// exitcodes.ExternalAbort
-	//
-	// This is testing the current behavior, which is expected, but it's not
-	// actually the desired one! See https://github.com/grafana/k6/issues/2804
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, cloudapi.RunStatusAbortedSystem, cloudapi.ResultStatusPassed, exitcodes.GenericEngine,
+		t, script, nil, cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ExternalAbort,
 	)
 	asyncWaitForStdoutAndStopTestWithInterruptSignal(t, ts, 15, time.Second, "VU init sleeping for a while")
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
@@ -1135,10 +1129,29 @@ func TestAbortedByInterruptDuringVUInit(t *testing.T) {
 
 	assert.Contains(t, stdOut, `level=debug msg="Stopping k6 in response to signal..." sig=interrupt`)
 	assert.Contains(t, stdOut, `level=debug msg="Metrics emission of VUs and VUsMax metrics stopped"`)
+	assert.Contains(t, stdOut, `level=debug msg="Sending test finished" output=cloud ref=111 run_status=5 tainted=false`)
+	assert.Contains(t, stdOut, `level=error msg="test run was aborted because k6 received a 'interrupt' signal"`)
+}
 
-	// TODO: same as above, fix expected error message and run_status to 5
-	assert.Contains(t, stdOut, `level=debug msg="Sending test finished" output=cloud ref=111 run_status=6 tainted=false`)
-	assert.Contains(t, stdOut, `level=error msg="context canceled`)
+func TestAbortedByInterruptWhenPaused(t *testing.T) {
+	t.Parallel()
+	script := `export default function () {};`
+	ts := getSimpleCloudOutputTestState(
+		t, script, []string{"-v", "--log-output=stdout", "--paused"},
+		cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ExternalAbort,
+	)
+	asyncWaitForStdoutAndStopTestWithInterruptSignal(
+		t, ts, 10, time.Second, "Execution is paused, waiting for resume or interrupt...",
+	)
+	cmd.ExecuteWithGlobalState(ts.GlobalState)
+
+	stdOut := ts.Stdout.String()
+	t.Log(stdOut)
+
+	assert.Contains(t, stdOut, `level=debug msg="Stopping k6 in response to signal..." sig=interrupt`)
+	assert.Contains(t, stdOut, `level=debug msg="Metrics emission of VUs and VUsMax metrics stopped"`)
+	assert.Contains(t, stdOut, `level=debug msg="Sending test finished" output=cloud ref=111 run_status=5 tainted=false`)
+	assert.Contains(t, stdOut, `level=error msg="test run was aborted because k6 received a 'interrupt' signal"`)
 }
 
 func TestAbortedByScriptInitError(t *testing.T) {

core/engine.go

@@ -95,11 +95,6 @@ func NewEngine(testState *lib.TestRunState, ex *execution.Scheduler, outputs []o
 //   - The second returned lambda can be used to wait for that process to finish.
 func (e *Engine) Init(globalCtx, runCtx context.Context) (run func() error, wait func(), err error) {
 	e.logger.Debug("Initialization starting...")
-	// TODO: if we ever need metrics processing in the init context, we can move
-	// this below the other components... or even start them concurrently?
-	if err := e.ExecutionScheduler.Init(runCtx, e.Samples); err != nil {
-		return nil, nil, err
-	}
 
 	// TODO: move all of this in a separate struct? see main TODO above
 	processMetricsAfterRun := make(chan struct{})

core/engine_test.go

@@ -893,57 +893,6 @@ func TestSetupException(t *testing.T) {
 	}
 }
 
-// TODO: delete when implementing https://github.com/grafana/k6/issues/1889, the
-// test functionality was duplicated in cmd/integration_test.go
-func TestVuInitException(t *testing.T) {
-	t.Parallel()
-
-	script := []byte(`
-		export let options = {
-			vus: 3,
-			iterations: 5,
-		};
-
-		export default function() {};
-
-		if (__VU == 2) {
-			throw new Error('oops in ' + __VU);
-		}
-	`)
-
-	piState := getTestPreInitState(t)
-	runner, err := js.New(
-		piState,
-		&loader.SourceData{URL: &url.URL{Scheme: "file", Path: "/script.js"}, Data: script},
-		nil,
-	)
-	require.NoError(t, err)
-
-	opts, err := executor.DeriveScenariosFromShortcuts(runner.GetOptions(), nil)
-	require.NoError(t, err)
-
-	testState := getTestRunState(t, piState, opts, runner)
-
-	execScheduler, err := execution.NewScheduler(testState)
-	require.NoError(t, err)
-	engine, err := NewEngine(testState, execScheduler, nil)
-	require.NoError(t, err)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	_, _, err = engine.Init(ctx, ctx) // no need for 2 different contexts
-
-	require.Error(t, err)
-
-	var exception errext.Exception
-	require.ErrorAs(t, err, &exception)
-	assert.Equal(t, "Error: oops in 2\n\tat file:///script.js:10:9(29)\n", err.Error())
-
-	var errWithHint errext.HasHint
-	require.ErrorAs(t, err, &errWithHint)
-	assert.Equal(t, "error while initializing VU #2 (script exception)", errWithHint.Hint())
-}
-
 func TestEmittedMetricsWhenScalingDown(t *testing.T) {
 	t.Parallel()
 	tb := httpmultibin.NewHTTPMultiBin(t)