diff --git a/Project.toml b/Project.toml index 0a1f4d9..8bc06b7 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "TestShards" uuid = "acceef1d-f5e0-4fe4-a546-818dc56ce7b2" -version = "0.3.41" +version = "0.3.42" authors = ["sota shimozono "] [deps] diff --git a/src/provider.jl b/src/provider.jl index 2e4e109..336fa6e 100644 --- a/src/provider.jl +++ b/src/provider.jl @@ -122,10 +122,21 @@ _duration(ts) = 0.0 end +# Julia 1.13 gave `DefaultTestSet` a `results_lock` and takes it on every `record`. Take it on +# the read too, and copy rather than iterate under it, since `_section` recurses. A unit that +# left a task running — which `@shard` forbids — would otherwise have this walk a vector being +# pushed to. +_results_snapshot(ts::Test.DefaultTestSet) = + if hasfield(Test.DefaultTestSet, :results_lock) + @lock ts.results_lock copy(ts.results) + else + ts.results + end + function _section(ctx::ShardContext, ts::Test.DefaultTestSet) npass = nfail = nerror = nbroken = 0 kids = Section[] - for r in ts.results + for r in _results_snapshot(ts) if r isa Test.DefaultTestSet s = _section(ctx, r) push!(kids, s) diff --git a/src/run.jl b/src/run.jl index 562caf5..d50ed7b 100644 --- a/src/run.jl +++ b/src/run.jl @@ -31,6 +31,47 @@ function _key(ctx::ShardContext, path::AbstractString) return replace(relpath(abspath(path), ctx.root), '\\' => '/') end +# Record a unit's escaping error on its testset, and say whether that worked. +# +# `_run` catches so the shard survives a unit that throws. Recording must not undo that: a +# provider's testset type with no `Test.record` for `Test.Error` makes the record call itself +# throw, and before this that MethodError escaped `_run` — the unit went unrecorded, every unit +# after it never ran, and the failure surfaced as a TestShards internal rather than as the unit +# that could not load. +# +# Tested by doing it, not by `hasmethod`: a catch-all `record(ts, ::Any)` that throws for +# `Test.Error` satisfies `hasmethod` and still fails here. +function _record_unit_error(ts, key, err) + try + Test.record( + ts, + # XXX: `Base.catch_stack` is internal; `Base.current_exceptions()` is + # the public spelling — see `docs/src/testset-internals.md`. + Test.Error( + :nontest_error, Expr(:tuple), err, Base.catch_stack(), LineNumberNode(0) + ), + ) + return true + catch record_err + @error "TestShards: $(key) threw, and its testset could not record the error. The \ + unit is counted as errored and the shard continues." unit_error = err record_err + return false + end +end + +function _plus_one_error(s::Section) + return Section( + s.name, + s.duration, + s.npass, + s.nfail, + s.nerror + 1, + s.nbroken, + s.evidence, + s.sections, + ) +end + """ _run(ctx, key, body) @@ -50,6 +91,7 @@ function _run(ctx::ShardContext, key::AbstractString, body) ts = _unit_testset(key) t0 = time() + recorded = true _with_testset(ts) do try body() @@ -57,14 +99,7 @@ function _run(ctx::ShardContext, key::AbstractString, body) # An error escaping the unit (a load error, say) is recorded as the unit's error # rather than aborting the shard, so the remaining units still run and still get # recorded. - Test.record( - ts, - # XXX: `Base.catch_stack` is internal; `Base.current_exceptions()` is - # the public spelling — see `docs/src/testset-internals.md`. - Test.Error( - :nontest_error, Expr(:tuple), err, Base.catch_stack(), LineNumberNode(0) - ), - ) + recorded = _record_unit_error(ts, key, err) end end # A tool that attaches a finished testset to its parent can only do it now that the parent is @@ -72,6 +107,9 @@ function _run(ctx::ShardContext, key::AbstractString, body) _unit_close(ts) dt = time() - t0 sec = unit_fold(ctx, ts) + # The unit failed; its testset just could not say so. Count it anyway, or the shard + # reports green for a unit that threw. + recorded || (sec = _plus_one_error(sec)) push!( ctx.records, UnitRecord( diff --git a/test/core/helpers.jl b/test/core/helpers.jl index ee0bb7b..42bca86 100644 --- a/test/core/helpers.jl +++ b/test/core/helpers.jl @@ -9,7 +9,7 @@ using TestShards export make_suite, run_suite, unit_keys export shared_suite, whole_units, partition_check, shard_runs_clean export bare_context, shard_window, lcov_trace -export StubSet, stub_fold, with_provider +export StubSet, PartialSet, stub_fold, partial_fold, with_provider const PROJ = dirname(Base.active_project()) @@ -223,6 +223,21 @@ mutable struct StubSet <: Test.AbstractTestSet end StubSet(desc::AbstractString) = StubSet(String(desc), 0, 0, 0, 0, false) +""" +A provider testset that records `Pass`/`Fail`/`Broken` but NOT `Error` — the shape a provider +author naturally ends up with, since only a THROWING unit reaches the error branch. +""" +mutable struct PartialSet <: Test.AbstractTestSet + description::String + n::Int +end +PartialSet(desc::AbstractString) = PartialSet(String(desc), 0) +Test.record(ts::PartialSet, r::Test.Pass) = (ts.n += 1; r) +Test.record(ts::PartialSet, r::Test.Fail) = (ts.n += 1; r) +Test.record(ts::PartialSet, r::Test.Broken) = (ts.n += 1; r) +Test.finish(ts::PartialSet) = ts +partial_fold(ts::PartialSet) = (; npass=ts.n, nfail=0, nerror=0, nbroken=0, sections=()) + function Test.record(ts::StubSet, res) res isa Test.Pass && (ts.npass += 1) res isa Test.Fail && (ts.nfail += 1) diff --git a/test/core/test_provider.jl b/test/core/test_provider.jl index d1d80fb..6e3cc71 100644 --- a/test/core/test_provider.jl +++ b/test/core/test_provider.jl @@ -5,7 +5,7 @@ isdefined(Main, :TSHelpers) || include(joinpath(@__DIR__, "helpers.jl")) using .TSHelpers # The unit-testset provider seam, exercised with a STUB provider — no other package involved. The -# contract is what matters here: the type a unit runs in, the second step a hand-popped testset +# contract is what matters here: the type a unit runs in, the second step a hand-entered testset # needs, and the fold that keeps the counts right when the testset is not ours. The real consumer # (Pinax) is tested end to end in `test_pinax.jl`. @@ -120,3 +120,22 @@ end TestShards.UNIT_PROVIDER[] = saved end end + +@testset "a provider that cannot record an Error does not truncate the shard" begin + # The only way to reach `_run`'s Error branch is a unit whose body THROWS — not an + # ordinary `@test` failure — so a provider author testing assertions has no reason to + # cover it. Before, the resulting `MethodError` escaped `_run`: the unit went + # unrecorded, every unit after it never ran, and the failure read as a TestShards + # internal rather than as the unit that could not load. + ctx = bare_context(; shard="", nshards=1) + ran = String[] + with_provider(; open=k -> PartialSet(k), fold=partial_fold) do + TestShards._run(ctx, "u1.jl", () -> (push!(ran, "u1"); @test true)) + TestShards._run(ctx, "u2.jl", () -> (push!(ran, "u2"); error("a load error"))) + TestShards._run(ctx, "u3.jl", () -> (push!(ran, "u3"); @test true)) + end + @test ran == ["u1", "u2", "u3"] # the shard was not truncated + @test [r.key for r in ctx.records] == ["u1.jl", "u2.jl", "u3.jl"] + u2 = only(filter(r -> r.key == "u2.jl", ctx.records)) + @test u2.nerror >= 1 # ...and the unit that threw is NOT green +end