chore(testing): combine daemon+pre-install, cut linter.test.ts wall time 42%#35354
chore(testing): combine daemon+pre-install, cut linter.test.ts wall time 42%#35354FrozenPandaz wants to merge 9 commits intonrwl:masterfrom
Conversation
👷 Deploy request for nx-docs pending review.Visit the deploys page to approve it
|
👷 Deploy request for nx-dev pending review.Visit the deploys page to approve it
|
…ckages move packages that were lazily installed via ensurePackage() during generator runs into the packages array passed to newProject(), so they are pre-installed during workspace setup instead of mid-test. reduces per-test install overhead across 97 e2e test files.
…ckages [Self-Healing CI Rerun]
- Add install timing logs to installPackagesTask - Improve error logging in e2e utils (PATH, env info on ENOENT) - Fix stripped env to include PATH, HOME, USER, SHELL - Use create-nx-workspace@latest instead of pinned version - Add prep-e2e script to package.json
Reverts debug logging, env stripping, shell option, and create-nx-workspace version changes that broke e2e type checks.
The daemon was being stripped from the environment by getStrippedEnvironmentVariables(), causing every nx command in e2e tests to rebuild the project graph from scratch. This added ~20s per generator call. Hardcode NX_DAEMON=true so the daemon stays alive between commands and caches the project graph.
Adds performance.measure() spans so NX_PERF_LOGGING=true reveals where a generator spends its time: - generate:project-graph, generate:run-generator, generate:flush-changes, generate:post-task — added to generate() in packages/nx - install-packages-task:<pm> — wraps the pnpm/npm install exec - format-files — wraps the prettier loop Also drains pending PerformanceObserver records on process.exit() so measures emitted near the tail of a generator (which normally die when process.exit skips pending microtasks) reach the log. Allows NX_PERF_LOGGING through the e2e stripped-env allowlist so subprocess generators inherit it.
FrozenPandaz
force-pushed
the
speed-e2e-combined
branch
from
ecb34d5 to
3b146f0
Compare
|
View your CI Pipeline Execution ↗ for commit 9bb6c25
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We fix a race condition in DaemonClient.startInBackground() where a concurrent reset() call (triggered by the socket-close handler) could null out this._out between the two await open() calls, causing TypeError: Cannot read properties of null (reading 'fd') when spawn() was invoked. By capturing the FileHandle return values into local variables (out/err) and passing those to spawn() instead of this._out/this._err, the daemon restart is no longer affected by any concurrent reset() execution.
Warning
❌ We could not verify this fix.
diff --git a/packages/nx/src/daemon/client/client.ts b/packages/nx/src/daemon/client/client.ts
index c915ee7a69..e4a0fd9f3c 100644
--- a/packages/nx/src/daemon/client/client.ts
+++ b/packages/nx/src/daemon/client/client.ts
@@ -1302,8 +1302,11 @@ export class DaemonClient {
writeFileSync(DAEMON_OUTPUT_LOG_FILE, '');
}
- this._out = await open(DAEMON_OUTPUT_LOG_FILE, 'a');
- this._err = await open(DAEMON_OUTPUT_LOG_FILE, 'a');
+ // Capture file handles in local variables to avoid a race condition where
+ // reset() may null out this._out/this._err between the async open() calls
+ // and the spawn() call, causing "Cannot read properties of null (reading 'fd')".
+ const out = (this._out = await open(DAEMON_OUTPUT_LOG_FILE, 'a'));
+ const err = (this._err = await open(DAEMON_OUTPUT_LOG_FILE, 'a'));
clientLogger.log(`[Client] Starting new daemon server in background`);
@@ -1312,7 +1315,7 @@ export class DaemonClient {
[join(__dirname, `../server/start.js`)],
{
cwd: workspaceRoot,
- stdio: ['ignore', this._out.fd, this._err.fd],
+ stdio: ['ignore', out.fd, err.fd],
detached: true,
windowsHide: true,
shell: false,
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
Because this branch comes from a fork, it is not possible for us to apply fixes directly, but you can apply the changes locally using the available options below.
Apply changes locally with:
npx nx-cloud apply-locally IV5P-ZJfX
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
1 participant
Current Behavior
The
nx run e2e-eslint:e2e-ci--src/linter.test.tstarget takes ~678s on CI. A handful of other e2e files have the same shape and pay similar wall-time.We opened two PRs targeting this on different levers:
ensurePackagedeps innewProject(), so the tmp-dirpnpm addinside each generator call short-circuits.NX_DAEMON: 'true'to the stripped env.Each PR on its own is a regression on
linter.test.ts:runCLIpays +2 plugin cold-loads (the new pre-installed packages are Nx plugin packages like@nx/vite,@nx/vitest,@nx/jestthat get auto-discovered). With daemon off, plugin-worker boot is paid on every invocation. Net: +61s (678 → 739).The two levers are complementary: 35218 kills the overhead that 35303 introduces, 35303 gives 35218 bigger wins to amortize.
Expected Behavior
Stacking the two PRs drops
linter.test.tswall time from 678s → 393s (−42%) on the same test architecture:Biggest win is
lint plugin module boundaries(−56s): that test has 4 sequential generators + many lint commands — exactly the pattern where skipped tmp installs + warm daemon stack up.What this PR contains
This branch sits on top of both PRs and adds one more commit:
nx generateso future e2e perf investigations can see where time actually goes without having to patchnode_modules:generate:project-graph,generate:run-generator,generate:flush-changes,generate:post-taskspans inpackages/nx/src/command-line/generate/generate.tsinstall-packages-task:<packageManager>inpackages/devkit/src/tasks/install-packages-task.tsformat-filesinpackages/devkit/src/generators/format-files.tsprocess.on('exit')drain inpackages/nx/src/utils/perf-logging.tsso measures emitted just beforeprocess.exit()actually reach the observerNX_PERF_LOGGINGadded to the e2e stripped-env allowlist so subprocess generators inherit itRun with
NX_PERF_LOGGING=true pnpm nx run e2e-eslint:e2e-ci--src/linter.test.tsto see per-generator timing breakdowns.Related Issue(s)
Draft — depends on #35303 and #35218 landing first (or this PR can be retargeted once one of them merges).