Record the eighth sweep, and count the cold dump's boxes properly
The sweep over the 6,300 lines of tests #19 adds, and over the seventh sweep's own commit. Seven findings, in iris `77ed7a2`. Also corrects a number four earlier rounds carried: the cold dump over 400 depth-5 trees is 34,488 boxes, not 34,492. The run prints eight lines that are not a box, so `wc -l` gives 34,496 and a partial filter gives whatever it gives. Nothing any round concluded moves, because each compared two dumps rather than their line counts -- but the handoff now says how to count, so the next round does not have to guess which figure was right. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5b34d406c7
commit
0b60713806
2 files changed
+137
-5
No files matched your search
@@ -6,6 +6,125 @@ nothing here is rediscovered. Each entry says who found it and when.
|
||||
it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and
|
||||
the current plan is in `docs/HANDOFF.md`.
|
||||
|
||||
## Eighth sweep: the tests, and the seventh sweep's fix (2026-09-20)
|
||||
|
||||
Over the part no earlier round named -- the 6,300 lines under `tests/`, which
|
||||
is more than half of what #19 adds -- and once more over `f8aa0c5`, which was
|
||||
the seventh sweep's own fix and so unreviewed. Seven findings, all in
|
||||
`77ed7a2`. No library code changed: the cold dump over 400 depth-5 trees is
|
||||
byte-identical to `f8aa0c5` across all **34,488** boxes, and the seed scans
|
||||
have nothing to find, since nothing that decides a layout moved.
|
||||
|
||||
That count is not the 34,492 the four rounds before this one recorded, nor
|
||||
the 34,490 `77ed7a2`'s own message gives. The dump prints eight lines that
|
||||
are not a box -- cargo's two, libtest's four, and two blanks -- so a `wc -l`
|
||||
of the run comes to 34,496 and any partial filter lands somewhere between.
|
||||
The boxes are the lines matching `^[0-9]+ [0-9]+ `, and there are 34,488 of
|
||||
them at both `f8aa0c5` and `77ed7a2`. What every round actually established
|
||||
is still true, since each diffed two dumps rather than trusting a count.
|
||||
|
||||
**A shrunk fixture that names one widget three times.** `width`, `sized` and
|
||||
`align` set a rule on the widget they are handed and give its own id back;
|
||||
only `pad` and `wrapper` make a new widget. So in
|
||||
|
||||
let wrapped = wtext("Wrapping shapes").size(16).wrap(true).add(&mut h.rsc);
|
||||
let sized = wrapped.width(76).add(&mut h.rsc);
|
||||
let aligned = sized;
|
||||
|
||||
all three names are one text, and all three went into the vector of ids the
|
||||
case compares warm against cold. Measured: `plant` and `plant_fixed` list six
|
||||
ids and hold four widgets, `plant_pair` lists four and holds three,
|
||||
`plant_scrolled` lists eight and holds seven; the other nine fixtures are
|
||||
honest. So four cases check fewer boxes than they say, and the doc comment of
|
||||
each quotes the inflated count as the size of the tree the shrinker reduced
|
||||
to -- which is the number a reader uses to judge whether a case is still the
|
||||
minimal one. The names are gone and the counts are what the fixtures build.
|
||||
`trace_unsettled.rs` carries a copy of two of these fixtures and had the same
|
||||
aliases.
|
||||
|
||||
The check that mattered here is that a rebuilt fixture is the same tree: a
|
||||
regression test whose fixture quietly changed still passes and no longer
|
||||
covers its defect. Each was diffed against its old self -- same widget slots,
|
||||
same regions, for both settings of `swapped`.
|
||||
|
||||
**A helper at the top of the file, and seven copies of its body below it.**
|
||||
`assert_same_regions` collects every widget whose box moved and reports them
|
||||
together. Six tests call it; seven more spell the eight lines out instead,
|
||||
byte for byte. They call it now, and it took `#[track_caller]` so the panic
|
||||
names the case rather than the helper.
|
||||
|
||||
**The GPU rigs' adapter probe, written twice.** `draw_cost` and `chain_cost`
|
||||
each held a `config` (identical) and an adapter probe (identical but for the
|
||||
feature it asks for and what it returns). The probe leaks its `Instance` on
|
||||
purpose -- a Vulkan loader may unload the driver as a test thread exits --
|
||||
and only `draw_cost` said so, with `chain_cost` referring the reader to it.
|
||||
Both come from `tests/gpu/mod.rs` now, shared through `#[path]` the way
|
||||
`scenario/mod.rs` already is, with the justification on the thing it is
|
||||
about.
|
||||
|
||||
**The mask a widget is clipped by, resolved three times**, two of them a
|
||||
byte-identical closure defined inside a loop. `mask_bounds` takes the
|
||||
`MaskIdx` rather than the widget, because the third site reads the slot it
|
||||
saved before the frame: that a redraw keeps the same slot is exactly what it
|
||||
is checking, and a helper that looked the slot up again would have made that
|
||||
assertion pass for the wrong reason.
|
||||
|
||||
**A field nothing reads.** `Layered::_revision` existed to be incremented, to
|
||||
mark its widget dirty. Two tests in the same file already do that with
|
||||
`widgets_mut().get_dyn_mut(id)`, which is the idiom `tests/scenario/mod.rs`
|
||||
uses as well. The underscore was hiding the dead-code warning that would have
|
||||
said so.
|
||||
|
||||
**A claim the test below it does not make.** `plan.rs` said "Every
|
||||
simplification is strictly smaller, so taking them in turn reaches a fixed
|
||||
point instead of circling", and then asserted `small.size() <= node.size()`.
|
||||
Measured: 53 of one tree's 101 simplifications keep the widget count, because
|
||||
`Plan::size` counts widgets and dropping an alignment or stepping `Wrapped ->
|
||||
OneLine` does not change it. The assertion is the right one and the claim was
|
||||
not. What actually rules out circling is that those steps are one-way too --
|
||||
a `Some` becomes a `None`, a kind steps down a ladder with no way back up --
|
||||
and the comment now says that. The test is `no_simplification_of_a_plan_is_larger_than_it`.
|
||||
|
||||
**Numbers and lines that had gone stale.** `generated.rs` says "Eight that
|
||||
have never failed" and "the ten the others check"; both said one fewer,
|
||||
having been written when `SEEDS` had nine entries and not updated when
|
||||
`d8ae9c3` added seed 20. The `should_panic` scroll test ended in an
|
||||
`h.frame()` that cannot run, because `Harness::set_root` lays the tree out
|
||||
and is where the panic comes from -- verified by deleting it. Two
|
||||
`drop(tree)` calls sat at the end of their own scope.
|
||||
|
||||
### Tripped a rule and left as it stands
|
||||
|
||||
- `env` is written four times under `tests/` (`layout_dump`,
|
||||
`layout_diagnostics`, `revision_cost`, `scenario`). `revision_cost`'s is
|
||||
deliberate and documented: that file is kept in the API subset an old
|
||||
worktree also has, so it can be dropped in and measured there, and a
|
||||
`#[path]` module would break that. Sharing the other three means either a
|
||||
new file for six lines of `std` or pulling `scenario`'s 494 lines into two
|
||||
more binaries.
|
||||
- `determinism.rs`'s `BranchesOnMeasurement` is `iris::random::Branch` with
|
||||
the same four fields and nearly the same body, and `unsettled.rs` beside it
|
||||
uses the real `Branch`. The difference is load-bearing: the copy states no
|
||||
contract, so the framework must re-ask it at every width, which is the
|
||||
whole point of a test about whether a re-measure branches the same way.
|
||||
`Branch` also has a `size_hint` that changes how a span treats it.
|
||||
- Three tests call `h.frame()` immediately after `h.set_root`, which already
|
||||
frames. Unlike the `should_panic` one, these are reachable and assert that
|
||||
a settled second frame adds no draws. Left; the redundancy reads as noise
|
||||
but removing it removes a check.
|
||||
- `primitive_bounds` and `primitive_masks` share the walk from a widget's
|
||||
primitives to their instances and differ only in the field they take.
|
||||
Left: naming the intermediate means exporting the instance type into the
|
||||
test, which is more coupling than the four shared lines are worth.
|
||||
- `Harness::replay` ignores each sample's `t_ms`, and the parser rejects
|
||||
time running backwards with a comment about the wait between samples --
|
||||
which is `replay-touch`'s behaviour, not the harness's. Left: the parser
|
||||
is shared by both replays and the rule is the real one, but a harness
|
||||
gesture has no timing, so nothing in-process can measure a fling velocity.
|
||||
- `layout_diagnostics.rs`'s `report` takes `_harness` and reads it, because
|
||||
it is only used under `layout-diagnostics`. An underscore on a parameter
|
||||
the body uses is backwards, but the alternative is a `cfg_attr`ed allow.
|
||||
|
||||
## Seventh sweep: the rigs, `Fixed`, and the sixth sweep's fix (2026-09-20)
|
||||
|
||||
Over what no earlier round named -- `src/random.rs` and `tests/scenario/`,
|
||||
|
||||
Reference in new issue
Block a user