Add abortTest() helper function #1920

gernest · 2021-03-22T08:02:38Z

Closes #1001
This adds abortTest() helper function to the k6 module. This function when
called inside a script it will

stop the whole test run and k6 will exit with 107 status code
stops immediately the VU that called it and no more iterations are started
make sure the teardown() is called
the engine run status is 7 (RunStatusAbortedScriptError)

(*goja.Runtime).Interrupt is used for halting script execution and capturing
stack traces for better error message of what is happening with the script.

We introduce InterruptError which is used with (*goja.Runtime).Interrupt to
identify interrupts emitted by abortTest(). This way we use special handling of
this type.

Example script is

import { abortTest, sleep } from 'k6';

export default function () {
    // We abort the test on second iteration
    if (__ITER == 1) {
        abortTest();
    }
    sleep(1);
}

export function teardown() {
    console.log('This function will be called even when we abort script');
}

abortTest() can be called anywhere in the script.

The following script will fail with the error

ERRO[0000] Using abortTest() in the init context is not supported at (...path to the script )init.js:13:43(34)

import {
    abortTest
} from 'k6';

abortTest();

export function setup() {
}

export default function () {
    // ... some test logic ...
    console.log('mayday, mayday');
}

You can customize the reason for abortTest() by passing values to the function

abortTest("Exceeded expectations");

Will emit "Exceeded expectations" on the logs

Missing

Tests
- Check status code is 107 when the abortTest() is called in a script
- Wait for CTRL+C when --linger flag is set
- Check correct default log message for abortTest() without arguments
- Check correct custom log message like abortTest("maybday!")
- Check teardown is called when a script aborts

Closes grafana#1001 This adds abortTest() helper function to the k6 module. This function when called inside a script it will - stop the whole test run and k6 will exit with 107 status code - stops immediately the VU that called it and no more iterations are started - make sure the teardown() is called - the engine run status is 7 (RunStatusAbortedScriptError) `(*goja.Runtime).Interrupt` is used for halting script execution and capturing stack traces for better error message of what is happening with the script. We introduce InterruptError which is used with `(*goja.Runtime).Interrupt` to identify interrupts emitted by abortTest(). This way we use special handling of this type. Example script is ```js import { abortTest, sleep } from 'k6'; export default function () { // We abort the test on second iteration if (__ITER == 1) { abortTest(); } sleep(1); } export function teardown() { console.log('This function will be called even when we abort script'); } ``` abortTest() can be called in both default and setup functions, however you can't use it in the init context The following script will fail with the error ``` ERRO[0000] Using abortTest() in the init context is not supported at (...path to the script )init.js:13:43(34) ``` ```js import { abortTest } from 'k6'; abortTest(); export function setup() { } export default function () { // ... some test logic ... console.log('mayday, mayday'); } ``` You can customize the reason for abortTest() by passing values to the function ```js abortTest("Exceeded expectations"); ``` Will emit `"Exceeded expectations"` on the logs

CLAassistant · 2021-03-22T08:02:44Z

All committers have signed the CLA.

codecov-io · 2021-03-22T08:10:45Z

Codecov Report

Merging #1920 (d7e8deb) into master (3abd529) will increase coverage by 1.78%.
The diff coverage is 86.00%.

❗ Current head d7e8deb differs from pull request most recent head a425d48. Consider uploading reports for the commit a425d48 to get more accurate results

@@            Coverage Diff             @@
##           master    #1920      +/-   ##
==========================================
+ Coverage   71.26%   73.04%   +1.78%     
==========================================
  Files         183      183              
  Lines       14336    14382      +46     
==========================================
+ Hits        10216    10505     +289     
+ Misses       3480     3206     -274     
- Partials      640      671      +31

Flag	Coverage Δ
ubuntu	`73.04% <86.00%> (+1.80%)`	⬆️
windows	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/run.go	`60.33% <55.55%> (+48.36%)`	⬆️
lib/errors/interrupt_error.go	`60.00% <60.00%> (ø)`
lib/executor/helpers.go	`96.03% <94.44%> (-0.35%)`	⬇️
core/engine.go	`88.88% <100.00%> (+3.75%)`	⬆️
core/local/local.go	`81.69% <100.00%> (+5.97%)`	⬆️
js/modules/k6/k6.go	`100.00% <100.00%> (ø)`
js/runner.go	`81.71% <100.00%> (+0.84%)`	⬆️
js/common/initenv.go	`86.66% <0.00%> (-13.34%)`	⬇️
js/initcontext.go	`90.10% <0.00%> (-2.20%)`	⬇️
cmd/ui_windows.go
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3abd529...a425d48. Read the comment docs.

na--

Thanks! I haven't done a full code review yet, I've only skimmed the changes and played around with the resulting binary a bit, but in general, your approach looks good! The only big issue I noticed is that these changes break the --linger functionality.

k6 run --linger script.js should still linger when abortTest() is used, i.e. the k6 process shouldn't exit directly after the test run has stopped. This behavior should remain the same as it was with the old way of aborting the test run (thresholds with abortOnFail: true).

This change makes abortTest() behave like thresholds with `abortOnFail: true` when called with --linger flag

gernest · 2021-03-23T16:20:45Z

@na-- thanks for the quick and good feedback. I had missed the --linger flag, but the docs were informative so I was able to address the flag use case.

When we run scripts that use abortTest() with --linger , basically every thing described on the PR message above stays the same except we don't immediately exit with 107 code .

Winding down of the engine will be observed like with thresholds and waiting for CTRL+C which will then allow us to exit with 107.

Relevant commit c07af18

imiric

This looks very good to me, nice work! I just left some minor comments and nitpicks.

Also there's a check next to "Check teardown is called when a script aborts", but I don't see that test and I agree that it should be added.

cmd/run.go

js/common/interupt_error.go

js/modules/k6/k6.go

lib/executor/helpers.go

mstoykov

Seems good in general 🎉

definitely, need to move the IterruptedError definition out of js/common and probably ... lib/errors, something else ?
not so certain that this isn't an abuse of contexts and that I wouldn't prefer a more direct approach, but looking at the code around this ... I think that will require some pretty big refactoring and probably will make things actually worse than better, so I am fine with it

cmd/run.go

js/common/interupt_error.go

js/modules/k6/k6.go

lib/executor/helpers.go

mstoykov · 2021-03-26T10:36:00Z

lib/executor/helpers.go

+				if handleInterupt(ctx, err) {
+					executionState.AddInterruptedIterations(1)
+					return false
+				}


Looking at the code nothing uses the return value. This seems to not matter as the contexts are getting checked constantly, but it is probably a good idea to figure out if we need to use it somehow

cc @imiric @na--

Yeah, we might make a small PR to refactor this, but it might be needed eventually (e.g. in #1250 or #877) and is so small that it's probably fine as it is.

Co-authored-by: Ivan Mirić <[email protected]>

* master: Fix DOCKER_IMAGE_ID and add GHCR_IMAGE_ID after repo move Drop all the go mod replaces Bump golang version to 1.16 in Dockerfile Bump versions in CI from 1.14->1.15 and from 1.15->1.16 Disable let/const babel plugin and co update goja to latest master

to avoid panics and data races pass empty flag address

gernest · 2021-03-29T15:31:14Z

@na-- @imiric @mstoykov thanks a lot for the amazing feedback, I have addressed all review comments (I think) PTAL

Also there's a check next to "Check teardown is called when a script aborts", but I don't see that test and I agree that it should be added.

Thanks for catching this one, I added the test in b5b8441 (that i improved a bit in later commits, like cancel context .. etc)

na--

LGTM! The only minor nitpick I have is that instead of directly type casting errors, you should probably use the recent errors.Is() and errors.As() stdlib functions.

Other than that, everything looks great! The changes ended up more complex than I expected, but can't think of a way to simplify them without significant refactoring and everything is still understandable, so this is going to be it! 🎉

imiric

LGTM, well done! Just left a tiny nitpick, feel free to disregard.

Also, please rebase on top of the latest master and cleanup the commits a bit before this is merged. Not necessarily squashing to a single commit, but whatever makes sense for the changes to be atomic.

imiric · 2021-04-01T14:42:15Z

cmd/run_test.go

+		cmd.Flags().AddFlag(&pflag.Flag{
+			Name: "address",
+		})


What is this for?

...

Ah, I see, it's to avoid a nil pointer panic in run.go:217. It doesn't seem to be required for this test, but the one below does fail without it. In any case, it would be good to mention this in a comment.

Also, both of these tests are quite noisy when ran with go test -v. It would be good if that could be captured internally and not actually printed to stdout. Though if it requires overriding the global stdout var, then it's probably not worth it 😄

rushby · 2021-06-17T14:20:51Z

Hi all,

Just been looking into this functionality would be great if this can get merged in soon!

na-- · 2021-06-18T07:07:03Z

@rushby, we wanted to merge this for k6 v0.33.0, but we weren't able because it's blocked by some other changes we needed to make in k6 (e.g. #1863) and in our cloud, so it can work both in single instance and in distributed tests. We plan to merge it for k6 v0.34.0, slated to be released sometime in the end of August/early September.

imiric · 2021-10-12T12:36:37Z

Closing this in favor of #2093.

We changed the API slightly but you'll get full credits @gernest. Thanks!

na-- requested changes Mar 22, 2021

View reviewed changes

gernest added 5 commits March 23, 2021 18:05

add godoc comments

9c7fe51

support --linger option

c07af18

This change makes abortTest() behave like thresholds with `abortOnFail: true` when called with --linger flag

fix handleInterupt helper function

cc0993e

fix linting errors

4ba9d24

fix linting error

2953b5b

gernest added 2 commits March 25, 2021 09:19

add TestAbortTest

ffb0186

add test for 107 status code

24410d2

imiric reviewed Mar 26, 2021

View reviewed changes

mstoykov requested changes Mar 26, 2021

View reviewed changes

gernest and others added 16 commits March 28, 2021 18:57

Update cmd/run.go

f2c9911

Co-authored-by: Ivan Mirić <[email protected]>

Update cmd/run.go

22b2438

Co-authored-by: Ivan Mirić <[email protected]>

Update js/common/interupt_error.go

7e11071

Co-authored-by: Ivan Mirić <[email protected]>

Update js/modules/k6/k6.go

4d863dc

Co-authored-by: Ivan Mirić <[email protected]>

fix typo

9250535

accept single argument to abortTest()

48a0ed6

Update lib/executor/helpers.go

54c939d

Co-authored-by: Ivan Mirić <[email protected]>

Update lib/executor/helpers.go

86292de

Co-authored-by: Ivan Mirić <[email protected]>

fix typo

825bb32

add copyright header

a7d66f9

don't export CancelExecutorContext

e58e3eb

fix typo

d89cd56

allow abortTest() to be called in init context

eaa9ea9

move InterruptError to lib/errors

75f8e87

fix linting/build errors

763be7a

check teardown is called after abortTest()

b5b8441

gernest added 8 commits March 28, 2021 22:36

fix lint error

7035c6f

fix lint error

d7e8deb

set address before running teardown

860faa9

fix lint

efc294a

fix address workaround

cc9511e

to avoid panics and data races pass empty flag address

gofumpt

a425d48

gernest changed the title ~~[WIP] Add abortTest() helper function~~ Add abortTest() helper function Mar 29, 2021

gernest marked this pull request as ready for review March 29, 2021 15:27

gernest requested review from imiric, mstoykov and na-- March 29, 2021 15:33

na-- approved these changes Apr 1, 2021

View reviewed changes

imiric approved these changes Apr 1, 2021

View reviewed changes

na-- added this to the v0.33.0 milestone Apr 7, 2021

na-- mentioned this pull request Apr 16, 2021

Print stacktrace on exception in init context or setup/teardown #1971

Merged

na-- modified the milestones: v0.33.0, v0.34.0 Jun 16, 2021

na-- mentioned this pull request Jun 17, 2021

Refactor executor contexts? #2068

Open

na-- mentioned this pull request Jun 18, 2021

Core changes for k6/x/execution JS module #1863

Merged

This was referenced Jul 13, 2021

Add JS function to abort test #2093

Merged

Add test.abort() function grafana/xk6-execution#3

Closed

mstoykov modified the milestones: v0.34.0, v0.35.0 Sep 1, 2021

imiric closed this Oct 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add abortTest() helper function #1920

Add abortTest() helper function #1920

gernest commented Mar 22, 2021 •

edited

Loading

CLAassistant commented Mar 22, 2021 •

edited

Loading

codecov-io commented Mar 22, 2021 •

edited

Loading

na-- left a comment •

edited

Loading

gernest commented Mar 23, 2021

imiric left a comment

mstoykov left a comment

mstoykov Mar 26, 2021

na-- Apr 1, 2021

gernest commented Mar 29, 2021 •

edited

Loading

na-- left a comment

imiric left a comment

imiric Apr 1, 2021

rushby commented Jun 17, 2021

na-- commented Jun 18, 2021

imiric commented Oct 12, 2021

Add abortTest() helper function #1920

Add abortTest() helper function #1920

Conversation

gernest commented Mar 22, 2021 • edited Loading

CLAassistant commented Mar 22, 2021 • edited Loading

codecov-io commented Mar 22, 2021 • edited Loading

Codecov Report

na-- left a comment • edited Loading

Choose a reason for hiding this comment

gernest commented Mar 23, 2021

imiric left a comment

Choose a reason for hiding this comment

mstoykov left a comment

Choose a reason for hiding this comment

mstoykov Mar 26, 2021

Choose a reason for hiding this comment

na-- Apr 1, 2021

Choose a reason for hiding this comment

gernest commented Mar 29, 2021 • edited Loading

na-- left a comment

Choose a reason for hiding this comment

imiric left a comment

Choose a reason for hiding this comment

imiric Apr 1, 2021

Choose a reason for hiding this comment

rushby commented Jun 17, 2021

na-- commented Jun 18, 2021

imiric commented Oct 12, 2021

gernest commented Mar 22, 2021 •

edited

Loading

CLAassistant commented Mar 22, 2021 •

edited

Loading

codecov-io commented Mar 22, 2021 •

edited

Loading

na-- left a comment •

edited

Loading

gernest commented Mar 29, 2021 •

edited

Loading