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

Add JS function to abort test #2093

Merged
merged 8 commits into from
Dec 17, 2021
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
21 changes: 18 additions & 3 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"go.k6.io/k6/errext"
"go.k6.io/k6/errext/exitcodes"
"go.k6.io/k6/js"
"go.k6.io/k6/js/common"
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/consts"
"go.k6.io/k6/lib/metrics"
Expand Down Expand Up @@ -120,7 +121,7 @@ a commandline interface for interacting with it.`,
builtinMetrics := metrics.RegisterBuiltinMetrics(registry)
initRunner, err := newRunner(logger, src, runType, filesystems, runtimeOptions, builtinMetrics, registry)
if err != nil {
return err
return common.UnwrapGojaInterruptedError(err)
}

logger.Debug("Getting the script options...")
Expand Down Expand Up @@ -250,6 +251,7 @@ a commandline interface for interacting with it.`,
initBar.Modify(pb.WithConstProgress(0, "Init VUs..."))
engineRun, engineWait, err := engine.Init(globalCtx, runCtx)
if err != nil {
err = common.UnwrapGojaInterruptedError(err)
// Add a generic engine exit code if we don't have a more specific one
return errext.WithExitCodeIfNone(err, exitcodes.GenericEngine)
}
Expand All @@ -273,8 +275,18 @@ a commandline interface for interacting with it.`,

// Start the test run
initBar.Modify(pb.WithConstProgress(0, "Starting test..."))
if err := engineRun(); err != nil {
return errext.WithExitCodeIfNone(err, exitcodes.GenericEngine)
var interrupt error
err = engineRun()
if err != nil {
err = common.UnwrapGojaInterruptedError(err)
if common.IsInterruptError(err) {
// Don't return here since we need to work with --linger,
// show the end-of-test summary and exit cleanly.
interrupt = err
}
if !conf.Linger.Bool && interrupt == nil {
return errext.WithExitCodeIfNone(err, exitcodes.GenericEngine)
}
}
runCancel()
logger.Debug("Engine run terminated cleanly")
Expand Down Expand Up @@ -323,6 +335,9 @@ a commandline interface for interacting with it.`,
logger.Debug("Waiting for engine processes to finish...")
engineWait()
logger.Debug("Everything has finished, exiting k6!")
if interrupt != nil {
return interrupt
}
if engine.IsTainted() {
return errext.WithExitCodeIfNone(errors.New("some thresholds have failed"), exitcodes.ThresholdsHaveFailed)
}
Expand Down
91 changes: 91 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,28 @@ package cmd

import (
"bytes"
"context"
"errors"
"io"
"io/ioutil"
"os"
"path"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.k6.io/k6/errext"
"go.k6.io/k6/errext/exitcodes"
"go.k6.io/k6/js/common"
"go.k6.io/k6/lib/fsext"
"go.k6.io/k6/lib/testutils"
)

