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

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

Merged
merged 2 commits into from
Jan 30, 2023
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
73 changes: 43 additions & 30 deletions cmd/tests/cmd_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
})
}

Expand All @@ -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)
})
}

Expand All @@ -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)
})
}

Expand All @@ -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,
)
Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
5 changes: 0 additions & 5 deletions core/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
51 changes: 0 additions & 51 deletions core/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading