Repository navigation
Prune redundant unit tests and cover what they missed - #383
Merged
drehtuer merged 19 commits intoOct 5, 2026
Merged
Conversation
Removed tests whose every assertion another test already makes on the same path: - DieMaterialTest "clamp leaves a finite value inside its range alone" (covered by "a value already inside its range is left alone") - BuiltinDiceSetTest "a d2 is a coin, not a cube pretending to be one" (shape, values and order are held by the solid, value-range and shared-fixture tests) - NotationErrorTest "every error code the parser can raise is reachable" (asserted only Empty and SpacedDice, each with a test of its own) - NotationLimitsTest "the explosion depth is what bounds an exploding chain" and "500d6 parses" (subsets of "the published numbers" and the thousand-dice boundary) - CollectionReaderTest "a field the format does not know is ignored", folded into the pre-pinning import test that exercises the same rule - ReferencedFileTest "a good path has nothing to say about it" (implied by "an ordinary relative path is fine") Added tests for what nothing asserted: StandardDice's labels and set variations; ReferencedFile.staysInsidePackage, the extractor's sandbox check, which had no test in its own module; Formula.divides, which decides whether the result sheet offers rounding; collection entries that are not objects, lists that are not lists and the problem's printed form; a long .json name that is not a collection (the old cases were all refused by length alone); the package-wide texture budget; format 0; table id and name errors; and smaller boundaries in the model, stats, glyph and probability code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 14f3325d6dfa47a593d0ac42b21ff5eda2cef5d1)
ShapeHullTest "no corner of any solid pokes out through a face" compared every corner with the largest projection of those same corners, so it could not fail. Convexity is held by ShapeGeometryTest's "every corner is outside every face" and the face-depth tests; the tautology goes. Added: ClearSpace.roomForAnother with dice already promised, both for floor and for the engine's cap, which nothing exercised; TableGeometry's refusal of a width or aspect that is not a number; negative indices for BoardTrack.poseAt and Tumble; a contact strength below zero; negative turning in SimulationOutcome; the harness reading "no" as no, and naming a section that is not an object or a value that is not plain. build-logic gets a test for VerifyDeviceTestResultsTask, the only verdict on a device run now that AGP's own cannot pass over WiFi: it must still fail on a failing or erroring test, on no results where some were expected and on a report of no tests, and must not fail on a test that declined to run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit df4895c3ec8da5175f110d0b7cf45c5f2f3bd423)
DraftFile was the designer's weakest class for branches: a face entry, mark, colour, nib or dot list that is the wrong JSON type was never fed to it, so the promise that a hand-edited draft loses only the broken marks was unchecked. Adds those cases, plus negative cells, empty pips, a defaults table that only moves translucency, a DEL in a set name, and photo equality and subsampling on the axis that was never tried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 86574f1c007caab313033669ddd8c02e3eb5b096)
A tar hard link was never handed to SafeExtractor (only a symlink was), nor a one-byte file or an unpack folder that cannot be made. The forge URL parser had no test for www.github.com, a bare gitlab project, a tree with no ref, a gitlab subfolder or a gitea URL with no repository; the ref resolver none for a reply that names its commit in another forge's field or with non-hex characters; the fetcher none for a redirect with no or an unusable Location. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 779404b5a985f7936881474051f9d8ed95c6176e)
'a die resting on another is not read', 'a die that ends standing on another is counted as stacked, and as unread' and 'a tapped roll still ends the moment its dice are at rest' assert a strict subset of what 'a die that cannot be read is left where it lies' and 'a throw of dice that simply settle is read off their faces' already assert on the same world; their reasoning is folded into those tests' comments. countedSoFar, which the roll screen follows every frame, was never read by a test on either RollLoop or LiveRoll, nor LiveRoll.impacts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 4268c71d5ad9a9cd20ed3aeed8d33753a78ae446)
Neither TrayLoop nor PowerSavingTray had a test for a stalled roll: the dice offered back through onStalled, the faces already read reported through onCounted (power-saving only, and not when there are none), and no outcome or impact playback. TrayLoop.look was never called by a test. These live in a new TrayLoopEndingsTest because TrayLoopTest is at detekt's LargeClass limit. Also covers a die face with a uv short and a thumbnail plan with no height. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d45502e27051252103250b5857e00dd74b1b7a28)
HeadlessRendererTest's 'a frame says where every die is' only read data class fields back; the blend test now asserts the one thing it covered that mattered, that a blended die keeps its index. ShakeDetectorTest's 'a recorder counts what it kept' tested ShakeRecorder in the wrong file and only by size; ShakeRecorderTest now checks that reset also restarts the step clock. Adds the stepsTaken default, a non-increasing gyroscope timestamp, and an amplitude above what the vibrator API takes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit e7d8af6a986485fc70b82e8fe4508d33e2819cb1)
Folds four tests into the ones that already asserted the same path with more (the export sheet opening, a group sheet opening, an existing roll offering Delete, a clash naming what to do, a roll with no pin) and adds tests for the behaviour nothing pressed: Move down on the grip, closing the switcher, a long press on a group, marks, the empty group's New, deleting from the group sheet, un-nesting a group, the editor's group chooser, clearing a mark or colour, a formula naming a missing set, deleting leaving the editor, the import screen's reading state and its file picker, and the presenters' guards against stale presses. The no-downloader import test now really builds the presenter without one; it was handing in a stand-in and never ran the default it named. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 42c5c523b4d2e119007355348c530f23ed992981)
Drops the accessibility test that only re-asserted the histogram being on screen (StatsScreenTest opens a die and asserts the same), and folds the history's "export asks which shape" into a test that also puts the question away. New tests cover the paths nothing reached: the sheets' backdrop dismissal, the line above the list for every order, roll-up and session, the confirmation's wording, a never-thrown die's dash, a roll landing on an open die, a die opened inside a session, a reset with nothing confirmed, a set that stood in for another in the breakdown, the observed-only range, the chart's two colours on a real rasteriser, the session sheet's guards, and TimeStamp, the formatter the history ships with and no test ever ran. StatsScreenTest and HistoryScreenTest had reached detekt's LargeClass limit, so the session tests move to StatsSessionsTest and the forget tests to HistoryForgetTest, each with the setup it needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b3f531aa13cb86df9af3845c14ef703565e1814e)
Drops "tapping a row with nothing wired to it does nothing": the nothing-wired test and "tapping a row asks to open it" already assert the same tap, harmless and opening no sheet. The install-button test now asserts the half its name promised (the button says Installing and is dead while one runs), and the update test goes through the sheet's Update button instead of calling the presenter, which no test pressed. New tests: the fetch button installing from the typed link, the installed and replaced titles, the backdrop putting both sheets away without acting, the outdated count beside the check button, the minus stepper and "under average", a weight quoted as a range, and the export button sharing the package and saying so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 5d8380ac0a6dd3b065e3b3f94281c12c3577e849)
The chart's Canvas was never drawn in a test (Robolectric's legacy graphics skip the draw lambda), so nothing asserted which ink a bar is drawn in, where the band and the roll's line go, or that a tap becomes a total. GraphChartTest captures it under native graphics and checks each ink. The screen now has a test for a tapped bar being named, and the machine for a rolled total outside the distribution and for an exploding formula's truncated tail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ed80ab9c44ab4dc5d586aece1f4f51297c79472a)
The working state, the full "use a photo" row, a typed name reaching the library, answers arriving after the sheet was shut and removing a photo that is not the chosen table had no test. A gated photo library fake holds a photo at the door so the in-between states can be asserted. Drops a thumbnail-presenter test whose assertions were already made by TablesPresenterTest and TableThumbnailScreenTest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b6c203ac36a14a32ce892722770d71f953ab709c)
Removes four RollScreenTest cases whose assertions other tests already make, and the copies of DirectTray, LandingRolls and SilentTray that had drifted into DiceMenuTest, DiceTrayTest and RollAccessibilityTest in place of the shared fakes. Adds the screen over an exploding die waiting for its earned throw, the tray's spoken description at each stage of a throw, TrayReading for earned and stalled states, pixel tests for the debug plan's tints, the formula tab's accent, the progress track on light and dark grounds and a large die's pick ring, a real assertion for the progress rule's fill, and machine tests for an unknown die and a rounding with nothing landed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ce4f1229719cfa133ce01c199a77e04217606c22)
The developer screen had no test of a replay in flight, of the "different faces" verdict as the screen words it, or of a replay under another seed showing a result with no verdict. The presenter's verdict had two untried arms: the same seed typed by hand, and a replay that lands after the log was cleared. Settings had no test that its default, unwired callbacks leave every control where it was, and the menu glyph's drawing was never rasterised. Drops "the whole row is the switch, not just the switch": its one assertion (the power-saving row has a click action) is repeated by "a switch is still the whole row, sentence and all", which now carries its comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b94c282bd5035f7a3603f5e4e9d562e17b6d614b)
DieSilhouette, the up chevron, the formula field's cross and the formula error's wave were never drawn in a test: the legacy canvas records nothing, so their draw lambdas showed as uncovered and a broken drawing could not fail. Each now has one NATIVE-graphics test on the pixels that tell the picture apart (round coin vs square cube, an arrow pointing back, a cross rather than a box, a wave only under the blamed characters). The wave's runs across a line break, OptionBox's role semantics and its two fills had no assertions either. Removed as covered elsewhere: - TagTest "the two kinds do not look the same": a strict subset of "the three kinds do not look the same". - RuleTest "the heavier weight is the default": "a block rule is 2 dp and a hairline is 1" already draws the default rule at 2 dp. - PlateTest "a plate is tall enough for the words plus its padding": implied by the exact 7/8 dp padding test. - PlateTest "the shadow a plate is lifted by is the small one": only compared two tokens whose exact values ModernistTest pins. - SectionKickerTest "a kicker prints the words it was given": the first half of "a kicker follows the words when they change". - FormulaFieldTest "a field with neither label nor hint still draws": same setup as "the field shows the formula it was given", which asserts more. - FormulaFieldTest "a mistake with no obvious reading is offered no fix": the suggestion is FormulaError's decision and FormulaErrorTest asserts it. - FormulaErrorTest "a mistake with an obvious reading is offered as a fix": the button is pressed in "taking the fix hands back the formula". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 123204d0f889f402ee5ba850c997e60d28447b2c)
Three settings writers (active group, active session, default table) were never called by a data test, so the two-key table pin, its removal when cleared, and the "both halves or neither" read had no assertion here. Breakdown.read's per-field fallbacks (a value that is not a number, a flag that is not a boolean, an unknown note, a missing "asked") and a roll's label being written were untried, as were the true side of HistoryEntry.hasBreakdown and a half table pin through pin(). Removed as covered elsewhere: - "a chosen accent is read back": LightBlue is one of the accents "every accent survives the round trip" writes and reads. - "a default set is remembered even while that set is not installed": the same setDefaultSet/read path as "the set a plain d20 comes from is read back"; the repository never consults installed sets. Its comment and its id moved into that test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bfe1bc5213e8f30bce17a9c0dca561b8ac2c0423)
The header's redo button was never pressed by a test, and the rule that the eraser puts no colour in the pen (Palette's "chosen" ignores the swatches while erasing) had no assertion. Removed as covered elsewhere: - "a d20 has twenty faces to move between": "scrolling to the last face leaves the fill buttons where they were" reaches face 19 of a d20. - "tapping the strip moves to that face": "a face copied from the screen lands on the face the strip moved to" asserts the cell after a strip tap, and the announced-selection test covers the rest. - "a tap on the canvas with a pen in hand leaves nothing": the same centre tap with the default pen as "a finger that touches and lifts without moving leaves nothing". - "choosing a nib chooses it" and "tapping another die opens it": the "announced as the one that is chosen" tests click the same controls and assert selection, which the screen derives from state.nib and state.die. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 14d137ed543050fb2fd6ba3a91a1f93fe509be4b)
Most of the lambdas the navigation graph hands to its screens were never invoked: the graph's Roll this and Save as roll, the saved list's take/hold/new/import, the editor's chevron and Roll now, the tray's strip and the result sheet's odds and save. WaysBetweenScreensTest presses each and asserts where it lands, with which argument and, where the design says so, with nothing left under the tray. The file pickers' callbacks were unreachable without a picker. ChosenFilesTest swaps in an ActivityResultRegistry that answers at once and checks a collection read and imported, a file that is not one, one that cannot be opened, a dismissed picker, and a chosen archive that is refused with its cached copy removed. RollWiring.table, the "pin, then chosen, then bundled, then fallback" rule from docs/tables.md, was only reachable through a physics run. It is now internal (production visibility change only, no behaviour change) and RollWiringTest covers each arm, plus the per-visit reads of the catalogue and the choice and power saving drawing no thumbnails. Removed "the editor is opened on a roll, or on a new one": its route assertions are repeated verbatim in "the editor can be opened on a new roll with a formula already in it", which now also carries its one extra check (an existing roll's route resolves to the editor). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 30e1a128a7572e95fe215beab9388424c4598dde)
Branch coverage is 75.4 % and function coverage 92.8 % after the pass, up from 73.2 % and 91.1 %. The production problems the pass turned up are listed in docs/TODO.md rather than fixed here, so this PR stays tests only and none of the new tests asserts a behaviour that is about to change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
drehtuer
added this pull request to stack #385
October 5, 2026 06:36
drehtuer
merged commit Oct 5, 2026
14eb6db
into
feature/designer-material-and-roundness
9 checks passed
drehtuer
added a commit
that referenced
this pull request
Oct 5, 2026
Shows a "Preparing the dice" plate over the tray while a dice material compiles. The plate carries a progress bar and an estimate of the time left, so the wait after an install or update has a visible reason. **Based on #383** (`feature/test-cleanup`), stacked as `.claude/CLAUDE.md` asks. ## Why Filament's materials are compiled on the phone for the driver that is there (decision 46). They are kept in `codeCacheDir`, which Android empties on every update. So the first launch after an install or update spent about 3.2 s on a black tray. A translucent die, the glass table or a textured table then stalled about 2 s mid-throw the first time it was needed. Nothing said why. ## How - **`FilamentEngine`** reports a compile only when the material isn't on disk. It calls `started` before the compile and `finished` in a `finally`, so the plate can't get stuck if a compile throws. A normal launch reads the cache and shows nothing. - **`ShaderWork` / `ShaderProgress`** (plain Kotlin, JVM-tested): - `libfilamat` gives no progress, so the bar fills with elapsed time against an estimate. - It is capped at 95 % until the compile actually ends. - The text counts down "About N seconds left", then says "Almost done" once the estimate has run out. - **`ShaderTimings`** sets where the estimate comes from: - Defaults are the Pixel 10a's figures: opaque 2.4 s and resin 2.0 s measured; glass and table 2.0 s assumed. - What this phone actually took is saved to `noBackupFilesDir/shader-timings.txt`, which survives updates. The next update's bar is then this phone's own figure. - A garbled line falls back to its own default. - **Plumbing:** - `RollThread.shaders: StateFlow<ShaderWork>` → `Tray.shaders`, which defaults to never compiling, so fakes and the power-saving tray are unchanged. - `TrayDriver` forwards it to `RollScreen`. - Power-saving mode makes no engine, so the plate never shows there. - **UI (`PreparingTheDice`):** - A `Plate` centred over the tray, under the controls, that ignores touches. - The title names what is being prepared: dice, translucent dice, glass table or table. - TalkBack gets one polite live announcement, and the bar has progress semantics in whole per cent. - All strings are resources. - **Docs:** - Decision 95. - `docs/architecture.md`: threading, with a mermaid sequence diagram, and the storage layout. - `docs/physics-and-rendering.md`: "Preparing the dice". - Prototype option `1za` in `design/`, the `design/README.md` map, and README's rendering bullet. ## Tests and coverage - `ShaderWorkTest` (14), `ShaderTimingsTest` (10) and `PreparingTheDiceTest` (11, Robolectric, including `RollScreen` integration). - render/filament: function 76.6 → 77.5 %, branch 73.3 → 74.6 %. - feature/roll: function 94.5 → 94.6 %, branch 70.7 → 71.2 %. - `test`, `detekt`, `ktlintCheck` and `lint` are green for render/filament, feature/roll and app. render/filament's device-test sources compile. ## Device check — Pixel 10a, 2026-10-05 Debug build of this branch, fresh install: - **Cold compile.** After `run-as … rm -rf code_cache/materials` and a relaunch, "Preparing the dice" (opaque) shows, then "Preparing the table" (textured felt). Each has a moving bar and "About N seconds left". The plate goes once the felt is drawn. - **Learnt timings.** `no_backup/shader-timings.txt` holds `opaque=2437`, `table=2243` after the first launch and `2451` / `2180` after the second. The figures are stable, so the defaults are about right. - **Warm relaunch.** No plate at all. - **Fresh install.** The first-launch welcome covers the whole screen during the compile, so the plate is not seen there. The compile finishes behind the welcome text. - **render/filament device suite:** 60 tests. 58 passed and 0 failed. 2 declined (`RenderGalleryTest` and `RenderedHarnessTest` are opt-in and skip themselves without `-e gallery` / `-e harness.rolls`). For the owner to judge: the startup compile is shown as two plates in a row ("dice" then "table"), each with its own bar starting from empty. Is that fine, or should it be one bar across both? Also whether the wording reads right. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prunes redundant unit tests and adds tests where function or branch coverage was missing, across all 29 measured modules. Five agents did the work in parallel, one set of modules each, and their branches were merged locally.
Based on
feature/designer-material-and-roundness, which sits on #381. That branch has no PR of its own yet.What changed
SilentTray,DirectTray, presenter setup) now live once inRollScreenFakes. One "test" could never fail:ShapeHullTestcompared each corner with the maximum over the same corners.whenarms, not Compose$changedbookkeeping or getters. New files includeStandardDiceTest,VerifyDeviceTestResultsTaskTest(the only verdict on device runs),WaysBetweenScreensTest,ChosenFilesTest,RollWiringTest,GraphChartTest,EarnedThrowTest,TrayPlanInkTest,TrayLoopEndingsTestandTimeStampTest.StatsScreenTest,HistoryScreenTestandTrayLoopTestreached detekt'sLargeClasslimit. Their session, forget and stall tests moved to files of their own; no test was lost in the move.RollWiring.table(pinned)went fromprivatetointernal, so the table precedence rule (pin → chosen → bundled → fallback) can be tested without a Jolt run.docs/STATUS.mdanddocs/TODO.mdrecord the new coverage figures, and list the problems the pass found.Coverage (JaCoCo, merged JVM + Robolectric; base
3a342c5d→ this branch)No module drops on either counter.
Per module
Most of the branches still missed are Compose skip branches, plus the device-only bridges (
FilamentStage,TrayDriver,FilamentEngine,JoltWorld), which were deliberately not faked into coverage.Found, not fixed (now in
docs/TODO.md)These are kept out of this PR so that no new test locks in a wrong answer:
SafeExtractorextracts tar device and FIFO entries as empty files.TarArchiveEntry.isFileis true for anything that is not a directory, so the documented refusal never fires for them. This is not exploitable, but it is a validator gap.InstallSource.giteadrops a subfolder and installs from the repository root.GraphMachine.redraw()resetstruncatedMassto 0.ReferencedFile.staysInsidePackagerefuses a path with a trailing/.Checks
./gradlew test coverageReport detekt ktlintCheck lint: green in the devcontainer, no new warnings.build-logic's own tests: green.🤖 Generated with Claude Code