type mockWriter struct {
Expand Down Expand Up @@ -125,3 +134,85 @@ func TestHandleSummaryResultError(t *testing.T) {
assertEqual(t, "file summary 1", files[filePath1])
assertEqual(t, "file summary 2", files[filePath2])
}

func TestAbortTest(t *testing.T) { //nolint: tparallel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this nolint required 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just add t.Parallel to the ones below? Or just split them in two separate tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, missed this originally.

To answer your first question: yes, it's needed, otherwise tparallel fails with TestAbortTest's subtests should call t.Parallel. And both tparallel and paralleltest are enabled by default... It's confusing, I know 😞 Maybe we should disable tparallel globally?

We can't add t.Parallel() to either test because of several data races... And I prefer having them as subtests in this case. :)

t.Parallel()

testCases := []struct {
testFilename, expLogOutput string
}{
{
testFilename: "abort.js",
},
{
testFilename: "abort_initerr.js",
},
{
testFilename: "abort_initvu.js",
},
{
testFilename: "abort_teardown.js",
expLogOutput: "Calling teardown function after test.abort()",
},
Comment on lines +145 to +156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: for these really small testfiles I kind of feel like moving them to files just adds extra steps for whoever looks at the code. AFAIK the reason for this to be the case for other tests is that the testdata is quite ... big and hard to put in a string in the code, none of this is true for this couple of lines here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but these tests test the runCmd, so I can't see how we could turn the JS into strings... Writing to os.Stdin maybe? 😕

}

for _, tc := range testCases { //nolint: paralleltest
tc := tc
t.Run(tc.testFilename, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

logger := logrus.New()
logger.SetLevel(logrus.InfoLevel)
logger.Out = ioutil.Discard
hook := testutils.SimpleLogrusHook{
HookedLevels: []logrus.Level{logrus.InfoLevel},
}
logger.AddHook(&hook)

cmd := getRunCmd(ctx, logger)
// Redefine the flag to avoid a nil pointer panic on lookup.
cmd.Flags().AddFlag(&pflag.Flag{
Name: "address",
Hidden: true,
})
a, err := filepath.Abs(path.Join("testdata", tc.testFilename))
require.NoError(t, err)
cmd.SetArgs([]string{a})
err = cmd.Execute()
var e errext.HasExitCode
require.ErrorAs(t, err, &e)
assert.Equalf(t, exitcodes.ScriptAborted, e.ExitCode(),
"Status code must be %d", exitcodes.ScriptAborted)
assert.Contains(t, e.Error(), common.AbortTest)

if tc.expLogOutput != "" {
var gotMsg bool
for _, entry := range hook.Drain() {
if strings.Contains(entry.Message, tc.expLogOutput) {
gotMsg = true
break
}
}
assert.True(t, gotMsg)
}
})
}
}

func TestInitErrExitCode(t *testing.T) { //nolint: paralleltest
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
logger := testutils.NewLogger(t)

cmd := getRunCmd(ctx, logger)
a, err := filepath.Abs("testdata/initerr.js")
require.NoError(t, err)
cmd.SetArgs([]string{a})
err = cmd.Execute()
var e errext.HasExitCode
require.ErrorAs(t, err, &e)
assert.Equalf(t, exitcodes.ScriptException, e.ExitCode(),
"Status code must be %d", exitcodes.ScriptException)
assert.Contains(t, err.Error(), "ReferenceError: someUndefinedVar is not defined")
}
5 changes: 5 additions & 0 deletions cmd/testdata/abort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import exec from 'k6/execution';

export default function () {
exec.test.abort();
}
2 changes: 2 additions & 0 deletions cmd/testdata/abort_initerr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import exec from 'k6/execution';
exec.test.abort();
8 changes: 8 additions & 0 deletions cmd/testdata/abort_initvu.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import exec from 'k6/execution';

// This won't fail on initial parsing of the script, but on VU initialization.
if (__VU == 1) {
exec.test.abort();
}

export default function() {}
9 changes: 9 additions & 0 deletions cmd/testdata/abort_teardown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import exec from 'k6/execution';

export default function () {
exec.test.abort();
}

export function teardown() {
console.log('Calling teardown function after test.abort()');
}
1 change: 1 addition & 0 deletions cmd/testdata/initerr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
someUndefinedVar
8 changes: 6 additions & 2 deletions core/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"gopkg.in/guregu/null.v3"

