Skip to content

fix(e2e): kill dev server trees and stop teardown hook timeouts - #423

Merged
wyattjoh merged 1 commit into
mainfrom
wyattjoh/fix-e2e-flakes
Aug 17, 2026
Merged

fix(e2e): kill dev server trees and stop teardown hook timeouts#423
wyattjoh merged 1 commit into
mainfrom
wyattjoh/fix-e2e-flakes

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

Two teardown bugs have been failing E2E runs on main. Cleanup hooks ran on bun's 5s default timeout while each test-user delete shells out to a cold bun src/cli.ts and hits BAPI, which regularly takes longer than that with eight files running in parallel — so a test that had already passed got failed by its own cleanup. Cleanup hooks now get a 60s timeout, and users-list deletes its users concurrently.

That flake then became a guaranteed failure because killDevServer signalled only the spawned npx wrapper. npm forwards SIGTERM and exits, so the harness logged "dev server stopped" while the real dev server was reparented to init, still holding its port and dev lockfile; the retry reused the same project dir and died with "Another dev server is already running". killDevServer now snapshots the full process tree before signalling — descendants become unreachable from proc.pid once the wrapper exits — SIGTERMs it, and escalates to SIGKILL for anything that outlasts the grace period, comparing command lines to guard against pid reuse. Tree liveness replaces proc.exited as the stopping condition, which also fixes a hang when an orphan holds the inherited stdio pipes.

Test plan

  • format:check, lint, typecheck, test (2618 pass, 0 fail)
  • Orphan reproduced against a SIGTERM-ignoring grandchild; the new code kills it and frees the port, and the happy path stays fast (114ms)
  • nextjs-pages-router + users-list against the live API: 5 pass, 0 fail, no dev-server processes left behind
  • Full E2E suite in CI

@changeset-bot

changeset-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 409619e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@wyattjoh
wyattjoh marked this pull request as ready for review August 17, 2026 20:43
@wyattjoh
wyattjoh requested a review from rafa-thayto August 17, 2026 20:44
@wyattjoh
wyattjoh merged commit 56ed2e2 into main Aug 17, 2026
9 of 10 checks passed
@wyattjoh
wyattjoh deleted the wyattjoh/fix-e2e-flakes branch August 17, 2026 20:44
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4b1b0d4b-9683-45fa-90a2-bce46cd20b9a

📥 Commits

Reviewing files that changed from the base of the PR and between 5a58b22 and 409619e.

📒 Files selected for processing (3)
  • test/e2e/lib/dev-server.ts
  • test/e2e/lib/fixture-test.ts
  • test/e2e/users-list.test.ts

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.


📝 Walkthrough

Walkthrough

The end-to-end development-server cleanup now discovers and terminates the full process tree, escalates from SIGTERM to SIGKILL, and waits for cleanup on early exits. Fixture cleanup hooks use 60-second timeouts. Users-list teardown deletes tracked users concurrently before removing temporary configuration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: rafa-thayto


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

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