From 47968f88fe339f4fc0e9039d481ce772b819f8a6 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Thu, 20 Aug 2026 16:59:47 +0000 Subject: [PATCH 1/3] Ignore nested node_modules and .git in the workspace root watch The workspace-root parcel subscription only excluded the root's own top-level git-ignored directories plus the path ".git", so an umbrella root with untracked nested checkouts (or a non-git root) got one inotify watch per nested directory and could OOM the host. Add recursive glob ignores to every workspace-root subscribe. Fixes #1779 Co-Authored-By: Claude --- .../src/workspace-status-watcher.ts | 39 ++++- .../host-watcher/test/watch-status.test.ts | 15 +- .../test/workspace-root-ignores.test.ts | 155 ++++++++++++++++++ 3 files changed, 202 insertions(+), 7 deletions(-) create mode 100644 packages/host-watcher/test/workspace-root-ignores.test.ts diff --git a/packages/host-watcher/src/workspace-status-watcher.ts b/packages/host-watcher/src/workspace-status-watcher.ts index df266c0db6..2a0239baea 100644 --- a/packages/host-watcher/src/workspace-status-watcher.ts +++ b/packages/host-watcher/src/workspace-status-watcher.ts @@ -30,7 +30,23 @@ const WORKSPACE_STATUS_WATCH_MAX_RETRY_DELAY_MS = 30_000; // Give up after a bounded number of attempts; the server recreates this watch // (resetting the count) when the watch set changes. const WORKSPACE_STATUS_WATCH_MAX_SETUP_RETRY_ATTEMPTS = 10; +// Plain entries are paths relative to the watch root: `.git` only excludes +// `/.git`, the workspace's own repository. const WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS = [".git"]; +// Glob entries are matched against the root-relative path of every directory +// parcel crawls, and a match skips the whole subtree. The Git-derived ignore +// list below only covers the root's own top-level ignored directories; an +// "umbrella" root with untracked nested checkouts, or a root that is not a +// repository, otherwise gets one inotify watch per nested directory and can +// OOM the host (get-bb/bb#1779). `*/**/.git` skips nested repositories but +// keeps `/.git` watchable so a plain directory can still be promoted +// after `git init`. +const WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS = [ + "*/**/.git", + "**/node_modules", + "**/.cache", + "**/__pycache__", +]; const WORKSPACE_ROOT_IGNORE_STATUS_TIMEOUT_MS = 5_000; const WORKSPACE_ROOT_IGNORE_STATUS_MAX_BUFFER_BYTES = 10 * 1024 * 1024; @@ -101,10 +117,21 @@ function collectIgnoredDirectoryPaths(statusOutput: string): string[] { return Array.from(ignoredDirectoryPaths).sort(); } +function createGitWorkspaceRootIgnores(): string[] { + return [ + ...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS, + ...WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS, + ]; +} + +function createPlainWorkspaceRootIgnores(): string[] { + return [...WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS]; +} + function mergeWorkspaceRootIgnores(gitIgnoredPaths: string[]): string[] { const ignoredPaths = new Set(); for (const ignoredPath of [ - ...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS, + ...createGitWorkspaceRootIgnores(), ...gitIgnoredPaths, ]) { ignoredPaths.add(ignoredPath); @@ -220,9 +247,11 @@ export class WorkspaceStatusWatcher { } if (!(await pathExists(path.join(this.args.cwd, ".git")))) { // A plain workspace can become a repository after `git init`. Watch the - // root without excluding `.git` until that marker appears. + // root without excluding `/.git` until that marker appears, but + // still skip nested repositories and heavy directories. this.startWatchSubscription({ kind: "workspace-root", + options: { ignore: createPlainWorkspaceRootIgnores() }, rootPath, }); return; @@ -267,11 +296,11 @@ export class WorkspaceStatusWatcher { } this.reportWorkspaceRootSetupError(rootPath, error); // Ignore discovery is an optimization, not a correctness gate. Keep the - // workspace live with the mandatory `.git` exclusion even when Git is - // too slow or its metadata is temporarily unavailable. + // workspace live with the mandatory `.git` and nested-tree exclusions + // even when Git is too slow or its metadata is temporarily unavailable. this.startWatchSubscription({ kind: "workspace-root", - options: { ignore: [...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS] }, + options: { ignore: createGitWorkspaceRootIgnores() }, rootPath, }); } diff --git a/packages/host-watcher/test/watch-status.test.ts b/packages/host-watcher/test/watch-status.test.ts index 3a4122e3a7..e9d84ed4b8 100644 --- a/packages/host-watcher/test/watch-status.test.ts +++ b/packages/host-watcher/test/watch-status.test.ts @@ -420,7 +420,8 @@ describe.sequential("watchWorkspaceStatus", () => { try { await ready.promise; - expect(workspaceRootOptions[0]?.ignore).toBeUndefined(); + expect(workspaceRootOptions[0]?.ignore).not.toContain(".git"); + expect(workspaceRootOptions[0]?.ignore).toContain("*/**/.git"); await runGit({ args: ["init", "-b", "main"], cwd: workspacePath }); const canonicalWorkspacePath = await fs.realpath(workspacePath); @@ -732,6 +733,10 @@ describe.sequential("watchWorkspaceStatus", () => { await ready; expect(getWorkspaceRootSubscribeOptions()?.ignore).toEqual([ ".git", + "*/**/.git", + "**/node_modules", + "**/.cache", + "**/__pycache__", ".turbo", "coverage", ]); @@ -785,7 +790,13 @@ describe.sequential("watchWorkspaceStatus", () => { expect(ignoreDiscoveryErrors).toHaveLength(1); expect(ignoreDiscoveryErrors[0]).toContain(normalizeWatchPath(repoPath)); expect(subscribedRoots).toEqual([normalizeWatchPath(repoPath)]); - expect(subscribedOptions[0]?.ignore).toEqual([".git"]); + expect(subscribedOptions[0]?.ignore).toEqual([ + ".git", + "*/**/.git", + "**/node_modules", + "**/.cache", + "**/__pycache__", + ]); } finally { await stopWatching(); } diff --git a/packages/host-watcher/test/workspace-root-ignores.test.ts b/packages/host-watcher/test/workspace-root-ignores.test.ts new file mode 100644 index 0000000000..ed1d07a51c --- /dev/null +++ b/packages/host-watcher/test/workspace-root-ignores.test.ts @@ -0,0 +1,155 @@ +// Regression test for get-bb/bb#1779: the workspace-root watch of an +// "umbrella" root (a directory with untracked nested checkouts that contain +// node_modules and .git) must not register an inotify watch on every nested +// directory. The Git-derived ignore list only covers the root's own top-level +// ignored directories, so the watcher has to add recursive glob ignores. +// +// Linux only: it reads the real inotify watch count from /proc/self/fdinfo. +import { execFile } from "node:child_process"; +import fsSync from "node:fs"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import parcelWatcher from "@parcel/watcher"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { watchWorkspaceStatus } from "../src/watch-status.js"; + +const execFileAsync = promisify(execFile); +const tempDirs: string[] = []; + +const NESTED_REPOS = 4; +const PACKAGES_PER_NESTED_REPO = 300; +// Root, apps/, apps/child-N and the root's own git-dir metadata watches. +const MAX_EXPECTED_WATCHES = 20; + +async function git(cwd: string, ...args: string[]): Promise { + await execFileAsync("git", args, { cwd, encoding: "utf8" }); +} + +async function initRepo(dir: string): Promise { + await fs.mkdir(dir, { recursive: true }); + await git(dir, "init", "-q", "-b", "main"); + await git(dir, "config", "user.name", "BB Tests"); + await git(dir, "config", "user.email", "bb@example.com"); + await fs.writeFile(path.join(dir, "README.md"), "hello\n"); + await git(dir, "add", "README.md"); + await git(dir, "commit", "-q", "-m", "init"); +} + +async function buildUmbrellaRoot(args: { gitRoot: boolean }): Promise<{ + root: string; + nestedDirCount: number; +}> { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "bb-1779-umbrella-")); + tempDirs.push(root); + if (args.gitRoot) { + await initRepo(root); + } + let nestedDirCount = 0; + for (let i = 0; i < NESTED_REPOS; i += 1) { + const child = path.join(root, "apps", `child-${i}`); + await initRepo(child); + // The nested repo ignores its own node_modules, like every real project. + await fs.writeFile(path.join(child, ".gitignore"), "node_modules/\n"); + await git(child, "add", ".gitignore"); + await git(child, "commit", "-q", "-m", "ignore node_modules"); + for (let p = 0; p < PACKAGES_PER_NESTED_REPO; p += 1) { + const pkgLib = path.join(child, "node_modules", `pkg-${p}`, "lib"); + await fs.mkdir(pkgLib, { recursive: true }); + await fs.writeFile(path.join(pkgLib, "index.js"), "module.exports={}\n"); + nestedDirCount += 2; + } + } + return { root, nestedDirCount }; +} + +function countInotifyWatches(): number { + const fdinfoDir = "/proc/self/fdinfo"; + let count = 0; + for (const fd of fsSync.readdirSync(fdinfoDir)) { + try { + const info = fsSync.readFileSync(path.join(fdinfoDir, fd), "utf8"); + count += info + .split("\n") + .filter((line) => line.startsWith("inotify wd:")).length; + } catch { + // fd closed between readdir and read + } + } + return count; +} + +async function measureWorkspaceRootWatch(root: string): Promise<{ + ignore: string[] | undefined; + watches: number; +}> { + const seenOptions: Array<{ dir: string; ignore: string[] | undefined }> = []; + const realSubscribe = parcelWatcher.subscribe.bind(parcelWatcher); + vi.spyOn(parcelWatcher, "subscribe").mockImplementation( + async (dir, cb, opts) => { + seenOptions.push({ dir, ignore: opts?.ignore }); + return realSubscribe(dir, cb, opts); + }, + ); + const baselineWatches = countInotifyWatches(); + let ready!: () => void; + const readyPromise = new Promise((resolve) => { + ready = resolve; + }); + const stop = watchWorkspaceStatus(root, { + onChange: () => undefined, + onReady: () => ready(), + onWatchError: () => undefined, + }); + try { + await readyPromise; + // Settle the metadata (git-dir) subscriptions too. + await new Promise((resolve) => setTimeout(resolve, 300)); + const realRoot = fsSync.realpathSync(root); + const workspaceRootSubscribe = seenOptions.find( + (o) => o.dir === root || o.dir === realRoot, + ); + return { + ignore: workspaceRootSubscribe?.ignore, + watches: countInotifyWatches() - baselineWatches, + }; + } finally { + await stop(); + } +} + +afterEach(async () => { + vi.restoreAllMocks(); + for (const dir of tempDirs.splice(0)) { + await fs.rm(dir, { force: true, recursive: true }); + } +}); + +describe.skipIf(process.platform !== "linux")( + "workspace root watch ignores nested heavy directories (#1779)", + () => { + it("does not watch nested node_modules or .git under a git umbrella root", async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: true, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + expect(ignore).toContain(".git"); + expect(ignore).toContain("**/node_modules"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }); + + it("does not watch nested node_modules or .git under a non-git root", async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: false, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + // `/.git` must stay watchable so `git init` promotion still fires. + expect(ignore).not.toContain(".git"); + expect(ignore).toContain("**/node_modules"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }); + }, +); From ebcce620935a9d006b0a86d7e1a7ca5bbcc60c74 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Thu, 20 Aug 2026 18:17:11 +0000 Subject: [PATCH 2/3] Ignore children of nested heavy directories on every platform Parcel tests each event path against the ignore globs on macOS and Windows, so `**/node_modules` alone let events from files inside nested node_modules through. Use `/**` globs, which picomatch matches against both the directory and its children, and add a portable real-watcher test that writes inside nested node_modules and .git. Co-Authored-By: Claude --- .../src/workspace-status-watcher.ts | 27 ++-- .../host-watcher/test/watch-status.test.ts | 18 +-- .../test/workspace-root-ignores.test.ts | 115 ++++++++++++++++-- 3 files changed, 130 insertions(+), 30 deletions(-) diff --git a/packages/host-watcher/src/workspace-status-watcher.ts b/packages/host-watcher/src/workspace-status-watcher.ts index 2a0239baea..8dd7e459d3 100644 --- a/packages/host-watcher/src/workspace-status-watcher.ts +++ b/packages/host-watcher/src/workspace-status-watcher.ts @@ -33,19 +33,22 @@ const WORKSPACE_STATUS_WATCH_MAX_SETUP_RETRY_ATTEMPTS = 10; // Plain entries are paths relative to the watch root: `.git` only excludes // `/.git`, the workspace's own repository. const WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS = [".git"]; -// Glob entries are matched against the root-relative path of every directory -// parcel crawls, and a match skips the whole subtree. The Git-derived ignore -// list below only covers the root's own top-level ignored directories; an -// "umbrella" root with untracked nested checkouts, or a root that is not a -// repository, otherwise gets one inotify watch per nested directory and can -// OOM the host (get-bb/bb#1779). `*/**/.git` skips nested repositories but -// keeps `/.git` watchable so a plain directory can still be promoted -// after `git init`. +// Glob entries are matched against the root-relative path. On Linux parcel +// tests every directory during its crawl and a match skips the whole subtree, +// so no inotify watch is created below it. On macOS and Windows parcel tests +// each event path instead, so the trailing `/**` is required: picomatch lets +// it match zero segments, so `**/node_modules/**` matches both the directory +// and everything inside it. The Git-derived ignore list below only covers the +// root's own top-level ignored directories; an "umbrella" root with untracked +// nested checkouts, or a root that is not a repository, otherwise gets one +// inotify watch per nested directory and can OOM the host (get-bb/bb#1779). +// `*/**/.git/**` skips nested repositories but keeps `/.git` watchable +// so a plain directory can still be promoted after `git init`. const WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS = [ - "*/**/.git", - "**/node_modules", - "**/.cache", - "**/__pycache__", + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", ]; const WORKSPACE_ROOT_IGNORE_STATUS_TIMEOUT_MS = 5_000; const WORKSPACE_ROOT_IGNORE_STATUS_MAX_BUFFER_BYTES = 10 * 1024 * 1024; diff --git a/packages/host-watcher/test/watch-status.test.ts b/packages/host-watcher/test/watch-status.test.ts index e9d84ed4b8..7cadd9c9d4 100644 --- a/packages/host-watcher/test/watch-status.test.ts +++ b/packages/host-watcher/test/watch-status.test.ts @@ -421,7 +421,7 @@ describe.sequential("watchWorkspaceStatus", () => { try { await ready.promise; expect(workspaceRootOptions[0]?.ignore).not.toContain(".git"); - expect(workspaceRootOptions[0]?.ignore).toContain("*/**/.git"); + expect(workspaceRootOptions[0]?.ignore).toContain("*/**/.git/**"); await runGit({ args: ["init", "-b", "main"], cwd: workspacePath }); const canonicalWorkspacePath = await fs.realpath(workspacePath); @@ -733,10 +733,10 @@ describe.sequential("watchWorkspaceStatus", () => { await ready; expect(getWorkspaceRootSubscribeOptions()?.ignore).toEqual([ ".git", - "*/**/.git", - "**/node_modules", - "**/.cache", - "**/__pycache__", + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", ".turbo", "coverage", ]); @@ -792,10 +792,10 @@ describe.sequential("watchWorkspaceStatus", () => { expect(subscribedRoots).toEqual([normalizeWatchPath(repoPath)]); expect(subscribedOptions[0]?.ignore).toEqual([ ".git", - "*/**/.git", - "**/node_modules", - "**/.cache", - "**/__pycache__", + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", ]); } finally { await stopWatching(); diff --git a/packages/host-watcher/test/workspace-root-ignores.test.ts b/packages/host-watcher/test/workspace-root-ignores.test.ts index ed1d07a51c..fb559bf299 100644 --- a/packages/host-watcher/test/workspace-root-ignores.test.ts +++ b/packages/host-watcher/test/workspace-root-ignores.test.ts @@ -1,10 +1,13 @@ -// Regression test for get-bb/bb#1779: the workspace-root watch of an +// Regression tests for get-bb/bb#1779: the workspace-root watch of an // "umbrella" root (a directory with untracked nested checkouts that contain // node_modules and .git) must not register an inotify watch on every nested -// directory. The Git-derived ignore list only covers the root's own top-level -// ignored directories, so the watcher has to add recursive glob ignores. +// directory, and must not report changes inside those trees. The Git-derived +// ignore list only covers the root's own top-level ignored directories, so the +// watcher has to add recursive glob ignores. // -// Linux only: it reads the real inotify watch count from /proc/self/fdinfo. +// The inotify tests are Linux only: they read the real watch count from +// /proc/self/fdinfo. The event test runs on every platform because parcel +// applies the globs during the crawl on Linux but per event on macOS/Windows. import { execFile } from "node:child_process"; import fsSync from "node:fs"; import fs from "node:fs/promises"; @@ -14,12 +17,14 @@ import { promisify } from "node:util"; import parcelWatcher from "@parcel/watcher"; import { afterEach, describe, expect, it, vi } from "vitest"; import { watchWorkspaceStatus } from "../src/watch-status.js"; +import type { WorkspaceStatusChangeEvent } from "../src/watch-status-types.js"; const execFileAsync = promisify(execFile); const tempDirs: string[] = []; const NESTED_REPOS = 4; const PACKAGES_PER_NESTED_REPO = 300; +const EVENT_TIMEOUT_MS = 5_000; // Root, apps/, apps/child-N and the root's own git-dir metadata watches. const MAX_EXPECTED_WATCHES = 20; @@ -37,7 +42,11 @@ async function initRepo(dir: string): Promise { await git(dir, "commit", "-q", "-m", "init"); } -async function buildUmbrellaRoot(args: { gitRoot: boolean }): Promise<{ +async function buildUmbrellaRoot(args: { + gitRoot: boolean; + nestedRepos?: number; + packagesPerNestedRepo?: number; +}): Promise<{ root: string; nestedDirCount: number; }> { @@ -46,15 +55,18 @@ async function buildUmbrellaRoot(args: { gitRoot: boolean }): Promise<{ if (args.gitRoot) { await initRepo(root); } + const nestedRepos = args.nestedRepos ?? NESTED_REPOS; + const packagesPerNestedRepo = + args.packagesPerNestedRepo ?? PACKAGES_PER_NESTED_REPO; let nestedDirCount = 0; - for (let i = 0; i < NESTED_REPOS; i += 1) { + for (let i = 0; i < nestedRepos; i += 1) { const child = path.join(root, "apps", `child-${i}`); await initRepo(child); // The nested repo ignores its own node_modules, like every real project. await fs.writeFile(path.join(child, ".gitignore"), "node_modules/\n"); await git(child, "add", ".gitignore"); await git(child, "commit", "-q", "-m", "ignore node_modules"); - for (let p = 0; p < PACKAGES_PER_NESTED_REPO; p += 1) { + for (let p = 0; p < packagesPerNestedRepo; p += 1) { const pkgLib = path.join(child, "node_modules", `pkg-${p}`, "lib"); await fs.mkdir(pkgLib, { recursive: true }); await fs.writeFile(path.join(pkgLib, "index.js"), "module.exports={}\n"); @@ -119,6 +131,19 @@ async function measureWorkspaceRootWatch(root: string): Promise<{ } } +async function waitFor( + predicate: () => boolean, + timeoutMs: number, +): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error("Timed out waiting for workspace change events"); + } + await new Promise((resolve) => setTimeout(resolve, 20)); + } +} + afterEach(async () => { vi.restoreAllMocks(); for (const dir of tempDirs.splice(0)) { @@ -136,7 +161,7 @@ describe.skipIf(process.platform !== "linux")( const { ignore, watches } = await measureWorkspaceRootWatch(root); expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); expect(ignore).toContain(".git"); - expect(ignore).toContain("**/node_modules"); + expect(ignore).toContain("**/node_modules/**"); expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); }); @@ -148,8 +173,80 @@ describe.skipIf(process.platform !== "linux")( expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); // `/.git` must stay watchable so `git init` promotion still fires. expect(ignore).not.toContain(".git"); - expect(ignore).toContain("**/node_modules"); + expect(ignore).toContain("**/node_modules/**"); expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); }); }, ); + +describe("workspace root watch events inside nested heavy directories (#1779)", () => { + it("does not report changes inside nested node_modules or nested .git", async () => { + const { root } = await buildUmbrellaRoot({ + gitRoot: true, + nestedRepos: 1, + packagesPerNestedRepo: 1, + }); + const realRoot = fsSync.realpathSync(root); + const nestedPackageFile = path.join( + realRoot, + "apps", + "child-0", + "node_modules", + "pkg-0", + "lib", + "index.js", + ); + const nestedGitFile = path.join( + realRoot, + "apps", + "child-0", + ".git", + "bb-marker", + ); + const visibleFile = path.join(realRoot, "apps", "child-0", "visible.txt"); + const events: WorkspaceStatusChangeEvent[] = []; + let ready!: () => void; + const readyPromise = new Promise((resolve) => { + ready = resolve; + }); + const stop = watchWorkspaceStatus(root, { + onChange: (event) => { + events.push(event); + }, + onReady: () => ready(), + onWatchError: () => undefined, + }); + try { + await readyPromise; + // Let the initial crawl and the FSEvents stream settle. + await new Promise((resolve) => setTimeout(resolve, 300)); + + await fs.writeFile(nestedPackageFile, "module.exports={changed:true}\n"); + await fs.writeFile(nestedGitFile, "marker\n"); + // The visible write is the control: it proves the watch is live and + // delivers events, so the absence of the nested paths is meaningful. + await fs.writeFile(visibleFile, "visible\n"); + await waitFor( + () => + events.some((event) => event.changedPaths.includes(visibleFile)), + EVENT_TIMEOUT_MS, + ); + // Give any straggling nested events a chance to arrive. + await new Promise((resolve) => setTimeout(resolve, 300)); + + const changedPaths = events.flatMap((event) => event.changedPaths); + expect(changedPaths).toContain(visibleFile); + expect(changedPaths).not.toContain(nestedPackageFile); + expect(changedPaths).not.toContain(nestedGitFile); + expect( + changedPaths.filter( + (changedPath) => + changedPath.includes(`${path.sep}node_modules${path.sep}`) || + changedPath.includes(`${path.sep}.git${path.sep}`), + ), + ).toEqual([]); + } finally { + await stop(); + } + }); +}); From c59bac34ef3f4520b9d6f447a8806ea35fb96925 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Thu, 20 Aug 2026 18:22:16 +0000 Subject: [PATCH 3/3] Make the umbrella watch tests robust on slow CI runners Build the nested tree in parallel, give the tests a longer timeout, and bind the real parcel subscribe once at module load so a spy left behind by a failed test cannot recurse into itself. Co-Authored-By: Claude --- .../test/workspace-root-ignores.test.ts | 216 ++++++++++-------- 1 file changed, 120 insertions(+), 96 deletions(-) diff --git a/packages/host-watcher/test/workspace-root-ignores.test.ts b/packages/host-watcher/test/workspace-root-ignores.test.ts index fb559bf299..55e12344cf 100644 --- a/packages/host-watcher/test/workspace-root-ignores.test.ts +++ b/packages/host-watcher/test/workspace-root-ignores.test.ts @@ -21,10 +21,15 @@ import type { WorkspaceStatusChangeEvent } from "../src/watch-status-types.js"; const execFileAsync = promisify(execFile); const tempDirs: string[] = []; +// Bind once at module load so a spy left behind by a failed test can never +// become the "real" implementation of the next spy. +const realParcelSubscribe = parcelWatcher.subscribe.bind(parcelWatcher); const NESTED_REPOS = 4; const PACKAGES_PER_NESTED_REPO = 300; const EVENT_TIMEOUT_MS = 5_000; +// The umbrella tree has thousands of directories; CI runners build it slowly. +const TEST_TIMEOUT_MS = 60_000; // Root, apps/, apps/child-N and the root's own git-dir metadata watches. const MAX_EXPECTED_WATCHES = 20; @@ -66,12 +71,17 @@ async function buildUmbrellaRoot(args: { await fs.writeFile(path.join(child, ".gitignore"), "node_modules/\n"); await git(child, "add", ".gitignore"); await git(child, "commit", "-q", "-m", "ignore node_modules"); - for (let p = 0; p < packagesPerNestedRepo; p += 1) { - const pkgLib = path.join(child, "node_modules", `pkg-${p}`, "lib"); - await fs.mkdir(pkgLib, { recursive: true }); - await fs.writeFile(path.join(pkgLib, "index.js"), "module.exports={}\n"); - nestedDirCount += 2; - } + await Promise.all( + Array.from({ length: packagesPerNestedRepo }, async (_unused, p) => { + const pkgLib = path.join(child, "node_modules", `pkg-${p}`, "lib"); + await fs.mkdir(pkgLib, { recursive: true }); + await fs.writeFile( + path.join(pkgLib, "index.js"), + "module.exports={}\n", + ); + }), + ); + nestedDirCount += packagesPerNestedRepo * 2; } return { root, nestedDirCount }; } @@ -97,11 +107,10 @@ async function measureWorkspaceRootWatch(root: string): Promise<{ watches: number; }> { const seenOptions: Array<{ dir: string; ignore: string[] | undefined }> = []; - const realSubscribe = parcelWatcher.subscribe.bind(parcelWatcher); vi.spyOn(parcelWatcher, "subscribe").mockImplementation( async (dir, cb, opts) => { seenOptions.push({ dir, ignore: opts?.ignore }); - return realSubscribe(dir, cb, opts); + return realParcelSubscribe(dir, cb, opts); }, ); const baselineWatches = countInotifyWatches(); @@ -154,99 +163,114 @@ afterEach(async () => { describe.skipIf(process.platform !== "linux")( "workspace root watch ignores nested heavy directories (#1779)", () => { - it("does not watch nested node_modules or .git under a git umbrella root", async () => { - const { root, nestedDirCount } = await buildUmbrellaRoot({ - gitRoot: true, - }); - const { ignore, watches } = await measureWorkspaceRootWatch(root); - expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); - expect(ignore).toContain(".git"); - expect(ignore).toContain("**/node_modules/**"); - expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); - }); + it( + "does not watch nested node_modules or .git under a git umbrella root", + async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: true, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + expect(ignore).toContain(".git"); + expect(ignore).toContain("**/node_modules/**"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }, + TEST_TIMEOUT_MS, + ); - it("does not watch nested node_modules or .git under a non-git root", async () => { - const { root, nestedDirCount } = await buildUmbrellaRoot({ - gitRoot: false, - }); - const { ignore, watches } = await measureWorkspaceRootWatch(root); - expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); - // `/.git` must stay watchable so `git init` promotion still fires. - expect(ignore).not.toContain(".git"); - expect(ignore).toContain("**/node_modules/**"); - expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); - }); + it( + "does not watch nested node_modules or .git under a non-git root", + async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: false, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + // `/.git` must stay watchable so `git init` promotion still fires. + expect(ignore).not.toContain(".git"); + expect(ignore).toContain("**/node_modules/**"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }, + TEST_TIMEOUT_MS, + ); }, ); describe("workspace root watch events inside nested heavy directories (#1779)", () => { - it("does not report changes inside nested node_modules or nested .git", async () => { - const { root } = await buildUmbrellaRoot({ - gitRoot: true, - nestedRepos: 1, - packagesPerNestedRepo: 1, - }); - const realRoot = fsSync.realpathSync(root); - const nestedPackageFile = path.join( - realRoot, - "apps", - "child-0", - "node_modules", - "pkg-0", - "lib", - "index.js", - ); - const nestedGitFile = path.join( - realRoot, - "apps", - "child-0", - ".git", - "bb-marker", - ); - const visibleFile = path.join(realRoot, "apps", "child-0", "visible.txt"); - const events: WorkspaceStatusChangeEvent[] = []; - let ready!: () => void; - const readyPromise = new Promise((resolve) => { - ready = resolve; - }); - const stop = watchWorkspaceStatus(root, { - onChange: (event) => { - events.push(event); - }, - onReady: () => ready(), - onWatchError: () => undefined, - }); - try { - await readyPromise; - // Let the initial crawl and the FSEvents stream settle. - await new Promise((resolve) => setTimeout(resolve, 300)); - - await fs.writeFile(nestedPackageFile, "module.exports={changed:true}\n"); - await fs.writeFile(nestedGitFile, "marker\n"); - // The visible write is the control: it proves the watch is live and - // delivers events, so the absence of the nested paths is meaningful. - await fs.writeFile(visibleFile, "visible\n"); - await waitFor( - () => - events.some((event) => event.changedPaths.includes(visibleFile)), - EVENT_TIMEOUT_MS, + it( + "does not report changes inside nested node_modules or nested .git", + async () => { + const { root } = await buildUmbrellaRoot({ + gitRoot: true, + nestedRepos: 1, + packagesPerNestedRepo: 1, + }); + const realRoot = fsSync.realpathSync(root); + const nestedPackageFile = path.join( + realRoot, + "apps", + "child-0", + "node_modules", + "pkg-0", + "lib", + "index.js", + ); + const nestedGitFile = path.join( + realRoot, + "apps", + "child-0", + ".git", + "bb-marker", ); - // Give any straggling nested events a chance to arrive. - await new Promise((resolve) => setTimeout(resolve, 300)); + const visibleFile = path.join(realRoot, "apps", "child-0", "visible.txt"); + const events: WorkspaceStatusChangeEvent[] = []; + let ready!: () => void; + const readyPromise = new Promise((resolve) => { + ready = resolve; + }); + const stop = watchWorkspaceStatus(root, { + onChange: (event) => { + events.push(event); + }, + onReady: () => ready(), + onWatchError: () => undefined, + }); + try { + await readyPromise; + // Let the initial crawl and the FSEvents stream settle. + await new Promise((resolve) => setTimeout(resolve, 300)); - const changedPaths = events.flatMap((event) => event.changedPaths); - expect(changedPaths).toContain(visibleFile); - expect(changedPaths).not.toContain(nestedPackageFile); - expect(changedPaths).not.toContain(nestedGitFile); - expect( - changedPaths.filter( - (changedPath) => - changedPath.includes(`${path.sep}node_modules${path.sep}`) || - changedPath.includes(`${path.sep}.git${path.sep}`), - ), - ).toEqual([]); - } finally { - await stop(); - } - }); + await fs.writeFile( + nestedPackageFile, + "module.exports={changed:true}\n", + ); + await fs.writeFile(nestedGitFile, "marker\n"); + // The visible write is the control: it proves the watch is live and + // delivers events, so the absence of the nested paths is meaningful. + await fs.writeFile(visibleFile, "visible\n"); + await waitFor( + () => + events.some((event) => event.changedPaths.includes(visibleFile)), + EVENT_TIMEOUT_MS, + ); + // Give any straggling nested events a chance to arrive. + await new Promise((resolve) => setTimeout(resolve, 300)); + + const changedPaths = events.flatMap((event) => event.changedPaths); + expect(changedPaths).toContain(visibleFile); + expect(changedPaths).not.toContain(nestedPackageFile); + expect(changedPaths).not.toContain(nestedGitFile); + expect( + changedPaths.filter( + (changedPath) => + changedPath.includes(`${path.sep}node_modules${path.sep}`) || + changedPath.includes(`${path.sep}.git${path.sep}`), + ), + ).toEqual([]); + } finally { + await stop(); + } + }, + TEST_TIMEOUT_MS, + ); });