One conformance corpus and one definitions contract for both packages - #255
Open
jat255 wants to merge 4 commits into
Open
One conformance corpus and one definitions contract for both packages#255jat255 wants to merge 4 commits into
jat255 wants to merge 4 commits into
Conversation
…ckages The 14 YAML fixtures both packages need move from pkg-r's test fixtures to tests/shared/, so there is one copy rather than a second hand-maintained one in pkg-py. The R suite reads the synced copy, as it does for every shared fixture. tests/shared/definitions.json pins what both packages agree to consume: the export-record contract, the grain metadata call_metrics needs for its mixed-grain guard, and the data-dict problem code each invalid fixture must produce. export_records is generated from the data-dict binary at the pinned commit and the generator refuses to run against any other build, since a fixture from a different revision would bless whatever that build does. mixed_grain comes from the typed IR rather than the export, so it is hand-maintained and the generator preserves it. The fixture does not replace the conformance harness. That harness compares against a real binary; this pins the contract.
…ed binary The fixture landed with no runner exercising export_records or mixed_grain, so most of it could drift without either package noticing. tests/shared/README.md asks for runners to land with a fixture for exactly this reason. R can assert it now, and does: its export contract and its grain metadata are compared against the fixture for every valid case. Python joins when its compiler exists; until then its runner checks the fixture's own integrity. Comparison sorts both sides. The generated file sorts its keys so diffs stay readable, while the export keeps authored order, and this fixture is a keyed contract rather than a sequence. The generator checked the cargo installation and then ran whichever data-dict PATH resolved, so a different binary could have generated the fixture while the check passed. It now invokes the cargo-installed path directly. Regenerating produced a byte-identical fixture.
grep -q closes the pipe on its first match, so with pipefail set cargo can die of SIGPIPE and fail the pipeline, rejecting a correctly pinned install. grep -c reads the full stream.
`|| true` covered the whole pipeline, so cargo failing after emitting a matching line would have counted as verification. The listing is captured and its status checked first; the tolerated failure is now only grep's no-match.
|
Preview deployed to Connect ( Deployed from commit 7faab7f. |
|
Preview deployed to Connect ( Deployed from commit 7faab7f. |
jat255
marked this pull request as draft
September 2, 2026 04:01
jat255
marked this pull request as ready for review
September 2, 2026 23:48
Collaborator
Author
|
@simonpcouch this one's ready for a quick look on the R side. No changes to actual running code. Like the last few PRs, it's a restructuring of the test cases into shared fixtures that can be used on the python side. Thanks! |
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.
Summary
Fourth PR of M2 (data layer). Gives the two packages a single shared copy of the data-dict definition fixtures — 14 YAML files of valid and invalid table definitions written in data-dict's expression language, which both test suites use to check their exporters against the reference
data-dictbinary — and a shared fixture (tests/shared/definitions.json) pinning the expected export for every definition, so both packages are held to the same contract. Also includes some groundwork for the definitions registry and the compiler, which land next.The definition fixtures move out of
pkg-rThe 14 YAML fixtures were moved out of
pkg-r/tests/testthat/fixtures/to now live intests/shared/definition-export/. Both suites read the same files.definitions.jsonThis file pins behavior on three things, in three different sections within the file:
export_records— the expected export for each of the 42 valid definitions: its SQL translation, its inferred kind and type, and the columns and definitions it references. Generated from the realdata-dictbinary at pinned commitd950c5a; the binary exports more than commons needs, so the generator keeps only the fields both packages consume (the "projection"). Regenerate withscripts/generate-definitions-fixture.shrather than editing by hand.mixed_grain— a per-definition boolean: true when a definition's exported shape isrowbut its expression tree contains an aggregate, directly or through a definition it references, which is the combinationcall_metrics' mixed-grain guard rejects. The flag is absent from data-dict's export — it exists only in the typed IR, data-dict's internal type-annotated parse tree — so it cannot be generated from the binary. It is hand-maintained (computed with the R implementation's grain logic) and the generator preserves it. One of the 42 definitions is mixed-grain, and a test asserts the values are not all identical: an all-false fixture would pass against an implementation that always answers false, pinning nothing.invalid— maps each of the 11 invalid fixtures to the specific data-dict problem code a conforming compiler must report for it (cycle.yamlmust fail with the cycle error, not a generic parse failure), and both suites assert on that code. Replaces a literal map hard-coded in the R helper, which Python could not share.The generator refuses to run against the wrong binary
scripts/generate-definitions-fixture.shresolves the cargo-installed binary by path rather than trustingPATH, and verifies that installation is the pinned revision. A fixture generated from another build would bless whatever that build does, in a way nothing downstream could detect.Only R asserts the contract so far
There are no tests on the Python side for the actual data-dict definition contract in this PR (since that implementation does not exist yet). The only python tests are for the mechanics of loading the definition fixtures.
Verification
219 R assertions, none skipped. The new R assertions were then checked by perturbing one translation and one grain value in the fixture and confirming the suite fails.
Summary of R changes
No package code changed. Everything here is under
pkg-r/tests/, so no exported or internal behaviour moves. Files underpkg-r/tests/testthat/fixtures/shared/are generated byscripts/sync-shared-fixtures.sh.What changed and why. The definition-export fixtures used to live inside the R package's test fixtures, where only R could reach them. Python needs the same files to check its compiler against the same cases, and a second copy would drift, so the fixtures moved to the repository root and R now reads the synced copy under
fixtures/shared/, exactly as it already does for the provenance and citation fixtures. Alongside that, the map from an invalid fixture to its expected data-dict problem code moved out of the helper and into the shared fixture, because it is a fact about the shared cases rather than about R.