Skip to content

test(ci): probe — drop mocha --exit on Windows backend tests#7856

Open
JohnMcLear wants to merge 2 commits into
developfrom
probe-flake-no-exit-flag
Open

test(ci): probe — drop mocha --exit on Windows backend tests#7856
JohnMcLear wants to merge 2 commits into
developfrom
probe-flake-no-exit-flag

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

One-line workflow probe on a separate branch from #7855: remove the --exit flag from pnpm test on both Windows backend matrices.

Why

The current workflow has been invoking pnpm test -- --exit on Windows since PR #7762, with the comment:

--exit forces process.exit(failures) after the suite completes, closing the post-suite event-loop drain window where Windows + Node 24 hard-kills the process.

That's a direct admission of a known post-suite kill bug on Windows + Node 24, and --exit was added as a mitigation that hides it. The current silent-ELIFECYCLE flake fires mid-suite, but --exit is still in play after the dying test and may also be masking earlier signal.

Drop it on a separate probe branch. Either:

Any of these is data we don't currently have.

Test plan

🤖 Generated with Claude Code

The Windows backend-tests workflow has invoked mocha with --exit
since PR #7762, with the in-line comment:

  # --exit forces process.exit(failures) after the suite completes,
  # closing the post-suite event-loop drain window where Windows +
  # Node 24 hard-kills the process. Scoped to Windows so Linux/local
  # runs still surface real handle leaks via natural drain.

That comment is a direct admission that a kill HAPPENS at the
post-suite drain on Windows + Node 24, and --exit was added as a
**mitigation that hides it**. The current silent-ELIFECYCLE flake
fires MID-SUITE, but --exit is still in play after the dying test
fires and may also be masking earlier signal.

Probe: drop --exit on both Windows backend matrices. Expected
outcomes on a passing run:
  - if Node still exits cleanly via natural drain → the original
    post-suite hard-kill bug from PR #7762 is no longer reproducible
    in current Node 24.x, and we've been carrying a workaround for
    nothing.
  - if a fresh hard-kill appears at post-suite drain → we now have
    a SECOND data point (different from the mid-suite flake) and
    can compare its signature to the mid-suite kill in artifacts.

On a failing run (the mid-suite flake fires):
  - removing --exit may let mocha emit additional error reporting
    that --exit's process.exit() was suppressing.
  - or it may not change anything, which itself is a data point.

This is a one-line probe on a separate branch from
probe-flake-defender-eventlog-sidecar. Run in parallel; the two
ask different questions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Probe: remove mocha --exit flag from Windows backend tests

🧪 Tests

Grey Divider

Walkthroughs

Description
• Remove mocha --exit flag from Windows backend test matrices
• Probe to diagnose post-suite process hard-kill on Windows + Node 24
• Investigate whether original mitigation from PR #7762 still needed
• Enable detection of additional mocha error output without early exit
Diagram
flowchart LR
  A["Windows Backend Tests"] -->|Remove --exit flag| B["Natural Process Drain"]
  B -->|Outcome 1| C["Clean exit - bug resolved"]
  B -->|Outcome 2| D["Post-suite hard-kill - new data point"]
  B -->|Outcome 3| E["Mid-suite flake - unchanged behavior"]

Loading

Grey Divider

File Changes

1. .github/workflows/backend-tests.yml 🧪 Tests +2/-2

Remove --exit flag from Windows backend test runs

• Remove --exit flag from pnpm test command on line 259 (first Windows backend matrix)
• Remove --exit flag from pnpm test command on line 393 (second Windows backend matrix)
• Preserve existing diagnostic infrastructure and watcher process management
• Keep explanatory comments about the original mitigation rationale

.github/workflows/backend-tests.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 26, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Removed mitigation without guard ✓ Resolved 🐞 Bug ☼ Reliability
Description
The workflow comment says --exit was used as a Windows-only mitigation for a post-suite
natural-drain window where Windows + Node 24 hard-kills the process, but the Windows backend test
steps no longer pass --exit. This both risks reintroducing post-suite termination/hang behavior
(potentially preventing watcher cleanup) and will fail the vitest spec that asserts the workflow
contains exactly two pnpm test -- --exit invocations.
Code

.github/workflows/backend-tests.yml[395]

Evidence
In .github/workflows/backend-tests.yml, the inline documentation still describes --exit as the
Windows mitigation for the post-suite drain hard-kill and the steps rely on cleanup that only runs
after pnpm test returns, but the actual Windows commands now run pnpm test without --exit.
Separately, src/tests/backend-new/specs/backend-tests-flake-mitigation.test.ts matches the literal
string pnpm test -- --exit and asserts it appears exactly twice in the workflow, so with the
workflow no longer containing that string the match count becomes 0 and the test will fail, breaking
CI for runs that execute pnpm run test:vitest.

.github/workflows/backend-tests.yml[255-266]
.github/workflows/backend-tests.yml[389-400]
src/tests/backend-new/specs/backend-tests-flake-mitigation.test.ts[48-54]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Windows backend test steps in `.github/workflows/backend-tests.yml` removed `--exit` even though the workflow comment documents it as a Windows+Node24 mitigation for post-suite drain/hard-kill behavior, and the steps rely on post-command cleanup for a background watcher. This change also breaks a vitest spec that asserts the workflow contains exactly two `pnpm test -- --exit` invocations.
## Issue Context
The workflow starts a background watcher loop and then runs `pnpm test`; cleanup happens only after `pnpm test` returns, so hangs/early termination can prevent watcher cleanup. A vitest assertion in `src/tests/backend-new/specs/backend-tests-flake-mitigation.test.ts` is implemented by searching the workflow file for the exact string `pnpm test -- --exit` and expecting two occurrences, which no longer exist.
## Fix Focus Areas
- .github/workflows/backend-tests.yml[241-266]
- .github/workflows/backend-tests.yml[375-400]
- .github/workflows/backend-tests.yml[255-266]
- .github/workflows/backend-tests.yml[389-400]
- src/tests/backend-new/specs/backend-tests-flake-mitigation.test.ts[48-58]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

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.

1 participant