Skip to content

Julia 1.13 turned the testset stack into a ScopedValue - #90

Merged
sotashimozono merged 5 commits into
mainfrom
fix/testset-stack-became-a-scoped-value
Sep 10, 2026
Merged

sotashimozono merged 5 commits into
mainfrom
fix/testset-stack-became-a-scoped-value

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

What broke

Every repo calling sharded-tests.yml@main went red on the ci leg when the runner's
"latest Julia 1" moved. Measured on AbstractQAtlas.jl #144, same branch, four hours apart:

leg Julia result
ci (unpinned) 1.12.7 green, 22 checks
ci (unpinned) 1.13.0 all 4 shards fail
ci-lts (pinned 1.10) 1.10.12 green throughout
ERROR: LoadError: UndefVarError: `push_testset` not defined in `Test`

It is not a rename

Through 1.12 the current testset was a stack in task-local storage:

function push_testset(ts)                                    # 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 ScopedValue:

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. Test.@testset itself switched to
@with(CURRENT_TESTSET => ts, TESTSET_DEPTH => get_testset_depth() + 1, expr) for the
same reason. get_testset and get_testset_depth survive, reimplemented on top of it.

The fix

_run's push / try / finally pop becomes _with_testset(f, ts), one implementation
per side of the change. Nothing else in the file moves.

Measured, not assumed

  • The reason _run hand-rolls this is still true on 1.13. A top-level @testset
    throws TestSetException instead of returning, so a failed unit's tree cannot be
    read back — which is exactly what _run needs. So "just use @testset" is not an
    escape from the internals here. Checked on 1.13 directly.
  • Base.catch_stack() is not part of the breakage. On 1.13 it returns the same
    Base.ExceptionStack as current_exceptions() — == and all. Left alone rather than
    swept into this fix.
  • Negative control. Unfixed main on 1.13 fails with exactly the CI error; with the
    fix the suite passes on both 1.10 and 1.13, so the new branch did not buy 1.13 by
    breaking the old one.

What this does not fix

It swaps one Test internal for another. Neither push_testset nor CURRENT_TESTSET is
public API, so a later reshuffle breaks this the same way. That is stated in the comment
next to the code, together with why the public route is unavailable, so the next person
meets the constraint rather than rediscovering it.

Version

0.3.40 → 0.3.41, patch: a fix, nothing breaking.

🤖 Generated with Claude Code

Every repo calling `sharded-tests.yml@main` is red on the `ci` leg since
the runner's "latest Julia 1" moved 1.12.7 → 1.13.0:

    ERROR: LoadError: UndefVarError: `push_testset` not defined in `Test`

Not a rename. Through 1.12 the current testset was a STACK in task-local
storage, entered with `Test.push_testset` and left with
`Test.pop_testset`. In 1.13 it is `Test.CURRENT_TESTSET`, a `ScopedValue`
— which cannot be pushed or popped at all, so both functions are gone.
`Test.@testset` itself switched to `@with` for the same reason. The
`ci-lts` leg stayed green because it pins 1.10.

`_run` now enters through `_with_testset(f, ts)`, one implementation per
side of that change. Nothing else moves.

Measured rather than assumed, three things:

- The reason `_run` hand-rolls this at all — a top-level `@testset`
  throws instead of returning, so a FAILED unit's tree cannot be read
  back — is still true on 1.13. So `@testset` is not an escape from the
  internals here.
- `Base.catch_stack()` is not part of the breakage: on 1.13 it returns
  the same `Base.ExceptionStack` as `current_exceptions()`, `==` and all.
  Left alone.
- Negative control: unfixed `main` on 1.13 fails with exactly the CI
  error; fixed, the suite passes on BOTH 1.10 and 1.13, so the new branch
  did not buy 1.13 by breaking the old one.

Stated in the comment rather than hidden: this swaps one Test internal for
another. Neither `push_testset` nor `CURRENT_TESTSET` is public API, so a
later reshuffle breaks this again — and the reason we cannot use the
public route is written next to it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Docs preview: https://qatlashub.github.io/TestShards.jl/previews/PR90/

(updates on each push to this PR)

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/run.jl 45.45% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

