[PR #3519] [CLOSED] fix(test): isolate electron mock from ancestor node_modules #5797

Closed
opened 2026-10-31 01:09:31 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3519
Author: @MbappeWU
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/electron-active-hook-node-path-isolation


📝 Commits (1)

  • 496f2c1 fix(test): isolate electron mock resolution from ancestor node_modules

📊 Changes

1 file changed (+85 additions, -17 deletions)

View changed files

📝 tests/boundary/browser/electronActiveHook.test.ts (+85 -17)

📄 Description

Summary

Fixes #3473.

tests/boundary/browser/electronActiveHook.test.ts spawned node -r scripts/electron-active-hook.cjs with NODE_PATH=<tmpdir> holding a mock electron.js. CommonJS resolution walks ancestor node_modules of the hook script before NODE_PATH, so an electron package above the checkout (for example a parent workspace) was loaded instead of the mock. The hook then failed to patch BrowserWindow, and the exercise script (which correctly resolved the mock) crashed on windowOpenHandler.

Approach

  • Prepare a per-run launch tree under the temp module root: write the mock as node_modules/electron, and copy electron-active-hook.cjs plus its sibling electron-active-hook-boundaries.cjs beside the exercise script.
  • Clear NODE_PATH for the child so both the -r hook and the exercise script resolve the same local mock via normal CJS walking.
  • Add a regression that plants a string-export electron package in an ancestor directory and asserts the mock still wins.

No product capture behavior changes—test harness isolation only.

Verification

With ancestor pollution present at /workspace/oss-work/node_modules/electron and /workspace/node_modules/electron (string-export stubs):

npx vitest run tests/boundary/browser/electronActiveHook.test.ts

Result: 4 passed (3 existing + 1 new regression), Node v24.21.0.

Privacy-check on staged changes: exit 0.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3519 **Author:** [@MbappeWU](https://github.com/MbappeWU) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/electron-active-hook-node-path-isolation` --- ### 📝 Commits (1) - [`496f2c1`](https://github.com/morluto/rea/commit/496f2c1ee014381c2dcde383e56a6dedcb05014b) fix(test): isolate electron mock resolution from ancestor node_modules ### 📊 Changes **1 file changed** (+85 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/browser/electronActiveHook.test.ts` (+85 -17) </details> ### 📄 Description ## Summary Fixes #3473. `tests/boundary/browser/electronActiveHook.test.ts` spawned `node -r scripts/electron-active-hook.cjs` with `NODE_PATH=<tmpdir>` holding a mock `electron.js`. CommonJS resolution walks ancestor `node_modules` of the hook script **before** `NODE_PATH`, so an `electron` package above the checkout (for example a parent workspace) was loaded instead of the mock. The hook then failed to patch `BrowserWindow`, and the exercise script (which correctly resolved the mock) crashed on `windowOpenHandler`. ## Approach - Prepare a per-run launch tree under the temp module root: write the mock as `node_modules/electron`, and copy `electron-active-hook.cjs` plus its sibling `electron-active-hook-boundaries.cjs` beside the exercise script. - Clear `NODE_PATH` for the child so both the `-r` hook and the exercise script resolve the same local mock via normal CJS walking. - Add a regression that plants a string-export `electron` package in an ancestor directory and asserts the mock still wins. No product capture behavior changes—test harness isolation only. ## Verification With ancestor pollution present at `/workspace/oss-work/node_modules/electron` and `/workspace/node_modules/electron` (string-export stubs): ```sh npx vitest run tests/boundary/browser/electronActiveHook.test.ts ``` Result: **4 passed** (3 existing + 1 new regression), Node v24.21.0. Privacy-check on staged changes: exit 0. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:09:31 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5797
No description provided.