[codex] test(ui): wait for Watchdog menu item - #1314
Conversation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 6498f4c
Looks good. The test now waits for the observable Watchdog menu item instead of assuming one flush is enough. The assertion remains bounded by the existing retry helper, and production behavior is unchanged.
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The change is narrowly scoped to the asynchronous UI condition that the test actually depends on.
- The existing helper preserves a useful failure signal if the menu item never appears.
Recommended Action
- Merge when the remaining required checks pass.
allyblockcast
left a comment
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 6498f4c
Looks good. The test now waits for the observable Watchdog menu item instead of assuming one flush is enough. The assertion remains bounded by the existing retry helper, and production behavior is unchanged.
Critical Issues (0)
Important Issues (0)
Suggestions (0)
Strengths
- The change is narrowly scoped to the asynchronous UI condition that the test actually depends on.
- The existing helper preserves a useful failure signal if the menu item never appears.
Recommended Action
- Merge when the remaining required checks pass.
|
Merge exception evidence for exact reviewed head 6498f4c:
Proceeding with an admin squash merge under the requested infrastructure/baseline exception. |
Thinking Path
Linked Issues or Issue Description
Type: Bug / test reliability
What happened:
NewIssueDialog.test.tsxcould fail to find the Watchdog menu item after a singleflush()even though the async settings query subsequently rendered it.Expected behavior: The test should wait for the observable UI state that it asserts rather than assume one event-loop turn is sufficient.
Reproduction: Run
pnpm exec vitest run ui/src/components/NewIssueDialog.test.tsxunder a schedule where React Query's notification follows the test's one-tick flush. The prior failure was observed in theworkspaces-aCI shard while validating #1031.Version / deployment mode: Current
mastercheckout; local Vitest component-test execution. No production deployment behavior is affected.Related work: #1031 exposed the baseline test race. A search across the open PR heads found no existing PR that changes this test or its component.
What Changed
waitForAssertionhelper.Verification
pnpm exec vitest run ui/src/components/NewIssueDialog.test.tsx— 26 passed.Risks
Low risk. This is a test-only synchronization change. It could conceal a permanently absent item only until the helper timeout, after which the same assertion still fails with the relevant UI state.
Model Used
OpenAI Codex (GPT-5), tool-using coding agent with reasoning enabled; used for diagnosis, patch generation, and focused test execution.
Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template