Remove the unneccessary execution.Scheduler.Init()

There is no need to have 2 separate methods, 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 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_test.go

@@ -978,11 +978,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) {
@@ -997,12 +1004,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)
 	})
 }
 
@@ -1021,12 +1028,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)
 	})
 }
 
@@ -1043,12 +1050,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)
 	})
 }
 
@@ -1065,18 +1072,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,
 	)
@@ -1087,13 +1092,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) {
@@ -1112,14 +1112,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)
@@ -1129,10 +1123,8 @@ 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"`)
-
-	// 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`)
+	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)