Skip to content

Compose with Pinax: a unit-testset provider, so a sharded suite reports margins instead of counts #22

Description

@sotashimozono

A sharded suite currently reports counts. Pinax's test bridge reports margins, code and convergence figures — and the two cannot be used together at all. This issue is the design for the extension that composes them, plus the two measured reasons an extension is required rather than convenient.

1. Measured: they are mutually blind today

Pinax.test installs a capturing root testset (PinaxTestSet <: AbstractTestSet), and a suite's nested @testsets are captured because Test builds a nested set from the parent's type. TestShards does not go through @testset: _run constructs Test.DefaultTestSet(key) by hand (deliberately — a root @testset throws before returning its tree, so a failed unit could not be read back).

Consequence, measured on a two-line suite (@shard begin include("unit_a.jl") end, one passing @test isapprox(e, 0.8; rtol=0.01)):

TestShards: 1/1 units ran — 1 pass, 0 fail, 0 error
Pinax test report: 0/0 passed          ← empty, and GREEN

An empty-but-green report is the worst possible failure: it is indistinguishable from a suite with no tests. Two independent causes, both verified:

  1. Type. Inside a unit the ambient testset is a DefaultTestSet, so Pinax's one integration seam (Pinax._current_container()) returns :inert — no checks recorded, and @figure / @code / @desc written in a test silently no-op.
  2. Nesting. _run does push_testset / pop_testset without Test.finish, and Pinax nests a child into its parent in finish. So even with the right type the unit would not appear:
root = PinaxTestSet("root"); Test.push_testset(root)
u = PinaxTestSet("unit_a.jl"); Test.push_testset(u)
Test.record(Test.get_testset(), <a Pass>)
Test.pop_testset();  length(root.children)   # → 0     ← vanished
Test.finish(u);      length(root.children)   # → 1     ← nested, 1 check, got = 0.8

The reverse direction fails too: _section reads counts by walking ts.results and matching r isa Test.DefaultTestSet. A foreign testset type has neither, so its counts would be silently dropped from UnitRecord — and the balancing history and the completeness verdict are built on those counts.

So this is not "wire two packages together". Either package alone, in the other's presence, loses data quietly.

2. Why not translate the records instead

