One conformance corpus and one definitions contract for both packages - #255
Draft
jat255 wants to merge 4 commits into
Draft
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
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
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.
Fourth PR of M2. Gives the two packages a single copy of the data-dict conformance corpus and a shared fixture pinning the definitions contract they both consume. Groundwork for the definitions registry and the compiler, which land next.
The corpus moves out of
pkg-rThe 14 YAML fixtures live in
tests/shared/definition-export/now instead ofpkg-r/tests/testthat/fixtures/. Both suites read the same files. The alternative was a second hand-maintained copy underpkg-py/, which is the kind of drifttests/shared/exists to prevent.What
definitions.jsonpinsThree things, with different provenance, which is why they are separate sections:
export_recordsis generated from thedata-dictbinary at the pinned commitd950c5a, projected to the fields both packages consume.mixed_grainis not in data-dict's export at all. It is derived from the typed IR and is whatcall_metricsneeds for its mixed-grain guard, so it is hand-maintained and the generator preserves it. One of the 42 definitions is mixed-grain, and a test asserts the values are not all identical, since an all-false fixture would pin nothing.invalidmaps each invalid fixture to the problem code it must produce, replacing a map that was hard-coded in the R helper.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.That check took three review rounds to get right: it trusted
PATHwhile verifying cargo, thengrep -qrisked killing cargo with SIGPIPE underpipefail, then|| truemasked cargo failing outright. Regenerating after each change produced a byte-identical fixture.One contract detail worth knowing
data-dictomitstypeentirely when an expression infers no single one, such as aCASEover both a date and a datetime column. R does the same. My runner originally asserted every record has a type, which was wrong; it now pins the typeless case by name, because the obvious implementation invents a type rather than leaving it out.Verification
284 Python tests and 219 R assertions, none skipped, so the R conformance tests genuinely ran against the pinned binary rather than skipping as they do without it. The new R assertions were checked by perturbing one translation and one grain value in the fixture and confirming the suite fails.
R changes, for the R reviewer
No package code changed. Everything here is under
pkg-r/tests/, andpkg-r/R/is untouched, so no exported or internal behaviour moves. Files underpkg-r/tests/testthat/fixtures/shared/are generated byscripts/sync-shared-fixtures.sh; skip them.What changed and why. The definition-export corpus used to live inside the R package's fixtures, where only R could reach it. Python needs the same files to check its compiler against the same cases, and a second copy would drift, so the corpus 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 corpus rather than about R.The behaviour delta. For existing tests, none: the same files are read from a different path, and every assertion that passed before passes now. What is new is that the R suite asserts the shared contract. For each valid fixture,
definition_export_spec()'s contract projection and its mixed-grain result are compared againstdefinitions.json. That means a change to the R exporter which alters a translation, an inferred kind or type, a reference list, or a grain flag now fails the R suite rather than passing silently. That is the point: the fixture claims to be authoritative, and until this it was asserted by nobody.Blast radius.
definition_fixture_paths()changed its path and is called nine times acrosstest-definition-export.R.definition_fixture_error_code()changed its source from a literal map to the shared fixture and is called once. Six directtest_path("fixtures", "definition-export", ...)calls intest-definition-export.Randtest-definition-compile.Rnow go through a newdefinition_fixture_path()helper, so the next move touches one line rather than eight.definition_fixture_contract()anddefinition_export_grain()are new and used only by the two new tests. Nothing outsidetests/testthat/refers to any of them.Why it is safe, and what was checked.
devtools::test_local(filter = "definition")reports 219 assertions, zero failures, zero skips. The zero-skip count matters: these tests skip themselves when thedata-dictbinary is absent, so a green run without it would prove nothing, and this run exercised the real binary built from the pinned commit. The two new assertions were then checked by mutation rather than by inspection, perturbing a translation string and a grain flag in the synced fixture and confirming the suite failed, then restoring it and confirming it passed.What deserves scrutiny. The comparison sorts both sides by key.
definition_export_contract()preserves authored order while the generated JSON sorts its keys for readable diffs, and I decided this fixture pins a keyed contract rather than a sequence. If you think the order definitions come back in is itself observable, that decision is wrong and the fixture should preserve authored order instead. Also worth a look isdefinition_fixture_contract(), which normalizes the JSON side to the R shape: it flattens each list to a character vector so that an empty sequence compares equal whether it arrived ascharacter(0)orlist(). That normalization could in principle hide a real difference in how emptiness is represented, which I judged not worth pinning.Tests deleted. None. The only deletion is the hard-coded error-code map in
helper-data-dict.R, which the shared fixture now supplies to both suites.