sotashimozono and others added 4 commits September 10, 2026 13:29
…d the scope actually do

The depth line was the thin part. `TESTSET_DEPTH => get_testset_depth() + 1`
is new hand-maintained state with no analogue on the pre-1.13 branch, and
two separate things read it, both failing quietly at 0:

  - `Test.finish` decides top-level-ness from it. A failing nested
    `@testset` inside a unit would then throw instead of recording, `_run`
    would catch that as a `:nontest_error`, and a FAIL would be reported as
    an ERROR.
  - stdlib's `@testset` infers an untyped nested set's type as
    `get_testset_depth() == 0 ? DefaultTestSet : typeof(get_testset())`, so
    a 0 drops a registered provider's type — the case `provider.jl`'s own
    header describes as recording nothing at all, silently.

Neither was covered. The passing nested case already is (`test_records.jl`
and `test_diagnose.jl` assert "outer"/"inner" appear), the failing one was
not, and the only "a unit fails" test uses a bare `@test false`, which
never reaches the depth logic. Added it, and mutation-checked it rather
than trusting it: dropping the depth binding turns the suite into
"0 failed and 3 errored across 23 units" — the predicted signature, not
just some red.

CI could not have kept either branch honest. Only one side of the
`@static if` compiles per run; the sharded jobs take whatever `'1'`
resolves to, which moves with Julia's release calendar, and `compat`
pinned only the 1.10 floor. It now pins 1.13 as well, so both arms stay
deliberately under test after `'1'` moves on.

The scope also propagates differently, and this is the one real behaviour
change: a `ScopedValue` is inherited by tasks spawned inside it where
task-local storage was not. Measured both ways. For a unit that joins what
it spawns, 1.13 is strictly better — results that used to vanish into the
fallback testset now land correctly, even when the old code `wait`ed. For
a unit that does not join, a deterministic drop becomes a race against
`unit_fold`, which for a tool whose job is counting results is the worse
of the two. `@shard`'s docstring now states the requirement.

Also: `run.jl` described the same mechanism two ways three lines apart —
the docstring said "entered by hand" and the comment below still said
"hand-popped". `provider.jl` said "after the testset is popped" twice.
Timing was right in all three; the word was left over from the stack.

`Base.catch_stack` gets an `XXX` rather than a change: measured on 1.13 it
returns the same `Base.ExceptionStack` as `current_exceptions()`, and
upstream's own `@testset` has moved to the public name.

Not in this commit, deliberately: `Test.record` is unguarded against a
provider whose testset type has no `Error` method (pre-existing path, this
commit only moved it); `provider.jl` reads `ts.results` without the
`results_lock` 1.13 added to `DefaultTestSet`; and a late failure from an
unjoined task can segfault Julia 1.13's JIT — reproduced with bare `Test`
and `ScopedValues`, no TestShards involved, so it is an upstream report,
not a fix here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the name

Thirty-one lines of comment above fourteen lines of code. The whole
account of what Julia 1.13 changed, why the branch tests what it tests,
what `TESTSET_DEPTH` is for, and how task propagation differs now lives
in `docs/src/testset-internals.md`, registered in `make.jl`'s pages. What
stays in `run.jl` is what a reader at that line needs: there are two
implementations, both reach into `Test` internals, the branch tests the
NAME, and the depth travels with the testset. Six lines. The `XXX` on
`catch_stack` keeps its register and a pointer, not its argument.

`@shard`'s docstring keeps the "join what you spawn" requirement — that
is a requirement on the caller, not an internal, so the API doc is where
it belongs.

The branch tests `isdefined(Test, :CURRENT_TESTSET)` rather than
`VERSION >= v"1.13"`. They agree on every released version — measured on
1.10, 1.11, 1.12, 1.13 and 1.14-DEV — and disagree exactly where a
version test is wrong: `v"1.13.0-rc1" >= v"1.13"` is false, so an rc,
which does have `CURRENT_TESTSET`, would take the branch that calls
`push_testset` and die on the bug this split exists to avoid.

`compat` now runs 1.10, 1.11, 1.12 and 1.13 instead of just the two ends.
Suite passes on all four locally, plus 1.14-DEV.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of the four code blocks were assertions a reader has to take on
trust; `julia` blocks are inert, so they rot silently. Both are now
`jldoctest` — the repo's first, and `doctest` is on by default, so they
run in the existing docs build.

  - the contract both `_with_testset` implementations must meet
    (`get_testset() === ts`, one level deeper, and back afterwards);
  - `v"1.13.0-rc1" >= v"1.13"` is `false`, which is the whole reason the
    branch tests the name rather than the version.

The first one caught its own mistake on the way in. Written as an
absolute depth it read `(true, 1)` in a bare session and `(true, 2)`
under Documenter, which runs doctests inside a testset of its own — the
assertion was coupled to the ambient environment. The property
`_with_testset` actually has is the INCREMENT: one level deeper, then
back. Asserted that way it holds anywhere, and asserting the "back" half
covers the `finally` this change removed.

Verified it can fail: expecting `(true, 2)` turns the build red.

The other two blocks stay `julia` because they are quotations of
`stdlib/Test/src/Test.jl` and cannot run — the 1.12 one does not exist on
1.13, nor the 1.13 one before it. The prose now says so, rather than
leaving them looking like examples.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The docs preview failed on `Cannot resolve @ref for [`unit_fold`](@ref)`.
The identical link works at provider.jl:41 because a docstring resolves
@ref in its own module context; a hand-written page does not get that.
The link added nothing a code span does not, so it is a code span now.

The doctests themselves ran and passed in that build — the failure was in
the cross-reference pass after them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sotashimozono
sotashimozono merged commit ca91c27 into main Sep 10, 2026
35 checks passed
@sotashimozono
sotashimozono deleted the fix/testset-stack-became-a-scoped-value branch September 10, 2026 14:19
sotashimozono added a commit that referenced this pull request Sep 11, 2026
…it (#91)

* A unit that cannot record its own error must not take the shard with it

`_run` catches an escaping error so the shard survives a unit that throws.
Recording that error undid it. A provider testset type with no
`Test.record` for `Test.Error` — the natural gap, since only a THROWING
unit reaches that branch, not an ordinary `@test` failure — makes the
record call itself throw, and the `MethodError` escaped `_run`.

Reproduced before fixing, three units, the middle one throwing:

    before   [ok] u1.jl
             ESCAPED after [u1, u2] → MethodError: no method matching
                                      record(::PartialSet, ::Test.Error)
             units recorded = ["u1.jl"]

    after    [ok]   u1.jl
             Error: u2.jl threw, and its testset could not record the error
             [FAIL] u2.jl  (0 pass, 0 fail, 1 error)
             [ok]   u3.jl
             units recorded = ["u1.jl", "u2.jl", "u3.jl"]

u3 never ran, and the failure surfaced as a TestShards internal rather
than as the unit that could not load.

Counting the unit as errored when recording fails is the other half, and
the more important one: swallow the record failure alone and the unit
that THREW reports `0 error` and reads green — worse than the truncation.

Tested by recording, not by `hasmethod`. A catch-all
`record(ts, ::Any)` that throws for `Test.Error` satisfies `hasmethod`
and still fails here, so the proxy would license exactly the case it
claims to exclude.

Second, unrelated to that: `_section` walked `ts.results` unlocked.
Julia 1.13 gave `DefaultTestSet` a `results_lock` and takes it on every
`record`; the read side now takes it too, and copies rather than
iterating under it, since `_section` recurses. Only reachable from a unit
that left a task running, which `@shard`'s docstring forbids — but the
forbidden case should be a wrong count, not a walk over a vector being
pushed to.

Also the "hand-popped" left in test_provider.jl's header, missed when
#90 renamed the same phrase everywhere else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* The new test reached for `_begin`, which cannot run inside a shard

The suite runs inside a `@shard` block, and `_begin` installs into
`TestShards.CURRENT` — so calling it from a test replaces the running
block's own context. Every Julia version went red on the same line.

`bare_context` exists for exactly this and says so in its docstring; the
two testsets directly above use it. `test_claim.jl` already pins that
`_begin` from inside is an error. I had all three and still reached for
the driver under test.

`shard=""` makes `_owns` total, so all three units run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant