Skip to content

test(e2e): fix the project-rename flake at its root and run the suite once#443

Merged
tkislan merged 1 commit into
mainfrom
test/e2e-flakiness
Jul 20, 2026
Merged

test(e2e): fix the project-rename flake at its root and run the suite once#443
tkislan merged 1 commit into
mainfrom
test/e2e-flakiness

Conversation

@tkislan

@tkislan tkislan commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the flaky projectRename E2E suite at its root and drops the expensive whole-suite retry. Surgical — 4 files.

  • Runs once in CI. Removes the until … max=2 shell loop in e2e.yml that reran all 16 spec files on any failure.
  • Fixes the actual flake. projectRename failed reproducibly deep in the shared run but passed in a fresh instance. leaveUnsavedCellEdit located the first code cell's .view-line then .click()d it as a separate step; the notebook re-renders its cells on open, staling that reference before the click lands (StaleElementReferenceError) — far likelier when the shared instance is sluggish late in the run. It now locates and clicks in one retried driver.wait step (the fix from feat(telemetry): add PostHog analytics for notebook usage events #375).
  • Isolation. ExTester runs every spec in one shared VS Code instance, and no suite dismissed notification toasts in teardown, so they stacked up — waitForNotification polls every visible toast each tick, slowing later specs and overlapping the tree/menus/inputs they drive. A Mocha root afterEach (test/e2e/rootHooks.ts, wired via .mocharc.js) now dismisses them between tests.

Validation

Ran the full stack locally exactly as CI does — packaged the VSIX, downloaded test VS Code 1.111.0 + ChromeDriver, installed the built extension + the Python extension, under Xvfb:

  • Full suite, single run: 54 passing, 0 failing, 1 pending (the intentional it.skip). projectRename passes in-place — it failed 2/2 full runs before this fix.
  • npm run compile-e2e clean; npm run format-fix clean.

Known follow-up (not in this PR)

Running kernels/servers and global environments created by the cell-running suites are not torn down between suites. The run is green despite this now that the race is fixed, but fully isolating them means per-suite environment teardown, which re-provisions venvs and adds CI time — a separate cost/isolation decision.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ELgnvYGB68gTtHncpWN588

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when editing unsaved notebook cells during project renaming.
    • Prevented notification messages from lingering between end-to-end test scenarios.
  • Tests

    • Streamlined end-to-end test execution and improved handling of transient interface updates.
    • Enhanced test setup to consistently reset notifications and use compiled test configuration.

… once

The E2E job reran all 16 spec files on any failure (a shell `until` loop),
which was expensive, and the projectRename suite failed reproducibly deep in
the shared VS Code run while passing in isolation.

Root cause: leaveUnsavedCellEdit located the first code cell's view-line then
clicked it as a separate step, so the cell re-rendering between locate and
click threw a StaleElementReferenceError — far likelier when the shared
instance is sluggish late in the run, where undismissed notification toasts had
piled up. (My earlier retryable-setup wrapper masked this behind a misleading
open-folder error; it is dropped in favour of the root fix.)

- projectRename: locate and click the cell line in one retried step (from #375).
- rootHooks.ts + .mocharc.js: a Mocha root afterEach dismisses notification
  toasts between tests so they do not accumulate across the shared instance.
- e2e.yml: run the suite once (drop the until-loop whole-suite retry).

Validated locally against the packaged VSIX under Xvfb, exactly as CI runs:
full suite 54 passing, 0 failing, 1 pending in a single run; projectRename
passes in-place (it failed 2/2 full runs before this fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELgnvYGB68gTtHncpWN588
@tkislan
tkislan force-pushed the test/e2e-flakiness branch from 77b6096 to 79bb170 Compare July 18, 2026 09:03
@tkislan tkislan changed the title test(e2e): drop the whole-suite retry loop and fix cross-suite flakiness test(e2e): fix the project-rename flake at its root and run the suite once Jul 18, 2026
@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (b8505b2) to head (79bb170).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #443   +/-   ##
===========================
===========================
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tkislan
tkislan marked this pull request as ready for review July 19, 2026 06:36
@tkislan
tkislan requested a review from a team as a code owner July 19, 2026 06:36
@tkislan

tkislan commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 60408c96-8ae1-4ed1-8403-8f81235c0e3e

📥 Commits

Reviewing files that changed from the base of the PR and between b8505b2 and 79bb170.

📒 Files selected for processing (4)
  • .github/workflows/e2e.yml
  • test/e2e/.mocharc.js
  • test/e2e/rootHooks.ts
  • test/e2e/suite/projectRename.e2e.test.ts

📝 Walkthrough

Walkthrough

The E2E workflow now runs the prebuilt suite once under xvfb-run. Mocha loads compiled root hooks that dismiss notifications after each test. The project rename test also retries locating and clicking the first notebook code cell to tolerate transient re-rendering.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main fix for the project-rename E2E flake and the switch to a single suite run.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Updates Docs ✅ Passed Only CI/test files changed; no user-facing docs or roadmap content was touched, so this docs check is not applicable.

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

@tkislan
tkislan merged commit b696a91 into main Jul 20, 2026
14 checks passed
@tkislan
tkislan deleted the test/e2e-flakiness branch July 20, 2026 07:58
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