Julia 1.13 turned the testset stack into a ScopedValue

.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

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 <shimozono-sota631@g.ecc.u-tokyo.ac.jp>"]
 
 [deps]

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",
     ],

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.

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

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)))

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