Skip to content

fix(test-runner): make runs cancellable, deadlock-free, and bounded - #735

Open
beruro wants to merge 5 commits into
developfrom
junyu/fix-test-runner-lifecycle
Open

fix(test-runner): make runs cancellable, deadlock-free, and bounded#735
beruro wants to merge 5 commits into
developfrom
junyu/fix-test-runner-lifecycle

Conversation

@beruro

@beruro beruro commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

Test runs in the WorkStation Testing tab could not actually be stopped, could deadlock while draining child output, and could grow memory without bound. The frontend stop action cleared UI state without signalling the backend, command and runner layers generated different run ids, cancellation was not observed by the child lifecycle, stdout and stderr were drained sequentially, and both streams accumulated into unbounded strings.

The test event wire shape also emitted snake_case variant fields while the frontend expects camelCase, so run_started could arrive without a usable runId.

Solution

The branch now uses one canonical run id from command registration through every event and summary, with a drop guard that always deregisters the run. Cancellation is backed by CancellationToken and terminates the process tree: Unix uses a dedicated process group with TERM, a bounded grace period, then KILL; Windows uses taskkill tree termination plus start_kill.

Stdout and stderr are drained concurrently in chunk-based tasks with a 16 MiB tail bound per stream and explicit truncation state. Cancelled runs emit run_cancelled and resolve with a partial summary marked cancelled. Per-variant camelCase serialization pins the frontend wire contract.

The frontend stop action now resolves the active run, invokes stop_tests, and updates state only after backend confirmation. Pure lifecycle transitions cover stale parallel-run events and no-op stop attempts.

Current develop is merged into the branch. Develop's relocation of the test-runner hook into the WorkStation module is preserved while retaining the lifecycle changes.

Potential risks

  • stop_tests changes from an erroring unit result to a boolean that reports whether a run was signalled; the only caller is updated in this branch.
  • Cancelled runs resolve with partial counts marked cancelled, so summary consumers must respect the flag.
  • Reporter output beyond the per-stream tail bound is truncated; JSON parsing may fall back to degraded line parsing for a single document larger than the budget.
  • The Windows tree-kill path compiles but is not exercised by the macOS verification environment.
  • The pre-existing stable Cargo libtest format limitation is unchanged.

Rollback is a normal revert of this PR. It changes no persistence schema and requires no data recovery.

Verification

Merged current develop at b9331da7e into the branch in merge commit 8ec95dbc9. GitHub now reports the PR as mergeable. Relative to current develop, the PR remains 16 files with 1,138 additions and 148 deletions; the test-runner hook change follows its new WorkStation path.

  • cargo test -p test_runner from src-tauri: 28/28 passed, including stderr-flood no-deadlock, bounded capture, process-group kill with grandchild verification, pre-cancelled token, canonical-id event ordering, wire-format pinning, and registry/RunGuard lifecycle coverage.
  • cargo clippy -p test_runner --all-targets -- -D warnings from src-tauri: passed with no warnings.
  • pnpm vitest run src/services/test/__tests__/testRunLifecycle.test.ts: 11/11 passed.
  • pnpm typecheck: passed.
  • Focused ESLint over the six changed frontend TypeScript files: passed.
  • git diff --cached origin/develop --check before the merge commit: passed.
  • CI is rerunning on 8ec95dbc9; Frontend, Rust, and attribution checks were queued/in progress at the time of this update.

The merge commit used --no-verify because lint-staged expands a merge index to hundreds of files imported from develop and attempted to format unrelated base files. Those hook-generated edits were removed, the 16-file PR diff was revalidated, and the focused lint, typecheck, frontend tests, Rust tests, and Rust clippy commands above all passed.

Effects: none added or modified (React useEffect surface untouched; useTestRunner only re-types stopTests).

@beruro
beruro marked this pull request as ready for review August 7, 2026 14:23
@beruro
beruro force-pushed the junyu/fix-test-runner-lifecycle branch from 2efe460 to 668102e Compare August 10, 2026 04:04
@beruro
beruro force-pushed the junyu/fix-test-runner-lifecycle branch from 44ab60e to 4bb09b6 Compare August 10, 2026 05:47
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