test(exec): fix Windows failure in PnP require option test by zkochan · Pull Request #12499 · pnpm/pnpm

zkochan · 2026-06-18T13:18:52Z

Summary

The exec should merge node options with PnP require option test (added in #12430) is failing on the Windows CI runners on main, e.g. run 27752731292 (TS CI / Test / windows / Node 26).

The test hardcodes an unquoted expectation:

NODE_OPTIONS: `--max-old-space-size=4096 --require=${pnpPath}`

But #12430 made makeNodeRequireOption run the path through quotePathIfNeeded. On Windows the .pnp.cjs path contains backslashes, which match /[\s"'\\]/, so the path is wrapped in double quotes and backslash-escaped (--require="D:\\a\\...\\.pnp.cjs") for Node's NODE_OPTIONS tokenizer. The quoting is intentional and correct — only the test's expectation was wrong. On POSIX the path has no special characters, so it's left unquoted and the test happened to pass there.

Fix

Derive the expected NODE_OPTIONS from makeNodeRequireOption itself (the source of truth) instead of hardcoding the unquoted form, so the expectation matches the implementation's quoting on every platform. The test still guards the original regression (nodeOptions being dropped when the PnP require option is added): if exec.handler dropped --max-old-space-size=4096, the produced value would no longer equal the helper's output.

This is a test-only change — no published package behavior changes, so no changeset.

Test plan

pnpm --filter @pnpm/exec.commands run compile
test/exec.ts passes locally (4/4) on macOS.

Written by an agent (Claude Code, claude-opus-4-8).

Summary by CodeRabbit

Tests
- Updated test assertions to use shared helper functions instead of hardcoded values, ensuring test expectations align with implementation logic.

The 'exec should merge node options with PnP require option' test (added in #12430) hardcoded an unquoted '--require=<path>' expectation. On Windows the .pnp.cjs path contains backslashes, which makeNodeRequireOption quotes and escapes for Node's NODE_OPTIONS tokenizer, so the assertion mismatched and the test failed on Windows runners. Derive the expected NODE_OPTIONS from makeNodeRequireOption itself so the expectation matches the implementation's quoting on every platform.

coderabbitai · 2026-06-18T13:19:09Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 03748155-b7db-44b1-9128-3eccca87e362

📥 Commits

Reviewing files that changed from the base of the PR and between 8d17c7b and d35e967.

📒 Files selected for processing (1)

exec/commands/test/exec.ts

 _______________________________________________________________________________________________________________________________________________________________________________________________________________________________________
< There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies. - C.A.R. Hoare >
 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/exec-pnp-require-windows-test

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 and usage tips.}

qodo-free-for-open-source-projects · 2026-06-18T13:19:56Z

PR Summary by Qodo

Fix Windows CI failure in exec PnP NODE_OPTIONS merge test
🧪 Tests 🐞 Bug fix 🕐 Less than 10 minutes

Description

• Fix Windows-only failure in exec PnP require NODE_OPTIONS merge assertion.
• Derive expected NODE_OPTIONS via makeNodeRequireOption to match platform quoting.

Diagram

graph TD
  T["exec/commands/test/exec.ts"] --> H["exec.handler"] --> R["makeNodeRequireOption"] --> N["NODE_OPTIONS string"] --> X["execa call"] --> P["eslint process"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Platform-conditional expected string

➕ Keeps the test independent from makeNodeRequireOption implementation details
➖ Duplicates quoting/escaping rules and is easy to get wrong as Node tokenization rules evolve
➖ More branching/complexity in the test suite

2. Normalize pnpPath to avoid quoting triggers

➕ Keeps assertion simple (unquoted path)
➖ Stops testing the real Windows path behavior that exec must support
➖ May mask regressions in quoting/escaping behavior

Recommendation: Deriving the expected NODE_OPTIONS via makeNodeRequireOption is the best approach: it matches exec’s source-of-truth quoting/escaping across platforms while still validating the regression of interest (that existing nodeOptions are preserved when adding the PnP require option).

Files changed (1) +5 / -3

Tests (1) +5 / -3

exec.tsCompute expected NODE_OPTIONS via makeNodeRequireOption for cross-platform quoting +5/-3

Compute expected NODE_OPTIONS via makeNodeRequireOption for cross-platform quoting
• Imports makeNodeRequireOption and uses it to derive the expected NODE_OPTIONS value in the PnP require merge test. This fixes a Windows-only assertion mismatch caused by intentional quoting/backslash escaping in NODE_OPTIONS.
exec/commands/test/exec.ts

qodo-free-for-open-source-projects · 2026-06-18T13:21:44Z

Code Review by Qodo

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Auto-approved notes
- no code review findings
- 'auto_approve_for_no_suggestions' flag enabled

Previous review results

Review updated until commit d35e967

Results up to commit d35e967

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Auto-approved notes
- no code review findings
- 'auto_approve_for_no_suggestions' flag enabled

qodo-free-for-open-source-projects · 2026-06-18T13:24:10Z

Code review by qodo was updated up to the latest commit d35e967

coderabbitai Bot added the product: pnpm@11 label Jun 18, 2026

qodo-free-for-open-source-projects Bot approved these changes Jun 18, 2026

View reviewed changes

zkochan enabled auto-merge (squash) June 18, 2026 13:42

zkochan added the reviewed: qodo label Jun 18, 2026

zkochan mentioned this pull request Jun 18, 2026

fix(ci): grant pull-requests: write so the review label step can label PRs #12501

Merged

zkochan merged commit cb3d8e7 into main Jun 18, 2026
26 checks passed

zkochan deleted the fix/exec-pnp-require-windows-test branch June 18, 2026 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test(exec): fix Windows failure in PnP require option test#12499

test(exec): fix Windows failure in PnP require option test#12499
zkochan merged 1 commit into
mainfrom
fix/exec-pnp-require-windows-test

zkochan commented Jun 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026 •

edited

Loading

Great, no issues found!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zkochan commented Jun 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Test plan

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Great, no issues found!

Previous review results

Great, no issues found!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zkochan commented Jun 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 18, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 18, 2026 •

edited

Loading