Skip to content

fix(e2e): add race condition guards to checkpoint UI tests - #262

Merged
frankbria merged 1 commit into
mainfrom
fix/e2e-checkpoint-race-condition
Jan 13, 2026
Merged

fix(e2e): add race condition guards to checkpoint UI tests#262
frankbria merged 1 commit into
mainfrom
fix/e2e-checkpoint-race-condition

Conversation

@frankbria

@frankbria frankbria commented Jan 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fix race condition in 4 checkpoint UI tests where count() was called before React rendered
  • Tests now wait for checkpoint items OR empty state to be visible before checking count
  • Improves test reliability from intermittent failures to consistent passes

Changes

Added await expect(checkpointItems.first().or(emptyState)).toBeVisible() before count() calls in:

  • should show restore confirmation dialog
  • should display checkpoint diff preview
  • should display checkpoint metadata
  • should allow deleting checkpoint

Test Results

Browser Before After
Chromium 9/10 10/10 ✅
Firefox 9/10 10/10 ✅
WebKit 8/10 9/10
Mobile Safari 8/10 9/10
Total 34/40 48/50

Remaining 2 failures are WebKit login timeouts tracked in #230 (not checkpoint-specific).

Test plan

Summary by CodeRabbit

  • Tests
    • Improved test reliability and stability by adding synchronization mechanisms to checkpoint UI tests, preventing potential race conditions during automated testing.

✏️ Tip: You can customize this high-level summary in your review settings.

Four checkpoint tests were calling count() immediately after the API
response without waiting for React to render. This caused intermittent
failures where the locator found 0 checkpoint items despite the API
returning 3 checkpoints.

Fix: Wait for checkpoint items OR empty state to be visible before
calling count() to ensure React has finished rendering.

Tests fixed:
- should show restore confirmation dialog
- should display checkpoint diff preview
- should display checkpoint metadata
- should allow deleting checkpoint

Result: 48/50 checkpoint tests pass (96%). Remaining 2 failures are
WebKit login timeouts tracked in #230.
@coderabbitai

coderabbitai Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR adds explicit visibility waits to end-to-end tests for the checkpoint UI to synchronize test execution and prevent race conditions between DOM rendering and test assertions.

Changes

Cohort / File(s) Summary
Test synchronization
tests/e2e/test_checkpoint_ui.spec.ts
Added awaits for visibility checks on checkpoint items list or empty state using .toBeVisible() with DOM_UPDATE timeout across multiple test blocks to guard against race conditions

Possibly related issues

Possibly related PRs

Poem

🐰 A hop through the tests, we wait with care,
For items to show, or empty air,
No race conditions will catch us unaware,
Synchronized checkpoints, a flawless pair! 🎯

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding race condition guards to checkpoint UI tests, which aligns perfectly with the changeset's primary objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/e2e-checkpoint-race-condition

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between abe02fe and 8519af8.

📒 Files selected for processing (1)
  • tests/e2e/test_checkpoint_ui.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.{ts,tsx,test.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Never use test.skip() inside test logic; skip at describe level or use separate test projects for different states

Files:

  • tests/e2e/test_checkpoint_ui.spec.ts
tests/e2e/**/*.{ts,test.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

tests/e2e/**/*.{ts,test.ts}: Use loginUser() helper from tests/e2e/test-utils.ts for authentication in E2E tests
Assert UI elements existence with expect(element).toBeVisible() - fail if missing, never silently pass
Use TEST_PROJECT_IDS.PLANNING for tests requiring pre-seeded planning phase tasks
Use TEST_PROJECT_IDS.ACTIVE for tests requiring pre-seeded active phase with agents

Files:

  • tests/e2e/test_checkpoint_ui.spec.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/e2e/**/*.{ts,test.ts} : Assert UI elements existence with expect(element).toBeVisible() - fail if missing, never silently pass
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/e2e/**/*.{ts,test.ts} : Assert UI elements existence with expect(element).toBeVisible() - fail if missing, never silently pass

Applied to files:

  • tests/e2e/test_checkpoint_ui.spec.ts
📚 Learning: 2026-01-11T23:33:31.895Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T23:33:31.895Z
Learning: Applies to tests/e2e/**/*.{ts,test.ts} : Use loginUser() helper from tests/e2e/test-utils.ts for authentication in E2E tests

Applied to files:

  • tests/e2e/test_checkpoint_ui.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Backend Unit Tests
  • GitHub Check: Frontend Unit Tests
  • GitHub Check: E2E Smoke Tests (Chromium)
🔇 Additional comments (4)
tests/e2e/test_checkpoint_ui.spec.ts (4)

283-285: LGTM! Race condition guard correctly applied.

The .or() pattern properly waits for either state before calling count(), preventing the intermittent test failures. This aligns with the coding guideline to assert UI elements with toBeVisible().

Minor observation: checkpointList declared on line 279 is unused in this test.


316-318: LGTM! Consistent race condition guard.

Same pattern correctly applied here, ensuring React has finished rendering before count() is called on line 319.


361-363: LGTM!

Race condition guard properly applied before count() on line 364.


391-393: LGTM!

All four race condition guards follow the same pattern consistently, making the fix easy to understand and maintain.


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

@macroscopeapp

macroscopeapp Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Add race condition guards using checkpointItems.first().or(emptyState) with TIMEOUTS.DOM_UPDATE in Checkpoint UI e2e tests in test_checkpoint_ui.spec.ts to reduce timing-related failures

Insert precondition waits that assert visibility of either the first checkpoint item or the empty state before counting and interacting with list items in Checkpoint UI workflow tests in test_checkpoint_ui.spec.ts.

📍Where to Start

Start with the Checkpoint UI workflow tests and review the newly added precondition waits in test_checkpoint_ui.spec.ts.


Macroscope summarized 8519af8.

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.

[P1] Checkpoint UI fails to display seeded checkpoints - 4 E2E tests failing

1 participant