Skip to content

test: close the coverage gaps outside the transformer (98.94% -> 99.25%) - #423

Open
tschm wants to merge 1 commit into
janushendersonassetallocation:masterfrom
tschm:test/close-remaining-coverage-gaps
Open

test: close the coverage gaps outside the transformer (98.94% -> 99.25%)#423
tschm wants to merge 1 commit into
janushendersonassetallocation:masterfrom
tschm:test/close-remaining-coverage-gaps

Conversation

@tschm

@tschm tschm commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Follows #422. Together the two take the suite from 98.94% to 99.94%, with three lines left deliberately.

Fifteen lines across four modules. Each was picked because it is shipped behaviour that nothing held, not because it was a branch that happened to be missed.

MemoryBlobStore had no test at all

This is the one worth reading twice. MemoryBlobStore is exported from loman.serialization, its own docstring calls it "a worked example of the interface", and docs/paper/loman.tex:652 names it alongside DirBlobStore and ZipBlobStore as one of the three stores that ship.

grep MemoryBlobStore tests/ returned nothing. The suite has always brought its own store — RecordingStore in test_byos.py, written deliberately "the way a user would implement one" — so the store users are actually pointed at was never exercised, including the SerializationError its read_blob raises on a missing key.

Now covered: construction with and without a backing dict, the round-trip, the missing-key error, and a real end-to-end save/load routing a node's values through it.

The rest

  • _require_parent — saving under a directory that does not exist names that directory. The 0.7.0 changelog promises this, and specifically promises the error does not name the sibling .tmp the containers build into. Both halves are now asserted, for the zip and dir containers.
  • Named timezones== on two aware datetimes compares instants, so a round-trip that quietly replaced Europe/London with a fixed +00:00 would still have passed the AWKWARD_VALUES table. The new table entry covers the encode and decode paths; three explicit tests assert the zone key survives, which is the part equality cannot see.
  • put_blob with raw bytes — both the bytes and writer-callable forms are documented on put_blob; only the callable one was reached, because that is what the built-in transformers use.
  • write_dill_old — applying the __serialize__ tag rule, and writing to an open file object. Worth knowing: the deprecated path implements that rule a second time, because it has deleted __getstate__ for the duration of the write. Only the other copy was covered, so the two could have drifted silently.

Result

Before After this PR With #422 too
computeengine.py 99% 100% 100%
serialization/blobs.py 95% 100% 100%
serialization/values.py 98% 99% 99%
Suite total 98.94% 99.25% 99.94%
Uncovered lines 51 36 3

The three lines left, and why

ui/widget.py:947 (_select_key returning early with no view), ui/widget.py:1008 (the build-request early return) and serialization/values.py:485 (_reapply_freq returning unchanged when freq is None).

All three are reachable, so 100% is available. They are left because a test for each asserts little beyond "the guard is a guard", and because taking the gate to --cov-fail-under=100 has a cost worth deciding separately: this codebase carries 35 considered # pragma: no cover markers, and a 100% bar turns every future defensive branch into a choice between a tautological test and a pragma. 99.94% with three honest gaps reads truer than 100% leaning harder on suppressions. Happy to close them if you'd rather have the round number.

Verification

Gate Result
make fmt PASS — 21/21 hooks
make test PASS — 1557 passed, coverage 99.25%

Fifteen uncovered lines across four modules, chosen because each is
shipped behaviour that nothing held rather than a branch that happened
to be missed.

MemoryBlobStore was the notable one. It is exported from
`loman.serialization`, its docstring calls it "a worked example of the
interface", and docs/paper/loman.tex names it alongside DirBlobStore and
ZipBlobStore -- but the suite had always brought its own RecordingStore,
so the store users are actually pointed at had no test at all, including
its SerializationError on a missing key.

Also covered:

- `_require_parent`: saving under a directory that does not exist names
  that directory. The 0.7.0 changelog promises this, and specifically
  promises it does not name the sibling `.tmp` the containers build
  into; both halves are now asserted, for the zip and dir containers.
- Named timezones. `==` on aware datetimes compares instants, so a
  round-trip that quietly replaced Europe/London with a fixed +00:00
  would still pass the AWKWARD_VALUES table. The added table entry
  covers the encode and decode paths; three explicit tests assert the
  zone key itself survives, since that is the part equality cannot see.
- `put_blob` handed raw bytes rather than a writer callable. Both forms
  are documented; only the callable one was reached, because it is what
  the built-in transformers use.
- `write_dill_old` applying the `__serialize__` rule and writing to an
  open file object. The deprecated path implements that rule a second
  time -- it has deleted `__getstate__` for the duration of the write --
  and only the other copy was covered.

computeengine.py and serialization/blobs.py reach 100%; the suite total
goes from 98.94% to 99.25%, and 51 uncovered lines to 36. The remaining
36 are transformer.py's 33, covered separately in janushendersonassetallocation#417, and three
guard-clause lines left deliberately: `_select_key` with no view,
the build-request early return, and `_reapply_freq` with no frequency.

Refs janushendersonassetallocation#417

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