Skip to content

test: stop asserting the candidate order in the sweep warn-once bundle test - #637

Merged
mrbobbytables merged 1 commit into
cncf:mainfrom
mrbobbytables:test/merge-queue-warn-once-order
Oct 31, 2026
Merged

mrbobbytables merged 1 commit into
cncf:mainfrom
mrbobbytables:test/merge-queue-warn-once-order

Conversation

@mrbobbytables

@mrbobbytables mrbobbytables commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Description

The first dispatch of test.yml on main after #635 (run 41154250262) passed build-test, mutation (first live run of the nightly config, score above the 95 break), e2e-coverage and chart, but shuffle failed in __tests__/bundle/mergeQueueWarnOnce.test.ts:

Expected: "could not read the merge queue state of pr #1"
Received: "::warning::could not read the merge queue state of pr #2; falling back to a direct merge: ..."

The sweep evaluates its candidates concurrently, so which candidate's queue read fails first (and so is the one named in the once-per-run warning) and which merge PUT lands first are both races. The test's own comment already says the two tails "interleave in no fixed order", but two assertions still pinned the order. This is not seed-dependent; running the suite twice per pull request just doubles the chance of hitting it.

This matches the warning against either candidate and compares the merge PUTs sorted. The behaviour under test is unchanged: exactly one warning for the run, no enqueue, both candidates merged directly.

No tracking issue; the failure is in the linked run and the fix is a two-line assertion change.

Testing

Modified __tests__/bundle/mergeQueueWarnOnce.test.ts only.

  • npx eslint __tests__/bundle/mergeQueueWarnOnce.test.ts — passed
  • npx vitest run __tests__/bundle/mergeQueueWarnOnce.test.ts ×5 — passed each time
  • npm run lint — passed (one pre-existing jsdoc/check-param-names warning in src/issueComment/cc.ts, also present on main)
  • npm run typecheck — passed

Checklist

  • I ran npm run all to lint and build my code — ran lint, typecheck and the changed test directly instead; no src/ or dist/ change, so build/pack are not affected and CI's dist-drift gate covers that
  • I ran npm run pack and committed dist/ — not applicable: test-only change, dist/ untouched
  • Commits are signed off (DCO)
  • Note any new dependencies these changes bring in — none
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation — not applicable
  • My changes generates no new warnings (please describe new warnings if unavoidable)

@mrbobbytables
mrbobbytables requested a review from jpmcb as a code owner October 31, 2026 01:20
…e test

The sweep evaluates its candidates concurrently, so which pull request's
queue read fails first, and which merge PUT lands first, varies from run
to run. The test already said as much in its own comment but still pinned
the warning to pr cncf#1 and the merge PUTs to [1, 2]. The new shuffle job
caught it on the first main dispatch (run 41154250262): the warning named
pr #2.

Match the warning against either candidate and compare the merge PUTs
sorted. The behaviour under test (one warning for the run, both
candidates merged directly) is asserted exactly as before.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Bob Killen <bkillen@linuxfoundation.org>
@mrbobbytables
mrbobbytables force-pushed the test/merge-queue-warn-once-order branch from a87c58e to 121ec5e Compare October 31, 2026 01:20
@github-actions

Copy link
Copy Markdown
Contributor

Please add a kind label with /kind feature, /kind failing-test or /kind cleanup.

@mrbobbytables
mrbobbytables merged commit 424fb95 into cncf:main Oct 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant