Skip to content

Cover the typed batch payloads with offline tests and typechecking - #47

Merged
vigneshwerv merged 4 commits into
typed-batch-ledger-entriesfrom
claude/typed-entries-test-coverage
Aug 10, 2026
Merged

Cover the typed batch payloads with offline tests and typechecking#47
vigneshwerv merged 4 commits into
typed-batch-ledger-entriesfrom
claude/typed-entries-test-coverage

Conversation

@snoble

@snoble snoble commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Written by Claude, running in Steven's session — these are Claude's words and judgements, not Steven's.

Stacked on #45 (base typed-batch-ledger-entries). Tests and typechecking only — no behaviour changes. #45's five review items are all resolved there; this fills the coverage holes flagged alongside them.

What was uncovered

Nothing imported the generated per-entry classes. The existing tests assert on render_module's output as a string, and the sample fixture builds a model by hand from BASE_CLASS_SOURCE. A rendered module that declared the same field twice, or referenced a name the header does not import, satisfied all of them.

The optional-parameter branch had no coverage at all. Every CLI-generated parameter is non-null, so no snapshot query reaches the renderer's = None path.

The warning paths had none either. Each leaves a working but quietly worse SDK — a parameter that lost its type, a batch method that does not typecheck, models nothing accepts. A silent version of any of them passed the whole suite.

CI ran mypy -p fragment only. tests/ was never typechecked, so the mypy_path setting added in #45 was dead config, and the PR's central claim — that a caller gets real type errors — was verified by nothing.

On forks, no test ran at all. The only job that ran pytest needs secrets, and conftest.credentials fails the run without them.

What this adds

File Covers
tests/test_typed_entries_generated.py Renders into a throwaway package, imports it, uses the classes. Optional parameters, field collisions, escaped names, to_entry_inputs, the empty-module shape that fragment/sdk/typed_entries.py actually is, and every model in the committed snapshot.
tests/test_typed_entries_warnings.py All five degradation paths, plus the silent counterpart of each so the assertions mean something.
tests/type_checks/batch_entries.py mypy-only. Positive cases must pass; negative cases are # type: ignore[...], and warn_unused_ignores fails the day one starts passing.

make typecheck now covers tests/ too, make unit runs everything offline, and a new credential-free CI job runs both.

76 offline tests, 0.2s, no network.

Verification

Every new test was checked by breaking the thing it guards:

Mutation Result
Revert the field-collision fix 5 fail
Drop = None for optional parameters 5 fail
Emit the escaped Python name as the wire key 3 fail
Drop typeVersion from to_input 1 fails
Drop the list(entries) coercion 2 fail
Loosen entries to Any 4 unused-ignore errors
Narrow Sequence back to list 3 type errors

The last two are the point of type_checks/: it fails both when the signature loosens and when it tightens too far.

One-line change to #45's files

pytestmark = pytest.mark.integration in the two integration modules. Their snapshot_client fixture is untouched, including the comment about **credentials — that stays accurate, since the fixture still annotates Dict[str, str].

🤖 Generated with Claude Code

@snoble

snoble commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Claude, in Steven's session.

Correction to the original description: I claimed make lint was unrunnable and rewrote ~1000 lines. That was wrong, and I've removed it.

The churn came from my own environment using isort 8.0.1 where poetry.lock pins 5.13.2. With the pinned versions, make lint is a no-op on clean dev and on this branch. isort does reformat imports and black does put them back, so the intermediate state churns, but the pair converges and the committed tree is already at the fixed point.

vigneshwerv and others added 2 commits August 10, 2026 15:09
The typed-entry tests assert on render_module's output as a string, which
cannot tell a working module from one that merely contains the right
substrings. Nothing imported the per-entry classes, so a duplicated field
declaration or an annotation the header does not import passed every test.

tests/test_typed_entries_generated.py renders into a throwaway package and
imports it, then uses the classes: the optional-parameter branch (no snapshot
query has a nullable parameter), field-name collisions, escaped names, and
to_entry_inputs. It also exercises every model in the committed snapshot,
which is the artifact a customer gets.

tests/test_typed_entries_warnings.py covers the paths where codegen degrades
rather than fails. Each leaves a working but quietly worse SDK, and the
warning is the only signal, so a silent version of any of them passed the
whole suite.

tests/type_checks/ asserts what a caller sees, which no runtime test can:
a runtime test passes just as happily against `entries: Any`. Negative cases
are written as `# type: ignore[...]` and warn_unused_ignores makes them fail
if the call ever starts passing, so the signature cannot loosen unnoticed.

CI ran mypy -p fragment only, leaving both tests/ and the mypy_path setting
unchecked. `make typecheck` now covers tests/ as well, and typecheck plus the
offline tests run in a job with no secrets -- on forks the credentialed job
errors on every test, so nothing ran at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vigneshwerv
vigneshwerv force-pushed the claude/typed-entries-test-coverage branch from fbcd5fe to 1a1baa4 Compare August 10, 2026 19:13
Merging typed-batch-ledger-entries dropped the `pytestmark` line from
tests/test_add_ledger_entries.py. Nothing failed loudly: `make unit`
deselected 1 test instead of 3, ran the two credential-bound tests offline,
and they errored on missing environment variables.

conftest now marks anything whose fixture closure includes `credentials`,
so the marker follows from what a test actually needs. A new integration
test cannot forget it and a merge cannot drop it. The per-module markers
are redundant under that rule and are removed, leaving one mechanism.
@vigneshwerv
vigneshwerv merged commit 4708848 into typed-batch-ledger-entries Aug 10, 2026
3 checks passed
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.

2 participants