Add failfast option (#133)

* Add failfast

* support ENV var

* Improve logging for failfast mode

* Add tests

* Handle `@testset failfast=true`

* Bump test timeout to avoid unexpected failures

* Add testitem failfast

* Switch to separate `testitem_failfast` keyword

* testitem failfast printing

* Fallback to testset timing if tests errored

* Add testitem failfast tests

* Make testset name more distinctive

* More docs

* Setting `failfast` sets `testitem_failfast` by default

* Remove racy cancellation check and switch to an atomic

* Document current limitation of `failfast` with multiple workers

To make clear this may change in a non-breaking release

* Bump version

* fixup! Document current limitation of `failfast` with multiple workers

Author: nickrobinson251

Project.toml

@@ -1,6 +1,6 @@
 name = "ReTestItems"
 uuid = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
-version = "1.27.0"
+version = "1.28.0"
 
 [deps]
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"

README.md

@@ -95,7 +95,6 @@ Filtering by `name` and `tags` can be combined to run only test-items that match
 julia> runtests("test/Database/"; tags=:regression, name=r"^issue")
 ```
 
-
 #### Running tests in parallel
 
 You can run tests in parallel on multiple worker processes using the `nworkers` keyword.
@@ -117,6 +116,21 @@ threadpools](https://docs.julialang.org/en/v1/manual/multi-threading/#man-thread
 
 Note ReTestItems.jl uses distributed parallelism, not multi-threading, to run test-items in parallel.
 
+#### Stopping tests early
+
+You can set `runtests` to stop on the first test-item failure by passing `failfast=true`.
+
+
+> [!NOTE]
+> Note `failfast` prevents any new test-items starting to run after the first test-item failure, but
+> test-items that were already running on another worker in parallel with the failing test will complete and appear in the test report.
+> Tests that were not run will not appear in the test report.
+>
+> This may be improved in a future version of ReTestItems.jl, so that all test-items running in parallel are stopped on when one test-item fails
+
+If you want individual test-items to stop on their first test failure, but not stop the whole `runtests` early, you can instead pass just `testitem_failfast=true` to `runtests`.
+
+
 ## Writing tests
 
 Tests must be wrapped in a [`@testitem`](https://docs.juliahub.com/General/ReTestItems/stable/autodocs/#ReTestItems.@testitem-Tuple{Any,%20Vararg{Any}}).
@@ -194,6 +208,19 @@ The `skip` keyword allows you to define the condition under which a test needs t
 for example if it can only be run on a certain platform.
 See [filtering tests](#filtering-tests) for controlling which tests run in a particular `runtests` call.
 
+#### Failing early
+
+Each test-item can control whether or not it stops on the first failure using the `failfast` keyword.
+
+```julia
+@testitem "stops on first failure" failfast=true begin
+    @test 1 + 1 == 3
+    @test 2 * 2 == 4
+end
+```
+
+If a test-items set the `failfast` then that value takes precedence over the `testitem_failfast` keyword passed to `runtests`.
+
 #### Post-testitem hook
 
 If there is something that should be checked after every single `@testitem`, then it's possible to pass an expression to `runtests` using the `test_end_expr` keyword.
@@ -255,7 +282,7 @@ runtests("frobble_tests.jl"; nworkers, worker_init_expr)
       using ReTestItems, MyPackage
       runtests(MyPackage)
       ```
-    - Pass to `runtests` any configuration you want your tests to run with, such as `retries`, `testitem_timeout`, `nworkers`, `nworker_threads`, `worker_init_expr`, `test_end_expr`.
+    - Pass to `runtests` any configuration you want your tests to run with, such as `retries`, `failfast`, `testitem_failfast`, `testitem_timeout`, `nworkers`, `nworker_threads`, `worker_init_expr`, `test_end_expr`.
       See the [`runtests`](https://docs.juliahub.com/General/ReTestItems/stable/autodocs/#ReTestItems.runtests) docstring for details.
 
 ---

src/ReTestItems.jl

@@ -12,8 +12,8 @@ function save_current_stdio()
     DEFAULT_LOGGER[] = Base.CoreLogging._global_logstate.logger
 end
 
-# A lock that helps to stagger prints to DEFAULT_STDOUT, used in `print_errors_and_captured_logs`
-# which is called by multiple tasks on the coordinator
+# A lock that helps to stagger prints to DEFAULT_STDOUT, used in functions that print and
+# are called by multiple tasks on the coordinator, e.g. `print_errors_and_captured_logs`
 const LogCaptureLock = ReentrantLock()
 macro loglock(expr)
     return :(@lock LogCaptureLock $(esc(expr)))
@@ -279,15 +279,44 @@ function log_testitem_start(ti::TestItem, ntestitems=0)
     write(DEFAULT_STDOUT[], take!(io.io))
 end
 
-function log_testitem_done(ti::TestItem, ntestitems=0)
+function log_testitem_done(ti::TestItem, ntestitems=0; failedfast::Bool=false)
     io = IOContext(IOBuffer(), :color => get(DEFAULT_STDOUT[], :color, false)::Bool)
     print_state(io, "DONE", ti, ntestitems)
     x = last(ti.stats) # always print stats for most recent run
-    print_time(io; x.elapsedtime, x.bytes, x.gctime, x.allocs, x.compile_time, x.recompile_time)
+    if x == PerfStats() # no `@timed` stats if tests threw, fallback on testset timing
+        ts = last(ti.testsets)
+        if !isnothing(ts.time_end)
+            secs_taken = ts.time_end - ts.time_start
+            _print_scaled_one_dec(io, secs_taken, 1, " secs")
+        end
+    else
+        print_time(io; x.elapsedtime, x.bytes, x.gctime, x.allocs, x.compile_time, x.recompile_time)
+    end
+    failedfast && print(io, " (Failed Fast)")
     println(io)
     write(DEFAULT_STDOUT[], take!(io.io))
 end
 
+# So that the user is warned that we're cancelling the rest of the run.
+# Use loglock here as this is called when multiple tasks are running tests and printing output.
+function print_failfast_cancellation(ti::TestItem)
+    io = IOContext(IOBuffer(), :color => get(DEFAULT_STDOUT[], :color, false)::Bool)
+    printstyled(io, "[ Fail Fast: "; bold=true, color=Base.warn_color())
+    println(io, "Test item $(repr(ti.name)) at $(_file_info(ti)) failed. Cancelling tests.")
+    @loglock write(DEFAULT_STDOUT[], take!(io.io))
+    return nothing
+end
+
+# So that the user is warned that not all tests were run.
+# We don't use loglock here, because this is only called once on the coordinator after all
+# tasks running tests have stopped and we're printing the final test report.
+function print_failfast_summary(t::TestItems)
+    io = DEFAULT_STDOUT[]
+    printstyled(io, "[ Fail Fast: "; bold=true, color=Base.warn_color())
+    println(io, "$(t.count)/$(length(t.testitems)) test items were run.")
+    return nothing
+end
+
 function report_empty_testsets(ti::TestItem, ts::DefaultTestSet)
     empty_testsets = String[]
     _find_empty_testsets!(empty_testsets, ts)

src/log_capture.jl

@@ -121,6 +121,7 @@ struct TestItem
     retries::Int
     timeout::Union{Int,Nothing} # in seconds
     skip::Union{Bool,Expr}
+    failfast::Union{Bool,Nothing}
     file::String
     line::Int
     project_root::String
@@ -132,10 +133,10 @@ struct TestItem
     stats::Vector{PerfStats} # populated when the test item is finished running
     scheduled_for_evaluation::ScheduledForEvaluation # to keep track of whether the test item has been scheduled for evaluation
 end
-function TestItem(number, name, id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code)
+function TestItem(number, name, id, tags, default_imports, setups, retries, timeout, skip, failfast, file, line, project_root, code)
     _id = @something(id, repr(hash(name, hash(relpath(file, project_root)))))
     return TestItem(
-        number, name, _id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code,
+        number, name, _id, tags, default_imports, setups, retries, timeout, skip, failfast, file, line, project_root, code,
         TestSetup[],
         Ref{Int}(0),
         DefaultTestSet[],
@@ -241,6 +242,14 @@ expression that returns a `Bool` to determine if the testitem should be skipped.
 
 The `skip` expression is run in its own module, just like a test-item.
 No code inside a `@testitem` is run when a test-item is skipped.
+
+If a `@testitem` should stop running on the first test failure, then you can set the `failfast` keyword.
+
+    @testitem "stop early" failfast=true begin
+        @test false
+        @test true
+        @test error("oops")
+    end
 """
 macro testitem(nm, exs...)
     default_imports = true
@@ -249,6 +258,7 @@ macro testitem(nm, exs...)
     tags = Symbol[]
     setup = Any[]
     skip = false
+    failfast = nothing
     _id = nothing
     _run = true  # useful for testing `@testitem` itself
     _source = QuoteNode(__source__)
@@ -284,6 +294,9 @@ macro testitem(nm, exs...)
                 skip = ex.args[2]
                 # If the `Expr` doesn't evaluate to a Bool, throws at runtime.
                 @assert skip isa Union{Bool,Expr} "`skip` keyword must be passed a `Bool`"
+            elseif kw == :failfast
+                failfast = ex.args[2]
+                @assert failfast isa Bool "`failfast` keyword must be passed a `Bool`. Got `failfast=$failfast`"
             elseif kw == :_id
                 _id = ex.args[2]
                 # This will always be written to the JUnit XML as a String, require the user
@@ -309,7 +322,7 @@ macro testitem(nm, exs...)
     ti = gensym(:ti)
     esc(quote
         let $ti = $TestItem(
-            $Ref(0), $nm, $_id, $tags, $default_imports, $setup, $retries, $timeout, $skip,
+            $Ref(0), $nm, $_id, $tags, $default_imports, $setup, $retries, $timeout, $skip, $failfast,
             $String($_source.file), $_source.line,
             $gettls(:__RE_TEST_PROJECT__, "."),
             $q,

src/macros.jl

@@ -89,10 +89,19 @@ mutable struct TestItems
     # they are populated once the full graph is done by doing
     # a depth-first traversal of the graph
     testitems::Vector{TestItem} # frozen once flatten_testitems! is called
+    @atomic cancelled::Bool # if true, no more testitems should run
     @atomic count::Int # number of testitems that have been taken for eval so far
 end
 
-TestItems(graph) = TestItems(graph, TestItem[], 0)
+TestItems(graph) = TestItems(graph, TestItem[])
+TestItems(graph, testitems) = TestItems(graph, testitems, false, 0)
+# Prevent any new testitems from being scheduled, and return a `Bool` indicating whether or
+# not the testitems were already cancelled.
+cancel!(t::TestItems) = @atomicswap t.cancelled = true
+# Check whether the testitems have been cancelled.
+# Should _not_ be used to decide whether or not to cancel testitems, instead just call
+# `cancel` and check the return value to know if they had already been cancelled.
+is_cancelled(t::TestItems) = @atomic t.cancelled
 
 ###
 ### record results
@@ -124,9 +133,12 @@ end
 
 function record_results!(file::FileNode, ti::TestItem)
     @debugv 1 "Recording TestItem $(repr(ti.name)) to file $(repr(file.path))"
-    # Always record last try as the final status, so a pass-on-retry is a pass.
-    Test.record(file.testset, last(ti.testsets))
-    junit_record!(file.junit, ti)
+    # If `failfast`, this testitem might never have been run, so nothing to record.
+    if ti.eval_number[] != 0
+        # Always record last try as the final status, so a pass-on-retry is a pass.
+        Test.record(file.testset, last(ti.testsets))
+        junit_record!(file.junit, ti)
+    end
 end
 
 # DirNode and FileNode have `junit=nothing` when no report is needed.
@@ -160,6 +172,7 @@ end
 
 # i is the index of the last test item that was run
 function next_testitem(ti::TestItems, i::Int)
+    is_cancelled(ti) && return nothing
     len = length(ti.testitems)
     n = len
     while n > 0