diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 8bf49e5..9f493d7 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -130,7 +130,7 @@ jobs: strategy: fail-fast: false matrix: - julia: ['1.10'] + julia: ['1.10', '1.11', '1.12', '1.13'] steps: - uses: actions/checkout@v7 - uses: julia-actions/setup-julia@v3 diff --git a/Project.toml b/Project.toml index cc10b2e..0a1f4d9 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "TestShards" uuid = "acceef1d-f5e0-4fe4-a546-818dc56ce7b2" -version = "0.3.40" +version = "0.3.41" authors = ["sota shimozono "] [deps] diff --git a/docs/make.jl b/docs/make.jl index eee9d66..9ade2e0 100644 --- a/docs/make.jl +++ b/docs/make.jl @@ -20,6 +20,7 @@ makedocs(; "Records" => "records.md", "Guarantees" => "guarantees.md", "Composing" => "composing.md", + "Testset internals" => "testset-internals.md", "API" => "api.md", "References" => "references.md", ], diff --git a/docs/src/testset-internals.md b/docs/src/testset-internals.md new file mode 100644 index 0000000..2995ac6 --- /dev/null +++ b/docs/src/testset-internals.md @@ -0,0 +1,116 @@ +# Testset internals + +A unit runs inside a testset this package enters by hand, not through `@testset`. That one +decision is why `Test`'s internals appear in `src/run.jl` at all, and why the file carries two +implementations of the same six lines. + +## Why not `@testset` + +A top-level `@testset` **throws instead of returning** when something inside it fails. TestShards +needs the opposite: a failed unit's tree has to be readable, because the counts in +`unit_fold` and the per-unit record are built from it, and because the remaining units +still have to run. So `_run` enters the testset itself, runs the body, leaves, and re-signals +failure once at the end of the whole shard. + +Measured on 1.10, 1.12 and 1.13 alike — this is not a version-specific quirk that a newer Julia +removed the need for. + +## What Julia 1.13 changed + +Through 1.12, "the current testset" was a **stack in task-local storage** (quoting +`stdlib/Test/src/Test.jl`, so these two blocks are not runnable here — the first no longer +exists on 1.13, the second not before it): + +```julia +function push_testset(ts::AbstractTestSet) # julia ≤ 1.12 + testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[]) + push!(testsets, ts) + setindex!(task_local_storage(), testsets, :__BASETESTNEXT__) +end +``` + +In 1.13 it is a scoped value: + +```julia +const CURRENT_TESTSET = ScopedValue{AbstractTestSet}(FallbackTestSet()) # julia ≥ 1.13 +const TESTSET_DEPTH = ScopedValue{Int}(0) +``` + +A scoped value cannot be pushed and popped — it is *entered* — so `push_testset` and +`pop_testset` are gone rather than renamed. This was deliberate and announced: Julia's own +`NEWS.md` for 1.13 carries "The testset stack was changed to use `ScopedValue` rather than task +local storage", and `Test.@testset` itself now expands to `@with(CURRENT_TESTSET => ts, +TESTSET_DEPTH => get_testset_depth() + 1, expr)`. + +`_with_testset` is that difference and nothing else. Everything downstream — `_unit_close`, +`unit_fold`, the records, the printed line — sees the same thing either way, which is the +contract both implementations have to meet: + +```jldoctest +julia> using Test, TestShards + +julia> ts = Test.DefaultTestSet("a unit"); + +julia> outer = Test.get_testset_depth(); + +julia> TestShards._with_testset(ts) do + Test.get_testset() === ts, Test.get_testset_depth() - outer + end +(true, 1) + +julia> Test.get_testset_depth() == outer +true +``` + +The depth is asserted as an INCREMENT, not as `1`: it counts from whatever is already open, so +the absolute value depends on the caller — inside Documenter's own testset this block reads `2` +where a bare session reads `1`. One level deeper, and back where it started, is the property +`_with_testset` actually has. + +## Why the branch tests the name, not the version + +`@static if isdefined(Test, :CURRENT_TESTSET)`, not `VERSION >= v"1.13"`. + +The two agree on every released version — measured on 1.10, 1.11, 1.12, 1.13 and 1.14-DEV — and +disagree in exactly the place a version test is wrong, because a prerelease sorts before its own +release: + +```jldoctest +julia> v"1.13.0-rc1" >= v"1.13" +false +``` + +An rc **has** `CURRENT_TESTSET`, so a version test would send it down the branch that calls +`push_testset` and it would die on the very bug this split exists to avoid. + +## Why `TESTSET_DEPTH` moves with `CURRENT_TESTSET` + +Two things read the depth, and both fail quietly when it reads `0`: + +- `Test.finish` decides from it whether a testset is top-level, and a top-level one **throws + instead of recording**. A failing nested `@testset` inside a unit would then be caught by + `_run`'s own `catch` and filed as a `:nontest_error` — a **fail reported as an error**. +- stdlib's `@testset` infers an untyped nested set's type as + `get_testset_depth() == 0 ? DefaultTestSet : typeof(get_testset())`. A `0` there drops a + registered provider's testset type (see [Composing](composing.md)), which is the case that + records nothing at all and reports that silently. + +`test/core/test_failure.jl` pins the first of these directly. + +## Tasks spawned inside a unit + +A scoped value is inherited by tasks spawned inside its scope; task-local storage was not. For a +unit that `wait`s or `fetch`es everything it spawns, 1.13 is strictly better — results that used +to vanish into the fallback testset now land on the right testset. For a unit that does **not** +join, the same inheritance means a late result targets a testset the shard has already folded +and reported, turning a deterministic drop into a race. + +So: **a unit must join every task it spawns before its own top-level code returns.** This is +stated on `@shard` as well, because it is a requirement on the caller, not an internal detail. + +## What is still not public + +Neither `push_testset` nor `CURRENT_TESTSET` is exported or marked `public`. This package +therefore depends on an internal on both sides of the branch, and a later reshuffle upstream +will break it again in the same way. The public route — `@testset` and its return value — is +unavailable for the reason at the top of this page. diff --git a/src/provider.jl b/src/provider.jl index 71f5e4a..2e4e109 100644 --- a/src/provider.jl +++ b/src/provider.jl @@ -13,8 +13,8 @@ # So the type is a registered choice. A provider supplies three operations: # # open(key) -> an AbstractTestSet, or `nothing` to decline this unit (use the default) -# close(ts) -> nothing; run after the testset is popped, for a tool that attaches a finished -# set to its parent there (`Test.finish` does exactly that) +# close(ts) -> nothing; run after the testset stops being current, for a tool that attaches +# a finished set to its parent there (`Test.finish` does exactly that) # fold(ts) -> the counts + structure, as PLAIN DATA (see `unit_fold`) # # `open` returning `nothing` is what keeps this inert: a provider decides per unit whether its @@ -36,8 +36,8 @@ rather than a method override, so nothing is overwritten at precompile time. - `open(key::String)` returns the `AbstractTestSet` for a unit, or **`nothing`** to decline it and leave the default in place. Decline unless the tool's capture is actually running: a suite that merely depends on the tool must not have its testset type changed underneath it. - - `close(ts)` runs after the testset is popped. A tool that attaches a finished testset to its - parent does it here (`Test.finish`), which is the only moment at which it can. + - `close(ts)` runs after the testset stops being current. A tool that attaches a finished + testset to its parent does it here (`Test.finish`), which is the only moment at which it can. - `fold(ts)` returns the counts and structure as plain data — see [`unit_fold`](@ref). This is what keeps the balancing history and the completeness verdict correct when the testset is not ours, and it is the first thing to test: the same suite must yield the same numbers whichever diff --git a/src/run.jl b/src/run.jl index 3c51cee..562caf5 100644 --- a/src/run.jl +++ b/src/run.jl @@ -2,6 +2,31 @@ # Running a unit # ───────────────────────────────────────────────────────────────────────────────────── +# Run `f` with `ts` as the current testset, and leave the previous one current afterwards. +# +# Two implementations because Julia 1.13 replaced the testset stack with a `ScopedValue`. Both +# reach into `Test` internals; the branch tests for the NAME rather than the version, and +# `TESTSET_DEPTH` has to travel with `CURRENT_TESTSET`. Why, for all three, is in +# `docs/src/testset-internals.md`. +@static if isdefined(Test, :CURRENT_TESTSET) + function _with_testset(f, ts) + return Base.ScopedValues.with( + f, + Test.CURRENT_TESTSET => ts, + Test.TESTSET_DEPTH => Test.get_testset_depth() + 1, + ) + end +else + function _with_testset(f, ts) + Test.push_testset(ts) + try + return f() + finally + Test.pop_testset() + end + end +end + function _key(ctx::ShardContext, path::AbstractString) return replace(relpath(abspath(path), ctx.root), '\\' => '/') end @@ -12,9 +37,10 @@ end Observe a unit, and run `body` if this shard owns it. The observation counter advances either way — that is what keeps `index`, and the round-robin fallback, identical across shards. -The testset is pushed and popped by hand rather than via `@testset` so that the tree can be -read back even when the unit failed: a top-level `@testset` throws before returning its result. -Failure is re-signalled once, at the end of the whole block. +The testset is entered by hand rather than via `@testset` so that the tree can be read back +even when the unit failed: a top-level `@testset` throws before returning its result (still +true on 1.13 — measured, not assumed). Failure is re-signalled once, at the end of the whole +block. """ function _run(ctx::ShardContext, key::AbstractString, body) ctx.seen += 1 @@ -23,24 +49,26 @@ function _run(ctx::ShardContext, key::AbstractString, body) push!(ctx.ran, (index, String(key))) ts = _unit_testset(key) - Test.push_testset(ts) t0 = time() - try - body() - catch err - # 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, - Test.Error( - :nontest_error, Expr(:tuple), err, Base.catch_stack(), LineNumberNode(0) - ), - ) - finally - Test.pop_testset() + _with_testset(ts) do + try + body() + catch err + # 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) + ), + ) + end end # A tool that attaches a finished testset to its parent can only do it now that the parent is - # current again — which is the whole reason a hand-popped testset needs this second step. + # current again — which is the whole reason a hand-entered testset needs this second step. _unit_close(ts) dt = time() - t0 sec = unit_fold(ctx, ts) @@ -243,6 +271,14 @@ docstring for the whole picture. The test root — what unit keys are relative to — is the directory of the file this macro is written in, so keys are stable no matter where CI runs from. + +A unit must `wait`/`fetch`/`@sync` every task it spawns before its own top-level code returns. +A unit is folded and reported the moment that code returns, so a result arriving later is +recorded into a testset nobody reads again. On Julia ≥ 1.13 this is worse than it sounds: a +spawned task inherits the unit's testset for its whole life, so a LATE FAILURE lands on the +right object after the unit has already been reported green. Before 1.13 the same result was +dropped on the floor instead — deterministically, which is the only thing that made it +survivable. """ macro shard(body) root = abspath(dirname(String(__source__.file))) diff --git a/test/core/test_failure.jl b/test/core/test_failure.jl index 34a96a0..6bf14c5 100644 --- a/test/core/test_failure.jl +++ b/test/core/test_failure.jl @@ -12,3 +12,31 @@ using .TSHelpers # re-signalled once at the end instead of thrown where it happened. @test "bad.jl" in unit_keys(out) end + +@testset "a failing NESTED testset is a fail, not an error" begin + # The depth the unit's testset is entered at is what tells an inner `@testset` it is not + # top-level. Get it wrong and the inner one's `finish` throws instead of recording, `_run` + # catches that as a `:nontest_error`, and a real FAIL is reported as an ERROR — the same + # count, in the wrong column, with nothing red to say so. + # + # The bare `@test false` above cannot see this: it records straight onto the current testset + # and never reaches the depth logic at all. + d = make_suite(; extra="include(\"nested.jl\")") + write( + joinpath(d, "nested.jl"), + """ + using Test + @testset "outer" begin + @test true + @testset "inner" begin + @test false + end + end + """, + ) + ok, log, out = run_suite(d) + @test !ok + @test occursin("[FAIL] nested.jl", log) + @test occursin("(1 pass, 1 fail, 0 error)", log) + @test !occursin("(0 pass, 0 fail, 1 error)", log) +end