Skip to content

Comments

perf: Fix worker test execution excessive delay#519

Merged
nev21 merged 1 commit intomainfrom
nev21/perf
Feb 24, 2026
Merged

perf: Fix worker test execution excessive delay#519
nev21 merged 1 commit intomainfrom
nev21/perf

Conversation

@nev21
Copy link
Contributor

@nev21 nev21 commented Feb 24, 2026

  • add shared worrker adapter/runner
  • switch worker Karma to mocha + karma-typescript and update globs
  • drop unused karma-* deps and refresh rush shrinkwrap

Copilot AI review requested due to automatic review settings February 24, 2026 07:19
@nev21 nev21 requested review from a team as code owners February 24, 2026 07:19
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.71%. Comparing base (d217d19) to head (ec767cb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #519      +/-   ##
==========================================
- Coverage   98.89%   98.71%   -0.18%     
==========================================
  Files         108      111       +3     
  Lines        2981     3198     +217     
  Branches      622      674      +52     
==========================================
+ Hits         2948     3157     +209     
- Misses         33       41       +8     

see 111 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes worker test execution by replacing the previous karma-rollup-preprocessor + karma-mocha-webworker setup with a more efficient mocha + karma-typescript configuration. This change addresses performance issues that required excessive memory allocation (16GB) and reduces test execution delays by implementing a custom worker adapter/runner infrastructure.

Changes:

  • Removed unused karma dependencies (karma-coverage, karma-coverage-istanbul-reporter, karma-mocha-webworker, karma-rollup-preprocessor) and eliminated the need for NODE_OPTIONS memory limit workaround
  • Added custom worker test infrastructure (worker-adapter.js and worker-test-runner.js) to bridge Karma and Web Worker environments
  • Added environment validation tests for browser, node, and worker contexts to ensure tests run in the correct environment

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
package.json Removed unused karma dependencies and eliminated NODE_OPTIONS memory limit from worker test scripts
karma.worker.conf.js Switched from rollup preprocessor to karma-typescript with updated file patterns and coverage configuration
karma.worker.esnext.conf.js Same changes as worker.conf.js but configured for ESNext target
karma.debug.worker.conf.js Debug configuration updated to use karma-typescript instead of rollup
karma.debug.worker.esnext.conf.js Debug ESNext configuration updated to use karma-typescript
common/test/worker-adapter.js New file implementing the main-thread adapter that creates workers and bridges test results to Karma
common/test/worker-test-runner.js New file implementing the worker-thread runner that loads and executes tests in worker context
lib/test/src/worker/isWorker.test.ts New test file validating worker environment detection
lib/test/src/node/isNode.test.ts New test file validating node environment detection
lib/test/src/browser/isBrowser.test.ts New test file validating browser environment detection

nevware21-bot
nevware21-bot previously approved these changes Feb 24, 2026
Copy link
Contributor

@nevware21-bot nevware21-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by nevware21-bot

- add shared worrker adapter/runner
- switch worker Karma to mocha + karma-typescript and update globs
- drop unused karma-* deps and refresh rush shrinkwrap
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

*/

import { assert } from "@nevware21/tripwire-chai";
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests import environment helpers from the published package entrypoint ("@nevware21/ts-utils"), while the rest of the test suite consistently imports from the local source tree (e.g., browser.environment.test.ts imports from "../../../src/helpers/environment"). Importing from the package here can unintentionally test the prebuilt dist output instead of the code under test and can mask source-only changes. Consider switching these imports to the matching local src module path.

Suggested change
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
import { hasDocument, hasWindow, isNode, isWebWorker } from "../../../src/helpers/environment";

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +3
// Default to using edge locally -- choose your own browser as required
process.env.CHROME_BIN = "C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe";
//process.env.CHROME_BIN = require("puppeteer").executablePath();
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHROME_BIN is hard-coded to a Windows-specific Edge install path. This makes the debug worker config fail on macOS/Linux (and on Windows machines with Edge in a different location). Prefer using an environment override if provided, and otherwise default to Puppeteer’s executablePath so the config is portable.

Suggested change
// Default to using edge locally -- choose your own browser as required
process.env.CHROME_BIN = "C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe";
//process.env.CHROME_BIN = require("puppeteer").executablePath();
// Prefer an environment-provided CHROME_BIN; otherwise, fall back to Puppeteer's executablePath for portability
if (!process.env.CHROME_BIN) {
try {
process.env.CHROME_BIN = require("puppeteer").executablePath();
} catch (e) {
// If puppeteer is not available, leave CHROME_BIN undefined and let Karma use its defaults
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +170 to +171
"test:worker": "karma start karma.worker.conf.js --single-run",
"test:worker_esnext": "karma start karma.worker.esnext.conf.js --single-run",
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The worker test scripts no longer use cross-env (the NODE_OPTIONS wrapper was removed), but cross-env remains in devDependencies. If it’s no longer used elsewhere in the repo, consider removing it to reduce dependency footprint and avoid carrying an unused tool.

Copilot uses AI. Check for mistakes.
Comment on lines +145 to +147
case "complete":
karma.complete(msg.coverage ? { coverage: msg.coverage } : {});
break;
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After receiving the worker "complete" message, the Web Worker is left running. In watch/debug sessions this can leak workers and keep resources alive. Consider terminating the worker after calling karma.complete (and similarly on error paths).

Copilot uses AI. Check for mistakes.
*/

import { assert } from "@nevware21/tripwire-chai";
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests import environment helpers from the published package entrypoint ("@nevware21/ts-utils"), while the rest of the test suite consistently imports from the local source tree (e.g., node.environment.test.ts imports from "../../../src/helpers/environment"). Importing from the package here can unintentionally test the prebuilt dist output instead of the code under test and can mask source-only changes. Consider switching these imports to the matching local src module path.

Suggested change
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
import { hasDocument, hasWindow, isNode, isWebWorker } from "../../../src/helpers/environment";

Copilot uses AI. Check for mistakes.
*/

import { assert } from "@nevware21/tripwire-chai";
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests import environment helpers from the published package entrypoint ("@nevware21/ts-utils"), while the rest of the worker/common test suite imports from the local source tree (e.g., worker.environment.test.ts imports from "../../../src/helpers/environment"). Importing from the package here can unintentionally test the prebuilt dist output instead of the code under test and can mask source-only changes. Consider switching these imports to the matching local src module path.

Suggested change
import { hasDocument, hasWindow, isNode, isWebWorker } from "@nevware21/ts-utils";
import { hasDocument, hasWindow, isNode, isWebWorker } from "../../../src/helpers/environment";

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +3
// Default to using edge locally -- choose your own browser as required
process.env.CHROME_BIN = "C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe";
//process.env.CHROME_BIN = require("puppeteer").executablePath();
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHROME_BIN is hard-coded to a Windows-specific Edge install path. This makes the debug worker ESNext config fail on macOS/Linux (and on Windows machines with Edge in a different location). Prefer using an environment override if provided, and otherwise default to Puppeteer’s executablePath so the config is portable.

Suggested change
// Default to using edge locally -- choose your own browser as required
process.env.CHROME_BIN = "C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe";
//process.env.CHROME_BIN = require("puppeteer").executablePath();
// Default to using a local Chromium-based browser. Allow override via CHROME_BIN and
// fall back to Puppeteer's detected executablePath for portability across platforms.
var puppeteer;
if (!process.env.CHROME_BIN) {
try {
puppeteer = require("puppeteer");
if (puppeteer && typeof puppeteer.executablePath === "function") {
process.env.CHROME_BIN = puppeteer.executablePath();
}
} catch (e) {
// eslint-disable-next-line no-console
console.warn("puppeteer is not available; using system Chrome/Chromium resolution");
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +149 to +153
case "error":
console.error("[worker-adapter] Worker error: " + msg.error);
karma.error("[worker-test] " + msg.error);
karma.complete({});
break;
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the worker "error" message, the worker is not terminated. Consider terminating the worker after reporting the error/complete to avoid leaking a broken worker instance in watch/debug runs.

Copilot uses AI. Check for mistakes.
sendMessage({
type: "complete",
coverage: self.__coverage__
});
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The worker sends the final "complete" message but does not close itself, so it can remain alive and retain memory after the run. Consider calling self.close() (after posting the complete message) to terminate cleanly.

Suggested change
});
});
self.close();

Copilot uses AI. Check for mistakes.
@nev21 nev21 enabled auto-merge (squash) February 24, 2026 07:52
Copy link
Contributor

@nevware21-bot nevware21-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by nevware21-bot

@nev21 nev21 merged commit 6ef0e65 into main Feb 24, 2026
14 checks passed
@nev21 nev21 deleted the nev21/perf branch February 24, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants