Skip to content

fix(ci): the producer integration test list is data, not part of the pinned selection logic - #6467

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/integration-list-not-hashed
Oct 26, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
fix/integration-list-not-hashed

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What was wrong

scripts/test-reachability.json pins a sha256 of packages/producer/scripts/test-classification.mjs, which held both the lane-selection logic and the data list of integration tests. Every PR that adds an integration test edits the list, so it changes the sha and conflicts with every other such PR (#6441, #6444 and #6455 all hit it).

Fix

The list moves to packages/producer/scripts/integration-tests.json and the module reads it. The pin stays on the selection logic only, so a changed selection still forces review while adding a test to the list no longer touches the pinned file. The list is checked directly, not hashed: discoverProducerTests already fails when an entry names a file that is not a test under src or tests.

Design calls:

  • JSON beside the module, not a second pinned source: a hashed list would bring the conflict back.
  • No "every ffmpeg-spawning test is listed" rule: four unit-lane tests already spawn ffmpeg by a text match (some mock or skip it), so the rule would fail on main without a call on each one. Left for its own change.

Test

test-classification.test.mjs fails on main's module (its source holds test paths) and passes here, and checks the list has no duplicates. The classifier output is unchanged (157 tests, the same lanes) and check-test-reachability verifies zero orphan tests with the re-pinned sha.

Why under 100 lines

CI tooling that no other change touches; nothing to fold into. The diff is the moved list plus a one-line pin.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 blockers · 0 important · 0 nits · head 0484fe2f

Scope

Checked the JSON list against origin/main's inline set (now at 3e16603d): the same 37 entries. Locally, test-classification.test.mjs passes 6/6, the classifier output matches main (157 tests: unit 49 bun + 71 vitest, integration 15 + 22), check-test-reachability reports zero orphans, and sha256 of the module equals the new pin. The first Windows run failed in chromeOrphans.process.test.ts and browserManager.test.ts, which this diff doesn't touch. The same chromeOrphans test also fails on other branches and in the merge queue (pr-5255). I re-ran the failed jobs and attempt 2 passed. All CI on this head is now green.

— Genesis

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 9b339e3 Oct 26, 2026
78 of 80 checks passed
@miguel-heygen
miguel-heygen deleted the fix/integration-list-not-hashed branch October 26, 2026 20:22
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.

1 participant