test(packaging): cover native manifest parse refusals - #419
Conversation
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging. All four parse-stage refusals are the only failure when their own raise goes:
not-valid-JSON refusal removed test_apply_refuses_invalid_manifest_json_before_mutation
artifacts-must-be-an-array removed test_apply_refuses_non_array_artifacts_before_mutation
not-canonical-JSON removed test_apply_refuses_semantically_identical_noncanonical_manifest...
duplicate-fields removed test_apply_refuses_duplicate_manifest_fields_before_mutation
The canonical-JSON test is the one I was most interested in and you built it correctly:
assert json.loads(noncanonical_bytes) == payload
assert noncanonical_bytes != manifest_path.read_bytes()Asserting both halves is what makes it a statement about form. Without the first assertion it could be refusing malformed content and passing for the wrong reason, which is the failure mode that makes a serialization test worthless. Same for the duplicate-field case asserting json.loads(...)["format"] == payload["format"]: it proves the document parses to the right value and is still refused, which is the whole point of a duplicate-key guard.
Every case asserting source bytes, artifacts, installed manifest, and RECORD are all untouched keeps these about the trust boundary rather than about exception types.
One thing to fix later, not now. _write_example_distribution and _example_record_path are now duplicated between this file and tests/test_packaging.py, and the two copies already disagree: yours returns package_root, the original returns a tuple. That is drift at birth rather than drift over time, and if the sample distribution ever grows a module, one copy will update and the other will quietly test a different fixture. I have filed it as #420 rather than reworking your PR, since the fix is a judgment call about where shared test fixtures should live in this repo and that is worth deciding once.
Gate on the merged result: ruff check, ruff format --check, ty check clean, 1569 passed / 6 skipped.
Closes #408.
Summary
Add focused regression coverage for the four native-overlay manifest refusals that occur before
_validate_manifest:artifactsfield;Each case builds a real native overlay, corrupts only
manifest.json, calls the public apply path, and verifies that package source bytes, native artifacts, the installed manifest, and wheelRECORDremain untouched.Closes #408.
Why
These checks protect the untrusted manifest boundary. The canonical-JSON and duplicate-field cases specifically pin the exact serialized document that integrity metadata is intended to describe: a payload can decode to the same Python value while still not be the canonical/signed byte representation.
The canonical case therefore writes raw, semantically identical JSON using a different serialization. The duplicate-field case also writes raw bytes because a Python dict cannot represent repeated keys.
Validation
GitHub Actions is green on the current head (
26a500a), including Python 3.11/3.14 checks and both workspace-example matrices. The initial 3.14 lint feedback was a trailing-newline-only issue in the new test file and was corrected in the current head.Repository gates exercised by CI include lint, format checking, type checking, and tests.
This supersedes #416, which was automatically closed only because the repository permits one open contributor PR at a time while #415 was under review. #415 has since merged.
AI assistance disclosure: AI assistance was used to inspect the issue and repository patterns, prepare the focused regression tests, diagnose CI feedback, and review the resulting change.