Skip to content

Conversation

@arii
Copy link
Owner

@arii arii commented Feb 10, 2026

Description

This PR refactors WebSocketContext.tsx and fixes a flaky E2E test, addressing directives to remove "AI slop" and improve E2E test reliability.

Specifically, it:

  1. context/WebSocketContext.tsx: Combines two identical if (process.env.NODE_ENV !== 'production' ...) blocks into a single block that sets up both global test controls and the message event listener.
  2. tests/playwright/stale-tile.spec.ts: Replaces await page.waitForTimeout(1000) with await page.waitForFunction(() => (window as unknown as { __TEST_CONTROLS__: unknown }).__TEST_CONTROLS__) to eliminate race conditions and ensure the test waits for the application to be fully initialized and ready to receive test commands.

Fixes # (issue)

Change Type: 🐛 Bug fix (non-breaking change fixing an issue)

PR Scope Checklist

This checklist is mandatory for all PRs.

  • PR has a clear, single purpose: The title and description of the PR clearly state the purpose of the change.
  • All changes relate to the stated objective: The code changes should be directly related to the purpose of the PR.
  • No unrelated cleanup or refactoring: The PR should not contain any changes that are not directly related to the stated objective.
  • Title and description match the actual changes: The title and description should accurately reflect the changes in the PR.
  • Tests cover the specific change scope: The tests should be focused on the changes in the PR and should not include unrelated tests.

Impact Assessment

  • Changes are backward compatible (or breaking changes are documented)
  • Tests are added/updated for new functionality
  • Documentation is updated if needed
  • ADR is created/updated for significant architectural changes
Original PR Body

This PR addresses the directives to remove "AI slop" and improve E2E test reliability.

Changes:

  1. context/WebSocketContext.tsx: Combined two identical if (process.env.NODE_ENV !== 'production' ...) blocks into a single block that sets up both global test controls and the message event listener.
  2. tests/playwright/stale-tile.spec.ts: Replaced await page.waitForTimeout(1000) with await page.waitForFunction(() => (window as unknown as { __TEST_CONTROLS__: unknown }).__TEST_CONTROLS__). This eliminates race conditions by ensuring the test waits for the application to be fully initialized and ready to receive test commands.

PR created automatically by Jules for task 3982628409866286911 started by @arii

- Consolidated duplicate environment checks in `context/WebSocketContext.tsx` to reduce code duplication and improve maintainability.
- Updated `tests/playwright/stale-tile.spec.ts` to replace flaky `waitForTimeout` with `waitForFunction`, ensuring the test waits for `window.__TEST_CONTROLS__` to be initialized before proceeding.
- Fixed linting issues in `stale-tile.spec.ts` by properly typing `window` access.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@google-labs-jules
Copy link
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions github-actions bot changed the title Refactor WebSocketContext and fix flaky E2E test test(small): Refactor WebSocketContext and fix flaky E2E test Feb 10, 2026
@arii
Copy link
Owner Author

arii commented Feb 10, 2026

Reviewed commit: 1a196a8d8859aa39fbd15b6a66d085593d128893

Review Summary

This PR successfully refactors the WebSocketContext and addresses a flaky E2E test. The changes are concise and directly improve code quality and test reliability.

Strengths

  • Improved Code Conciseness: The merging of redundant if blocks in WebSocketContext.tsx (lines 120-128) reduces duplication and improves readability, aligning with the "Eliminate Verbose and Redundant Code" principle from docs/CODE_REVIEW_GUIDELINES.md.
  • Enhanced E2E Test Stability: Replacing page.waitForTimeout(1000) with page.waitForFunction in stale-tile.spec.ts (lines 16-19) makes the E2E test significantly more robust and less prone to flakiness. This is a best practice for reliable asynchronous testing.
  • Adherence to Testing Patterns: The use of window.__TEST_CONTROLS__ for E2E test synchronization is consistent with the window.TEST_CONTROLS pattern described in docs/TYPESCRIPT_PATTERNS.md.

Issues

No blocking issues or significant concerns were found in this PR.

Test Coverage

The PR includes a targeted update to an E2E test, which directly addresses the identified flakiness. The change improves the quality and reliability of the existing test coverage.

Verdict

✅ Verified the refactoring in WebSocketContext.tsx and the fix for the flaky E2E test in stale-tile.spec.ts. No regressions or new issues found. Ready for approval.

Reviewed at commit: 1a196a8d8859aa39fbd15b6a66d085593d128893

@google-labs-jules
Copy link
Contributor

Reviewed commit: 1a196a8d8859aa39fbd15b6a66d085593d128893

Review Summary

This PR successfully refactors the WebSocketContext and addresses a flaky E2E test. The changes are concise and directly improve code quality and test reliability.

Strengths

  • Improved Code Conciseness: The merging of redundant if blocks in WebSocketContext.tsx (lines 120-128) reduces duplication and improves readability, aligning with the "Eliminate Verbose and Redundant Code" principle from docs/CODE_REVIEW_GUIDELINES.md.
  • Enhanced E2E Test Stability: Replacing page.waitForTimeout(1000) with page.waitForFunction in stale-tile.spec.ts (lines 16-19) makes the E2E test significantly more robust and less prone to flakiness. This is a best practice for reliable asynchronous testing.
  • Adherence to Testing Patterns: The use of window.__TEST_CONTROLS__ for E2E test synchronization is consistent with the window.TEST_CONTROLS pattern described in docs/TYPESCRIPT_PATTERNS.md.

Issues

No blocking issues or significant concerns were found in this PR.

Test Coverage

The PR includes a targeted update to an E2E test, which directly addresses the identified flakiness. The change improves the quality and reliability of the existing test coverage.

Verdict

✅ Verified the refactoring in WebSocketContext.tsx and the fix for the flaky E2E test in stale-tile.spec.ts. No regressions or new issues found. Ready for approval.

Reviewed at commit: 1a196a8d8859aa39fbd15b6a66d085593d128893

Thank you for the review. I am glad the refactoring and test improvements meet the project standards.

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