Skip to content

Generate dist in CI and validate against generated output - #968

Open
KaiKitJeffreyChan wants to merge 1 commit into
mainfrom
jc/ci-generate-dist
Open

Generate dist in CI and validate against generated output#968
KaiKitJeffreyChan wants to merge 1 commit into
mainfrom
jc/ci-generate-dist

Conversation

@KaiKitJeffreyChan

@KaiKitJeffreyChan KaiKitJeffreyChan commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

CI currently validates dist/ files committed to git. Once dist is published as release assets (#957), CI should generate dist/ itself and validate the generated output instead. Part of shop/issues-taxonomy#544.

What changed

  • tests.yml: adds an explicit Generate distribution files step to the test_integration and vet_schema matrix jobs. The step removes the checked-out dist/ tree (so checks can never read committed or stale files) and regenerates en from data/ and the current VERSION. Unit-test jobs skip it. Generated files are ephemeral job inputs — no flattening, gzip, or release writes (publication stays in the stable/unstable asset workflows).
  • Removes distribution_matches_data_test.rb: comparing regenerated output against a committed dist/ baseline is obsolete once generated files are no longer versioned.
  • Removes the /generate_dist commit-back workflow: superseded by generation-in-CI. It was already broken — it invokes make --file=Makefile build, and the Makefile was deleted in 354723b.

Gitignoring dist/ and rejecting PRs that touch it is deliberately not part of this PR — that's the final cutover task, which can't land while dist/ is still committed.

Validation

Full workflow run on a fork: all 4 jobs green.

  • test_integration / vet_schema logs show rm -rf ../dist → generation from VERSION → checks passing against the fresh tree (11 runs, 527791 assertions, 0 failures).
  • test_unit skips the generation step.

Known gaps / follow-ups

  1. Committed dist/ can drift until the cutover (shop/issues-taxonomy#546). With distribution_matches_data_test removed, no check compares the committed dist/ against data/ — a data PR that skips regeneration merges green and main's dist/ stays stale until the next release (GenerateReleaseCommand regenerates all locales). This only affects consumers reading dist/ from main rather than from releases. Open question for reviewers: is main-branch dist/ a consumption surface we support during the interim? If yes, I'll re-add the deleted test as a non-blocking job (continue-on-error: true, same shape as orphan_detection) until Remove old tooling #546 lands.
  2. MANIFEST_KEY repo secret is now orphaned. The removed workflow was its only consumer (it keyed-BLAKE3-signed the artifact handed between its own two jobs). Deleting the secret from repo settings loses nothing — will remove it after this merges.
  3. Cutover prerequisite logged on Remove old tooling #546: generate_release_command.rb:81 version-badges dist/README.md via File.read. When dist/ is deleted/gitignored at cutover, that line must be removed in the same change or the release command crashes with Errno::ENOENT.
  4. Post-merge verification: the Add "Other" entry as default base color to Color attribute #544 acceptance criterion ("representative data/ change runs green through the new CI") is demonstrated on a fork above but not yet on this repo — the first data/-touching PR after merge confirms it.

- Add an explicit 'Generate distribution files' step to the integration
  and schema-check matrix jobs. The step removes the checked-out dist/
  tree and regenerates en from data/ and VERSION, so checks validate
  generated output instead of committed files. Unit tests skip it.
- Remove distribution_matches_data_test.rb: comparing regenerated output
  with a committed dist/ baseline is obsolete once dist/ is unversioned.
- Remove the /generate_dist commit-back workflow (already broken: it
  invoked a Makefile deleted in 354723b).

Part of shop/issues-taxonomy#544

Co-authored-by: AI (Pi/Claude Fable 5) <noreply@pi.dev>
Assisted-By: devx/bf7ee413-9453-4826-b1dc-8893f26eb97d
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