"go.k6.io/k6/errext"
"go.k6.io/k6/js/common"
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/metrics"
"go.k6.io/k6/output"
Expand Down Expand Up @@ -263,9 +264,12 @@ func (e *Engine) startBackgroundProcesses(
if err != nil {
e.logger.WithError(err).Debug("run: execution scheduler returned an error")
var serr errext.Exception
if errors.As(err, &serr) {
switch {
case errors.As(err, &serr):
e.setRunStatus(lib.RunStatusAbortedScriptError)
} else {
case common.IsInterruptError(err):
e.setRunStatus(lib.RunStatusAbortedUser)
default:
e.setRunStatus(lib.RunStatusAbortedSystem)
}
} else {
Expand Down
23 changes: 20 additions & 3 deletions core/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import (
"github.com/sirupsen/logrus"

"go.k6.io/k6/errext"
"go.k6.io/k6/js/common"
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/executor"
"go.k6.io/k6/lib/metrics"
"go.k6.io/k6/stats"
"go.k6.io/k6/ui/pb"
Expand Down Expand Up @@ -346,7 +348,13 @@ func (e *ExecutionScheduler) Run(
executorsCount := len(e.executors)
logger := e.logger.WithField("phase", "local-execution-scheduler-run")
e.initProgress.Modify(pb.WithConstLeft("Run"))
defer e.state.MarkEnded()
var interrupted bool
defer func() {
e.state.MarkEnded()
if interrupted {
e.state.SetExecutionStatus(lib.ExecutionStatusInterrupted)
}
Comment on lines +352 to +356
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we get a comment that we on purpose first hit the ended and then the interrupted state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Not sure, doesn't the code speak for itself?

}()

if e.state.IsPaused() {
logger.Debug("Execution is paused, waiting for resume or interrupt...")
Expand Down Expand Up @@ -386,8 +394,14 @@ func (e *ExecutionScheduler) Run(
// Start all executors at their particular startTime in a separate goroutine...
logger.Debug("Start all executors...")
e.state.SetExecutionStatus(lib.ExecutionStatusRunning)

// We are using this context to allow lib.Executor implementations to cancel
// this context effectively stopping all executions.
//
// This is for addressing test.abort().
execCtx := executor.Context(runSubCtx)
for _, exec := range e.executors {
go e.runExecutor(runSubCtx, runResults, engineOut, exec, builtinMetrics)
go e.runExecutor(execCtx, runResults, engineOut, exec, builtinMetrics)
}

// Wait for all executors to finish
Expand All @@ -414,7 +428,10 @@ func (e *ExecutionScheduler) Run(
return err
}
}

if err := executor.CancelReason(execCtx); err != nil && common.IsInterruptError(err) {
interrupted = true
return err
}
return firstErr
}

Expand Down
1 change: 1 addition & 0 deletions errext/exitcodes/codes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ const (
ExternalAbort errext.ExitCode = 105
CannotStartRESTAPI errext.ExitCode = 106
ScriptException errext.ExitCode = 107
ScriptAborted errext.ExitCode = 108
)
69 changes: 69 additions & 0 deletions js/common/interrupt_error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
*
* k6 - a next-generation load testing tool
* Copyright (C) 2021 Load Impact
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package common

import (
"errors"

"github.com/dop251/goja"
"go.k6.io/k6/errext"
"go.k6.io/k6/errext/exitcodes"
)

// InterruptError is an error that halts engine execution
type InterruptError struct {
Reason string
}

var _ errext.HasExitCode = &InterruptError{}

// Error returns the reason of the interruption.
func (i *InterruptError) Error() string {
return i.Reason
}

// ExitCode returns the status code used when the k6 process exits.
func (i *InterruptError) ExitCode() errext.ExitCode {
return exitcodes.ScriptAborted
}

// AbortTest is the reason emitted when a test script calls test.abort()
const AbortTest = "test aborted"

// IsInterruptError returns true if err is *InterruptError.
func IsInterruptError(err error) bool {
if err == nil {
return false
}
var intErr *InterruptError
return errors.As(err, &intErr)
}

// UnwrapGojaInterruptedError returns the internal error handled by goja.
func UnwrapGojaInterruptedError(err error) error {
var gojaErr *goja.InterruptedError
if errors.As(err, &gojaErr) {
if e, ok := gojaErr.Value().(error); ok {
return e
}
}
return err
}
Loading