Skip to content

test(next-js): fix flaky deleteIndex test race - #550

Closed
juangl wants to merge 1 commit into
mainfrom
juan/fix-next-js-delete-index-test-race
Closed

juangl wants to merge 1 commit into
mainfrom
juan/fix-next-js-delete-index-test-race

Conversation

@juangl

@juangl juangl commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

  • next-js-test failed on Remove Codex PR review workflow #549 (an unrelated workflow-only change). The failing test was deleteIndex > shows an error message when deleting the index fails. At failure time, the button's accessible name was Deleting Index.
  • Cause: deleteIndex runs in an async useTransition. The state updates after the await commit before isPending clears. That leaves one render where the error or idle UI is visible while the button still reads "Deleting Index" and Build is still disabled. The two delete tests asserted right after the first waitFor passed, so they could land on that render.
  • Fix: both tests now wait for the delete button label to settle before asserting. This is a test-only change. The component is unchanged.

Test plan

  • Reproduced on the unchanged branch: 3 of 12 full-file runs failed when 12 ran in parallel. Both delete tests hit the race.
  • With the fix: 36 of 36 full-file runs passed when run in batches of 12 in parallel.
  • eslint app/page.test.tsx: no new warnings.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

Summary by CodeRabbit

  • Tests
    • Updated deletion tests to wait for the expected post-deletion state and verify that deletion errors appear alongside an enabled Delete Index button.

The delete action runs in an async useTransition. Its state updates after
the await commit before isPending clears, so for one render the error/idle
UI is visible while the button still reads "Deleting Index" and the build
button is still disabled. The tests asserted on that intermediate render
and failed intermittently on loaded CI runners.

Wait for the delete button label to return before asserting on it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Team
  • Run ID: e6f689a4-99da-44cc-b68f-9a3ef6abe576
📥 Commits

Reviewing files that changed from the base of the PR and between 7bb9c1e and 8276be9.

📒 Files selected for processing: 1
  • apps/next-js/app/page.test.tsx

Included review availability: This review used your included allowance. 5 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

📝 Walkthrough

The index deletion tests now wait for the Delete Index button to return after deletion and to be enabled after a deletion failure.

Changes: Index deletion tests

Layer / File(s) Summary
Deletion wait conditions
apps/next-js/app/page.test.tsx
The deletion helper waits for “Build first” and the Delete Index button. The failure test waits for the error and an enabled Delete Index button. It still checks that the ready state is absent and Load Index remains enabled.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 8276b

The tests now wait for the settled button state on both deletion outcomes. The change presents no identified merge-specific risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the test-only change and the flaky Delete Index test race.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 1 file

View guided diff | Turn on auto-fix | Re-trigger cubic

@juangl juangl closed this Oct 16, 2026
@juangl
juangl deleted the juan/fix-next-js-delete-index-test-race branch October 16, 2026 00:11
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