The cheap alternative is a one-way translation: UnitRecord/Section → Pinax.TestNode (Pinax's closure-free dump DTO), then Pinax.render_test_report. It needs no extension point at all.

It is the wrong design, because Section carries aggregate counts (npass/nfail/nerror/nbroken) and Pinax's entire value is per-assertion: Check(got, want, delta, tol, pass). Aggregates cannot produce a margin, a convergence figure, or the code region behind an assertion. Translating records would yield a report that shows verdicts and calls itself a Pinax report — strictly worse than what Pinax already produces unsharded, and it would look right.

Whatever we build, Pinax's own capture has to see the assertions. That means the unit testset must be Pinax's type, which means TestShards needs an extension point.

3. The extension point (three operations, one hook)

Everything the composition needs is at the boundary of _run. Introduce a unit-testset provider:

# core — the default is exactly today's behaviour
unit_testset(key)      = Test.DefaultTestSet(key)   # open
unit_close(ts)         = nothing                    # after pop_testset
unit_fold(ctx, ts)     = _section(ctx, ts)          # read a Section tree back out
  • unit_close and unit_fold dispatch on the testset type, so an extension adds methods and never overwrites one.
  • unit_testset is a choice, not a dispatch — there is nothing to dispatch on. Register it the way Pinax registers its own seam: a Ref assigned in the extension's __init__ (a Ref assignment, not a method override, so no precompile-time method overwriting). Registering a second provider is a loud error naming both — two tools cannot both own the type of one testset, and silently letting the last one win is the kind of thing that shows up as a mysteriously empty report.

_run becomes:

ts = unit_testset(key)
Test.push_testset(ts)
try  body()  catch err  Test.record(ts, Test.Error(...))  finally  Test.pop_testset()  end
unit_close(ts)                     # Pinax: Test.finish(ts) → nests into the capturing root
sec = unit_fold(ctx, ts)           # Pinax: counts from its own folded tree

Four lines changed, and with no extension loaded the bytes executed are the same as today.

ext/TestShardsPinaxExt.jl

[weakdeps]   Pinax = "e782a80a-…"
[extensions] TestShardsPinaxExt = "Pinax"

Test is already a hard dependency of TestShards, so loading Pinax alongside it also loads PinaxTestExt — PinaxTestSet is always available inside this extension.

  • unit_testset: return PinaxTestSet(key) only when a Pinax capture is ambient —
    Test.get_testset_depth() > 0 && Test.get_testset() isa PinaxTestSet. No environment sniffing, and this is the crucial guard: a package that merely depends on Pinax must not have its testset type changed. Under a bare Pkg.test() the check is false and the default stands (Pinax's own invariant V: the report changes nothing about how the suite runs).
  • unit_close(ts::PinaxTestSet): Test.finish(ts). Safe at depth > 0 — Pinax's finish only throws at the root, and TestShards' own end-of-block failure re-signal is unchanged.
  • unit_fold(ctx, ts::PinaxTestSet): build the Section tree from Pinax's tree. Pinax exposes _ntests / _nfail / _nerror / _nbroken duck-typed over it, and PinaxTestSet.elapsed carries the duration, so the balancing history and the completeness verdict are unaffected — the same numbers by a different route. That is the property to test first.

evidence! needs no change: it keys a Dict on the testset object, and a PinaxTestSet is mutable, so it works with any type.

4. What the composition buys

A shard's Pinax root dumps instead of rendering (PINAX_TEST_DUMP), each shard uploads its dump, and one merge job renders all shards as one document: one page per unit, sections mirroring the @testset nesting, per-check margins, the code behind each check, and @testset for sweeps folded into convergence figures. The shard boundary does not appear in the output.

This needs Pinax ≥ 0.39.16 (QAtlasHub/Pinax.jl#90): before it, a dump carried checks and codes only, so a sharded report silently lost every @desc / @table / @raw and re-ordered what survived. Sharded and unsharded are now the same document except for a captured @figure, whose generator is a closure and cannot be serialized (that case warns loudly).

CI shape — it fits the existing reusable workflow, which already has small merge jobs for timings, completeness and coverage:

shard s1..sN   TESTSHARDS_ID / PINAX_TEST_DUMP=pinax-dumps/s1.toml   → artifact
merge          Pinax.render_test_report(readdir("pinax-dumps"); out="test-report")

A new pinax: true workflow input, off by default.

5. The claim worth making

Neither package can state this alone: a sharded test report that can prove it is complete. TestShards observes the whole unit sequence in every shard, so it knows exactly which units should exist (that is what completeness already checks); Pinax holds the merged document. Together the report can carry the verdict "N units, all accounted for" as part of the artifact rather than as a line in an expired CI log — a sharded report that is missing a shard currently looks like a smaller suite.

That needs a small Pinax API (a panel injected into the overview page — render_test_report(…; overview_panels)), so it is phase 3, not phase 1.

6. Phases

  1. The extension point + TestShardsPinaxExt (§3). Test first that UnitRecord counts and durations are identical through both folds, on the same suite — that is the regression that would silently corrupt balancing. Then: a sharded run of a real suite renders one document with margins.
  2. evidence! → Pinax content. evidence!(; tolerance, achieved, oracle) is already "what this test established"; in a Pinax report it should appear, as a table beside the check. Content, not structure — no test file changes.
  3. Completeness as a first-class panel (§5), gated on the small Pinax API.

Not in scope: rendering from records (§2), and parallelism inside a job (ParallelTestRunner.jl / ReTestItems.jl — they compose with this and are unaffected).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions