Add failfast option

[nickrobinson251]

• Close Feature request: Fail-fast option #132
• Other test frameworks have a similar option
  • Go go test has -failfast
  • Rust cargo test has -no-fail-fast
  • Python pytest has --exitfirst
  • Ruby rspec has --fail-fast
  • Javascript jest has bail (https://jestjs.io/docs/configuration#bail-number--boolean)
• Also (TIL) the Test.jl stdlib has failfast since Julia v1.9 (JuliaLang/julia@88def1a)

Since for us "running tests" involves running test-items which themselves can run multiple tests, we can "fail fast" at two levels:

• Stop runtests from running new testitems as soon as one returns as a failure/error (mark that whole run as a failure)
• Stop @testitem from running the tests inside as soon as there is a failure/error (mark that test-item as a failure)

Originally this PR added just a failfast keyword to runtests: runtests(..., failfast=true) stops as soon as any testitem fails.

But i've extended it to add the ability for an @testitem to set failfast=true: @testitem "foo" failfast=true stops on the first test error/failure. This can be set for all test-items by a second new runtests keyword named testitem_failfast, (i.e. this can be used to set the default for all testitems).

This matches how @testitem "foo" timeout=60 corresponds to runtests(...; testitem_timeout=60)

failfast defaults to false. I've set testitem_failfast to default to the same value as given to failfast (if not set explicitly on a @testitem).

So the proposed behaviour is:

• runtests(...) => neither runtests nor individual testitems stop early
• runtests(...; failfast=true) => both stop early
• runtests(...; testitem_failfast=true) => only testitems stop early
• runtests(...; failfast=true, testitem_failfast=false) => only runtests stops early

API question

Would it be simpler to have the separate keywords operate independently?

The downside would be you have to set both to true to get "fail fastest", i.e. to both have individual testitems stop when they hit an error/failure, and to have no new testitems run once one has hit an error/failure you have to run with runtests(...; failfast=true, testitem_failfast=true)

Alternative (keywords operate independently):

• runtests(...;) => neither runtests nor individual testitems stop early
• runtests(...; failfast=true) => only runtests stops early
• runtests(...; testitem_failfast=true) => only testitems stop early
• runtests(...; failfast=true, testitem_failfast=true) => both stop early

Really the question is whether failfast=true should default to turning on both forms of early stopping (as currently proposed by this PR), or if it should mean only runtests fails fast?

I would quite like the seperation of the two... one keyword controls one, the other controls the other... BUT I think in practice it is more ergonomic for users to just pass failfast=true to get the "fastest" failures (hence proposing that) ...i'd love some feedback on this decision!

[Drvi]

I like the feature and I agree with the current API. 👍

I didn't manage to get through all of this today, will continue tomorrow.

[Drvi]

This looks good to me!

It's a bit sad that we can't guarantee actual fast failing when nworkers > 1 since then the current test items scheduled on other test workers might take up to timeout seconds to finish. What if we pretend that the timeout is effectively zero and just kill any other workers to guarantee truly fast failure? The user likely doesn't care about all the test results when they run runtests with failfast... especially if we'd call the kwarg failfastandfurious:)

[Drvi]

Not necessarily for this PR, but I think we should just bundle all these args into a Context struct

[nickrobinson251]

for sure! #186

[nickrobinson251]

It's a bit sad that we can't guarantee actual fast failing when nworkers > 1 since then the current test items scheduled on other test workers might take up to timeout seconds to finish. What if we pretend that the timeout is effectively zero and just kill any other workers to guarantee truly fast failure?

Yeah, i wonder about that too 🤔 It was so long ago that i did this i can't remember why i didn't go kill the other workers... I'll need to look into it. I suspect/hope it was laziness/simplicity (i.e. this implementation is so simple, because we just have next_testitem check for a Bool, so there's no need to coordinate workers)

In fact, i wonder if it's even worse than "might take up to timeout seconds to finish", because of retries? in which case that could be pretty bad, and we'd have to just name this failfastish or failabitfaster

[nickrobinson251]

Buuut, also i've no time to work on this at the minute, and it's a pain to keep rebasing and a bit of a shame not to have it at least in it's current form... so i think i might just update the documentation to call-out that we wait for testitems on others workers to finish in the current implementation, and say this may change in future releases to proactively cancel other running testitems to enable even faster failures, and merge what's here (i.e. let us land that improvement in a follow-up, non-breaking release) -- what do you think?

[Drvi]

Good point about the retries! I wonder if retries could be skipped relatively easily by checking first whether the run has been canceled. But I think it's fine to refine the behavior of this feature in the future 👍