Fix two integration tests that time out when npm test runs in a terminal by ryanwelcher · Pull Request #680 · WordPress/contributor-toolkit

ryanwelcher · 2026-10-13T18:18:32Z

Why

Running npm test from an interactive terminal fails two integration tests on macOS. kill-tree.integration.test.cjs times out after 60 seconds and orphan-pipe.integration.test.cjs reports "the orphan never reported its pid". CI has no terminal, so it stays green and only local runs fail.

What changes

When node --test writes to a terminal, it sets FORCE_COLOR for each test file. The node grandchild each test spawns inherits it, so console.log(process.pid) prints the pid in ANSI colour codes (\x1b[33m51099\x1b[39m). kill-tree's /^(\d+)/m never matches, and orphan-pipe's Number() returns NaN.

The grandchild now writes the pid with process.stdout.write(process.pid + "\n"). A string is never coloured, so the output is the same with or without a terminal. electron-node-compat.test.cjs already does this. Only the two test files change.

How to test this

Platforms: macOS (or Linux). Both tests are POSIX only and Windows skips them.

Starting state: a checkout with node_modules installed and the Node version from .nvmrc. Run every command from the repository root.

On trunk, run npm test in an interactive terminal. Both tests fail, kill-tree after 60 seconds.
Still on trunk, run FORCE_COLOR=1 npm test > out.txt 2>&1 (no terminal). The same two tests fail. That shows FORCE_COLOR alone is the trigger.
Switch to this branch and repeat steps 1 and 2. All tests pass, and the two integration tests take about 2 seconds each.

What must not have happened: the tests must still check what they did before. kill-tree should still need the forced group signal to end the grandchild, and orphan-pipe should still see exit without close while the orphan holds the pipes. Both assertions are unchanged.

Risks and limitations

None known. This is a test-only change. The self-review found nothing to fix.

Review: completed. Self-review per .github/instructions/code-review.instructions.md, judgement pass by a fresh-context agent; reviewed head 9ae7ac8 / base 5d5f701 (trunk). npm run lint passes. npm test passes (2098 tests, 0 failures) without a terminal, under a pseudo-terminal, with FORCE_COLOR=1, and on Electron's bundled Node via npm run test:electron. Outcome: no findings.
Since review: none.

The reviewer suggested a rule for the instructions file: a child process whose output a test parses should write it with process.stdout.write and a string, not console.log. Left out of this PR to keep it to the fix.

Screenshots or recording

Nothing on screen changes.

🤖 Generated with Claude Code

…rminal When `node --test` writes to a terminal it sets FORCE_COLOR for each test file, and the grandchild these tests spawn inherits it. console.log then colours the pid it prints, so kill-tree's match never succeeds and the test times out after 60 seconds, and orphan-pipe reads NaN and reports that the orphan never sent its pid. CI has no terminal, so only a local `npm test` failed. Writing the pid as a string leaves it uncoloured either way.

coderabbitai · 2026-10-13T18:18:45Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 4b215156-de77-4fd5-95ac-4850483cdf43

📥 Commits

Reviewing files that changed from the base of the PR and between 5d5f701 and 9ae7ac8.

📒 Files selected for processing: 2

tests/unit/kill-tree.integration.test.cjs
tests/unit/orphan-pipe.integration.test.cjs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The STUBBORN and orphan-process test scripts now write process IDs with process.stdout.write instead of console.log. Their comments explain that terminal color formatting can prevent the tests from parsing the IDs.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 9ae7a

The changes preserve parseable PID output in both integration tests. No remaining merge-blocking risk is evident.

Pre-merge checks: 1

Passed checks: 1 passed

Check name	Status	Explanation
Description check		The description includes the required rationale, changes, test steps, starting state, regression checks, risks, related issues, design decisions, review outcome, and screenshots status.

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

## Why 41 commits have reached trunk since `v1.2.1`, and milestone `v2.0.0-beta.1` has 77 closed issues and none open. The main changes are the redesign on the WordPress Design System (#591), a settings dialog with language, theme, PHP version and debug flags (#638 to #651), and translation of the whole interface (#634 to #664). This is a beta: it ships with `--prerelease`, so `releases/latest` stays on `v1.2.1`. `electron-builder` names the artifacts after `package.json`, so the version has to move before the tag and artifacts exist. ## What changes - **The version.** The three root version fields in `package.json` and `package-lock.json` move from `1.2.1` to `2.0.0-beta.1`. No dependency versions change. - **The first shipped translations.** `npm run i18n:download` added a catalog for each of the seven locales at 80% or more on translate.wordpress.org. None was removed, and no right-to-left locale was held back. | Locale | Translated | Strings | | --- | --- | --- | | cs | 94% | 1001 | | en-ca | 100% | 1062 | | en-gb | 100% | 1062 | | gl | 100% | 1062 | | hu | 100% | 1062 | | nl | 100% | 1062 | | pl | 96% | 1028 | ## How to test this **Platforms:** any for steps 1 to 3. The artifact checks need macOS for `Info.plist` and Windows for the `.exe`. **Starting state:** this branch checked out with `node_modules` installed. Run every command from the repository root. 1. `npm run lint` is clean and `npm test` passes. 2. `node -e "console.log(require('./package.json').version)"` prints `2.0.0-beta.1`. 3. `python3 -c "import json;d=json.load(open('package-lock.json'));print(d['version'], d['packages']['']['version'])"` prints `2.0.0-beta.1 2.0.0-beta.1`. 4. On the Buildkite build for the current head, the artifacts are named `wordpress-contributor-toolkit-2.0.0-beta.1-*`. The `app.asar` root holds exactly `node_modules`, `package.json` and `src`, with npm 11 under `node_modules/npm` and no `.codesigning`. On macOS, `CFBundleShortVersionString` reads `2.0.0-beta.1`. 5. In the installed app, set the language to Dutch in the settings and relaunch. The interface is in Dutch. **What must not have happened:** no dependency version may change. `git diff trunk -- . ':!src/languages'` shows exactly three changed lines, all of them the root version. ## Risks and limitations `npm test` run from an interactive terminal fails two integration tests on macOS. That is a test bug, fixed separately in #680. It does not affect CI or the app. ## Related Milestone `v2.0.0-beta.1`. Release notes follow on the release. --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 [fix here] · 0 [follow-up]. Automatic checks only, as in #533: this change has no logic to review. - **Review:** completed, deterministic layer, in session; head `9853082` / base `5d5f701` (trunk). `npm run lint` passes, `npm test` passes (2098 tests, 0 failures, 2 skipped), both files are valid JSON, and the diff outside `src/languages/` is the three root version fields. Outcome: nothing to fix. - **Since review:** none. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ryanwelcher · 2026-10-13T22:43:03Z

@coderabbitai review

coderabbitai · 2026-10-13T22:43:11Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

ryanwelcher mentioned this pull request Oct 13, 2026

Bump version to 2.0.0-beta.1 #681

Merged

ryanwelcher merged commit 6b77bc3 into trunk Oct 14, 2026
13 checks passed

ryanwelcher deleted the fix/test-pid-force-color branch October 14, 2026 15:50

Navigation Menu

Repository navigation

Fix two integration tests that time out when npm test runs in a terminal - #680

Fix two integration tests that time out when npm test runs in a terminal#680
ryanwelcher merged 1 commit into
trunkfrom
fix/test-pid-force-color

ryanwelcher commented Oct 13, 2026

Uh oh!

coderabbitai Bot commented Oct 13, 2026 •

edited

Loading

Uh oh!

ryanwelcher commented Oct 13, 2026

Uh oh!

coderabbitai Bot commented Oct 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ryanwelcher commented Oct 13, 2026

Why

What changes

How to test this

Risks and limitations

Related

Uh oh!

coderabbitai Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanwelcher commented Oct 13, 2026

Uh oh!

coderabbitai Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 13, 2026 •

edited

Loading

coderabbitai Bot commented Oct 13, 2026 •

edited

Loading