From f13246bac4660895781de4b236799b1ad6cc69d6 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 02:46:01 +0200 Subject: [PATCH 01/10] Add WorkerTestSet to avoid loss of context due to TestSetException --- src/ParallelTestRunner.jl | 38 ++++++++++++++++++++++++++++++++++++-- test/runtests.jl | 4 ++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index a5409e0..0d8bceb 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -214,12 +214,34 @@ end # entry point # +struct WorkerTestSet <: Test.AbstractTestSet + name::String + results_lock::ReentrantLock + results::Vector{Any} +end +function WorkerTestSet(desc::AbstractString) + return WorkerTestSet(desc, ReentrantLock(), Vector{Any}()) +end + +function Test.record(ts::WorkerTestSet, res) + @lock ts.results_lock push!(ts.results, res) + return nothing +end + +function Test.finish(ts::WorkerTestSet) + # This testset is just a placeholder, + # so it must be the top-most + @assert Test.get_testset_depth() == 0 + return ts +end + function runtest(::Type{TestRecord}, f, name, init_code, color) function inner() # generate a temporary module to execute the tests in mod = @eval(Main, module $(gensym(name)) end) @eval(mod, import ParallelTestRunner: Test, Random) @eval(mod, using .Test, .Random) + @eval(mod, import ParallelTestRunner: WorkerTestSet) Core.eval(mod, init_code) @@ -230,10 +252,11 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) mktemp() do path, io stats = redirect_stdio(stdout=io, stderr=io) do @timed try - @testset $name begin + @testset WorkerTestSet $name begin $f end catch err + # TODO: Should never receive a TestSetException here isa(err, Test.TestSetException) || rethrow() # return the error to package it into a TestRecord @@ -722,6 +745,8 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T if record.value isa Exception print_test_failed(record, wrkr, test_name, io_ctx) else + # TODO: Handle WorkerTestSet to print + # failed at print_test_finished(record, wrkr, test_name, io_ctx) end @@ -934,10 +959,18 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T # decode or fake a testset if result isa AbstractTestRecord - if result.value isa Test.AbstractTestSet + if result.value isa WorkerTestSet + testset = create_testset(testname; start, stop) + historical_durations[testname] = stop - start + for res in result.value.results + Test.record(testset, res) + end + elseif result.value isa Test.AbstractTestSet + @assert false testset = result.value historical_durations[testname] = stop - start else + @assert false # TODO: improve the Test stdlib to keep track of the exact failure # instead of flattening into an exception without provenance @assert result.value isa Test.TestSetException @@ -959,6 +992,7 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T # Record this testset as Errored. @assert result isa Exception testset = create_testset(testname; start, stop) + # TODO: Send backtrace with Exception Test.record(testset, Test.Error(:nontest_error, testname, nothing, Base.ExceptionStack(NamedTuple[(;exception = result, backtrace = [])]), LineNumberNode(1))) end diff --git a/test/runtests.jl b/test/runtests.jl index 4dcc0b4..8b194ea 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -90,7 +90,7 @@ end str = String(take!(io)) @test contains(str, r"basic .+ started at") - @test contains(str, r"failing test .+ failed at") + @test_broken contains(str, r"failing test .+ failed at") @test contains(str, "$(basename(@__FILE__)):$error_line") @test contains(str, "FAILURE") @test contains(str, "Test Failed") @@ -112,7 +112,7 @@ end str = String(take!(io)) @test contains(str, r"basic .+ started at") - @test contains(str, r"throwing test .+ failed at") + @test_broken contains(str, r"throwing test .+ failed at") @test contains(str, "$(basename(@__FILE__)):$error_line") @test contains(str, "FAILURE") @test contains(str, "Error During Test") From 0a83dd81ab5c8520b876d6f289ef95eaa0b8f9dd Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 14:32:06 +0200 Subject: [PATCH 02/10] Handle unnesting --- src/ParallelTestRunner.jl | 7 +++++-- test/basic.jl | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 0d8bceb..ac359fb 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -242,6 +242,7 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) @eval(mod, import ParallelTestRunner: Test, Random) @eval(mod, using .Test, .Random) @eval(mod, import ParallelTestRunner: WorkerTestSet) + @eval(mod, import Test: .DefaultTestSet) Core.eval(mod, init_code) @@ -252,8 +253,10 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) mktemp() do path, io stats = redirect_stdio(stdout=io, stderr=io) do @timed try - @testset WorkerTestSet $name begin - $f + @testset WorkerTestSet "placeholder" begin + @testset DefaultTestSet $name begin + $f + end end catch err # TODO: Should never receive a TestSetException here diff --git a/test/basic.jl b/test/basic.jl index 8a617b3..20806bf 100644 --- a/test/basic.jl +++ b/test/basic.jl @@ -1 +1,5 @@ @test true + +@testset "inner" begin + @test true +end From 9dae7c34599f17a5335d8b10ee467c3705cb9d0c Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 14:49:43 +0200 Subject: [PATCH 03/10] fixup! Handle unnesting --- src/ParallelTestRunner.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index ac359fb..8ecb727 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -242,7 +242,7 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) @eval(mod, import ParallelTestRunner: Test, Random) @eval(mod, using .Test, .Random) @eval(mod, import ParallelTestRunner: WorkerTestSet) - @eval(mod, import Test: .DefaultTestSet) + @eval(mod, import Test: DefaultTestSet) Core.eval(mod, init_code) From ad0ea5629124757e2031fd90959ef421012a1103 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 14:52:03 +0200 Subject: [PATCH 04/10] turn WorkerTestSet into a singleton container --- src/ParallelTestRunner.jl | 42 +++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 8ecb727..76f1ebd 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -13,6 +13,16 @@ import Test import Random import IOCapture +function anynonpass(ts::Test.AbstractTestSet) + @static if VERSION >= v"1.13.0-DEV.1037" + return Test.anynonpass(ts) + else + Test.get_test_counts(ts) + return ts.anynonpass + end +end + + #Always set the max rss so that if tests add large global variables (which they do) we don't make the GC's life too hard if Sys.WORD_SIZE == 64 const JULIA_TEST_MAXRSS_MB = 3800 @@ -214,17 +224,18 @@ end # entry point # -struct WorkerTestSet <: Test.AbstractTestSet - name::String - results_lock::ReentrantLock - results::Vector{Any} -end -function WorkerTestSet(desc::AbstractString) - return WorkerTestSet(desc, ReentrantLock(), Vector{Any}()) +mutable struct WorkerTestSet <: Test.AbstractTestSet + const name::String + wrapped_ts::Test.DefaultTestSet + function WorkerTestSet(name::AbstractString) + new(name) + end end function Test.record(ts::WorkerTestSet, res) - @lock ts.results_lock push!(ts.results, res) + @assert res isa Test.DefaultTestSet + @assert !isdefined(ts, :wrapped_ts) + ts.wrapped_ts = res return nothing end @@ -232,9 +243,12 @@ function Test.finish(ts::WorkerTestSet) # This testset is just a placeholder, # so it must be the top-most @assert Test.get_testset_depth() == 0 + @assert isdefined(ts, :wrapped_ts) return ts end +anynonpass(ts::WorkerTestSet) = anynonpass(ts.wrapped_ts) + function runtest(::Type{TestRecord}, f, name, init_code, color) function inner() # generate a temporary module to execute the tests in @@ -748,9 +762,12 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T if record.value isa Exception print_test_failed(record, wrkr, test_name, io_ctx) else - # TODO: Handle WorkerTestSet to print - # failed at - print_test_finished(record, wrkr, test_name, io_ctx) + ts = record.value::WorkerTestSet + if anynonpass(ts) + print_test_failed(record, wrkr, test_name, io_ctx) + else + print_test_finished(record, wrkr, test_name, io_ctx) + end end elseif msg_type == :crashed @@ -1040,8 +1057,7 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T end print(io_ctx.stdout, c.output) end - if (VERSION >= v"1.13.0-DEV.1037" && !Test.anynonpass(o_ts)) || - (VERSION < v"1.13.0-DEV.1037" && !o_ts.anynonpass) + if !anynonpass(o_ts) println(io_ctx.stdout, " \033[32;1mSUCCESS\033[0m") else println(io_ctx.stderr, " \033[31;1mFAILURE\033[0m\n") From 8e8da15695c2c2a6da935eedcff9361dcd0f0f5c Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 14:56:50 +0200 Subject: [PATCH 05/10] cleanup testset handling --- src/ParallelTestRunner.jl | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 76f1ebd..25864fd 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -977,34 +977,11 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T for (testname, result, start, stop) in results push!(completed_tests, testname) - # decode or fake a testset + # decode testset if result isa AbstractTestRecord - if result.value isa WorkerTestSet - testset = create_testset(testname; start, stop) - historical_durations[testname] = stop - start - for res in result.value.results - Test.record(testset, res) - end - elseif result.value isa Test.AbstractTestSet - @assert false - testset = result.value - historical_durations[testname] = stop - start - else - @assert false - # TODO: improve the Test stdlib to keep track of the exact failure - # instead of flattening into an exception without provenance - @assert result.value isa Test.TestSetException - testset = create_testset(testname; start, stop) - for i in 1:result.value.pass - Test.record(testset, Test.Pass(:test, nothing, nothing, nothing, LineNumberNode(@__LINE__, @__FILE__))) - end - for i in 1:result.value.broken - Test.record(testset, Test.Broken(:test, nothing)) - end - for t in result.value.errors_and_fails - Test.record(testset, t) - end - end + @assert result.value isa WorkerTestSet + testset = result.value.wrapped_ts + historical_durations[testname] = stop - start else # If this test raised an exception that is not a remote testset # exception, that means the test runner itself had some problem, so we From daa0ce5907d4785312b22922daeb126293d68877 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 14:57:26 +0200 Subject: [PATCH 06/10] tests are no longer broken --- test/runtests.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 8b194ea..4dcc0b4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -90,7 +90,7 @@ end str = String(take!(io)) @test contains(str, r"basic .+ started at") - @test_broken contains(str, r"failing test .+ failed at") + @test contains(str, r"failing test .+ failed at") @test contains(str, "$(basename(@__FILE__)):$error_line") @test contains(str, "FAILURE") @test contains(str, "Test Failed") @@ -112,7 +112,7 @@ end str = String(take!(io)) @test contains(str, r"basic .+ started at") - @test_broken contains(str, r"throwing test .+ failed at") + @test contains(str, r"throwing test .+ failed at") @test contains(str, "$(basename(@__FILE__)):$error_line") @test contains(str, "FAILURE") @test contains(str, "Error During Test") From 3558743d6ec2993a9c060b7e06822b2cb3a9ada8 Mon Sep 17 00:00:00 2001 From: Tim Besard Date: Fri, 17 Oct 2025 15:34:34 +0200 Subject: [PATCH 07/10] Add test. --- test/basic.jl | 4 ---- test/runtests.jl | 30 ++++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/test/basic.jl b/test/basic.jl index 20806bf..8a617b3 100644 --- a/test/basic.jl +++ b/test/basic.jl @@ -1,5 +1 @@ @test true - -@testset "inner" begin - @test true -end diff --git a/test/runtests.jl b/test/runtests.jl index 4dcc0b4..d5cabd8 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -97,6 +97,36 @@ end @test contains(str, "1 == 2") end +@testset "nested failure" begin + custom_tests = Dict( + "nested" => quote + @test true + @testset "foo" begin + @test true + @testset "bar" begin + @test false + end + end + end + ) + error_line = @__LINE__() - 5 + + io = IOBuffer() + @test_throws Test.FallbackTestSetException("Test run finished with errors") begin + runtests(ParallelTestRunner, ["--verbose"]; custom_tests, stdout=io, stderr=io) + end + + str = String(take!(io)) + @test contains(str, r"nested .+ started at") + @test contains(str, r"nested .+ failed at") + @test contains(str, r"nested .+ \| .+ 2 .+ 1 .+ 3") + @test contains(str, r"foo .+ \| .+ 1 .+ 1 .+ 2") + @test contains(str, r"bar .+ \| .+ 1 .+ 1") + @test contains(str, "FAILURE") + @test contains(str, "Error in testset bar") + @test contains(str, "$(basename(@__FILE__)):$error_line") +end + @testset "throwing test" begin custom_tests = Dict( "throwing test" => quote From b3ee3a3a477c27f137d26ae89da6889a204d960c Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 16:37:48 +0200 Subject: [PATCH 08/10] Cleanup Co-authored-by: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> --- src/ParallelTestRunner.jl | 57 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 25864fd..10aa075 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -12,6 +12,7 @@ using Serialization import Test import Random import IOCapture +import Test: DefaultTestSet function anynonpass(ts::Test.AbstractTestSet) @static if VERSION >= v"1.13.0-DEV.1037" @@ -22,7 +23,6 @@ function anynonpass(ts::Test.AbstractTestSet) end end - #Always set the max rss so that if tests add large global variables (which they do) we don't make the GC's life too hard if Sys.WORD_SIZE == 64 const JULIA_TEST_MAXRSS_MB = 3800 @@ -78,7 +78,7 @@ end abstract type AbstractTestRecord end struct TestRecord <: AbstractTestRecord - value::Any # AbstractTestSet or TestSetException + value::DefaultTestSet output::String # captured stdout/stderr # stats @@ -92,6 +92,10 @@ function memory_usage(rec::TestRecord) return rec.rss end +function Base.getindex(rec::TestRecord) + return rec.value +end + # # overridable I/O context for pretty-printing @@ -223,7 +227,14 @@ end # # entry point # +""" + WorkerTestSet +A test set wrapper used internally by worker processes. +`Base.DefaultTestSet` detects when it is the top-most and throws +a `TestSetException` containing very little information. By inserting this +wrapper as the top-most test set, we can capture the full results. +""" mutable struct WorkerTestSet <: Test.AbstractTestSet const name::String wrapped_ts::Test.DefaultTestSet @@ -240,21 +251,20 @@ function Test.record(ts::WorkerTestSet, res) end function Test.finish(ts::WorkerTestSet) - # This testset is just a placeholder, - # so it must be the top-most + # This testset is just a placeholder so it must be the top-most @assert Test.get_testset_depth() == 0 @assert isdefined(ts, :wrapped_ts) - return ts + # Return the wrapped_ts so that we don't need to handle WorkerTestSet anywhere else + return ts.wrapped_ts end -anynonpass(ts::WorkerTestSet) = anynonpass(ts.wrapped_ts) - function runtest(::Type{TestRecord}, f, name, init_code, color) function inner() # generate a temporary module to execute the tests in mod = @eval(Main, module $(gensym(name)) end) @eval(mod, import ParallelTestRunner: Test, Random) @eval(mod, using .Test, .Random) + # Both bindings must be imported since `@testset` can't handle fully-qualified names. @eval(mod, import ParallelTestRunner: WorkerTestSet) @eval(mod, import Test: DefaultTestSet) @@ -266,18 +276,13 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) mktemp() do path, io stats = redirect_stdio(stdout=io, stderr=io) do - @timed try - @testset WorkerTestSet "placeholder" begin - @testset DefaultTestSet $name begin - $f - end + # @testset CustomTestRecord switches the all lower-level testset to our custom testset, + # so we need to have two layers here such that the user-defined testsets are using `DefaultTestSet`. + # This also guarantees our invariant about `WorkerTestSet` containing a single `DefaultTestSet`. + @timed @testset WorkerTestSet "placeholder" begin + @testset DefaultTestSet $name begin + $f end - catch err - # TODO: Should never receive a TestSetException here - isa(err, Test.TestSetException) || rethrow() - - # return the error to package it into a TestRecord - err end end close(io) @@ -759,15 +764,10 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T test_name, wrkr, record = msg[2], msg[3], msg[4] clear_status() - if record.value isa Exception + if anynonpass(record[]) print_test_failed(record, wrkr, test_name, io_ctx) else - ts = record.value::WorkerTestSet - if anynonpass(ts) - print_test_failed(record, wrkr, test_name, io_ctx) - else - print_test_finished(record, wrkr, test_name, io_ctx) - end + print_test_finished(record, wrkr, test_name, io_ctx) end elseif msg_type == :crashed @@ -859,6 +859,7 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T Malt.stop(wrkr) end else + # One of Malt.TerminatedWorkerException, Malt.RemoteException, or ErrorException @assert result isa Exception put!(printer_channel, (:crashed, test, worker_id(wrkr))) if do_quickfail @@ -977,19 +978,17 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T for (testname, result, start, stop) in results push!(completed_tests, testname) - # decode testset if result isa AbstractTestRecord - @assert result.value isa WorkerTestSet - testset = result.value.wrapped_ts + testset = result[]::DefaultTestSet historical_durations[testname] = stop - start else # If this test raised an exception that is not a remote testset # exception, that means the test runner itself had some problem, so we # may have hit a segfault, deserialization errors or something similar. # Record this testset as Errored. + # One of Malt.TerminatedWorkerException, Malt.RemoteException, or ErrorException @assert result isa Exception testset = create_testset(testname; start, stop) - # TODO: Send backtrace with Exception Test.record(testset, Test.Error(:nontest_error, testname, nothing, Base.ExceptionStack(NamedTuple[(;exception = result, backtrace = [])]), LineNumberNode(1))) end From 213ed13e025631a7fa6bb48645038b59e069a947 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 16:43:52 +0200 Subject: [PATCH 09/10] fixup! Cleanup --- src/ParallelTestRunner.jl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 10aa075..8125c84 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -982,9 +982,8 @@ function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = T testset = result[]::DefaultTestSet historical_durations[testname] = stop - start else - # If this test raised an exception that is not a remote testset - # exception, that means the test runner itself had some problem, so we - # may have hit a segfault, deserialization errors or something similar. + # If this test raised an exception that means the test runner itself had some problem, + # so we may have hit a segfault, deserialization errors or something similar. # Record this testset as Errored. # One of Malt.TerminatedWorkerException, Malt.RemoteException, or ErrorException @assert result isa Exception From ff8d7bf1e95d5be12ffac7389b2425415a71920c Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 17 Oct 2025 16:48:34 +0200 Subject: [PATCH 10/10] Update src/ParallelTestRunner.jl Co-authored-by: Christian Guinard <28689358+christiangnrd@users.noreply.github.com> --- src/ParallelTestRunner.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ParallelTestRunner.jl b/src/ParallelTestRunner.jl index 8125c84..dc0d4a2 100644 --- a/src/ParallelTestRunner.jl +++ b/src/ParallelTestRunner.jl @@ -264,7 +264,7 @@ function runtest(::Type{TestRecord}, f, name, init_code, color) mod = @eval(Main, module $(gensym(name)) end) @eval(mod, import ParallelTestRunner: Test, Random) @eval(mod, using .Test, .Random) - # Both bindings must be imported since `@testset` can't handle fully-qualified names. + # Both bindings must be imported since `@testset` can't handle fully-qualified names when VERSION < v"1.11.0-DEV.1518". @eval(mod, import ParallelTestRunner: WorkerTestSet) @eval(mod, import Test: DefaultTestSet)