ci: run the benchmark, measure runner coverage and fail on circular mports

.github/workflows/ci.yml

@@ -40,10 +40,24 @@ jobs:
       run: npm run build --verbose
       timeout-minutes: 15
 
+    # Runs every project's `test` script. The benchmark harness's `test` is deliberately its
+    # validate.js check and its unit tests, a few seconds, and NOT the benchmark itself, so this
+    # step stays fast on all four Node versions -- the karma arms run once, in the step below.
+    # `npm run test:full` at the repo root is the two combined, for anyone who wants everything in
+    # one command.
     - name: Unit tests
       run: node common/scripts/install-run-rush.js test --verbose
       timeout-minutes: 20
 
+    # The benchmark harness is the only project that configures karma and ts-mocha, and karma 6's
+    # dependency tree does not tolerate every Node version in the matrix above, so it is measured on
+    # one version only. Its toolchain is pinned to exact versions -- see internal/benchmark's README,
+    # "Pinned toolchain". Changing the version here means changing that README too.
+    - name: Benchmark
+      if: matrix.node == 22
+      run: npm run test:bench
+      timeout-minutes: 30
+
     - name: Docs link check
       run: npm run readme-link-check
       timeout-minutes: 10
@@ -65,3 +79,12 @@ jobs:
         files: ./shim/chai/coverage/coverage-final.json
         token: ${{ secrets.CODECOV_TOKEN }}
         flags: chai_shim
+
+    - name: Upload Runner Coverage
+      if: matrix.node == 22
+      uses: codecov/codecov-action@v5
+      with:
+        slug: ${{ github.repository }}
+        files: ./internal/runner/coverage/coverage-final.json
+        token: ${{ secrets.CODECOV_TOKEN }}
+        flags: runner

codecov.yml

@@ -18,6 +18,7 @@ coverage:
         paths:
           - core/src
           - shim/chai/src
+          - internal/runner/src
     project:
       core:
         paths:
@@ -31,6 +32,22 @@ coverage:
         target: 70%
         flags:
           - chai_shim
+      # runner/ was outside coverage measurement entirely until the runner flag was
+      # added. The target is a floor set just under the first measured value (32.4%
+      # lines) rather than an aspiration -- raise it as execute/, ux/, codecov/,
+      # compile/ and client/ gain tests, and do not lower it.
+      #
+      # The figure is low partly because it is honest: ux/browser and assets are
+      # browser-side code that the node test process cannot execute, and they are
+      # deliberately left in the denominator rather than excluded, so the number
+      # reflects what is actually untested rather than what is conveniently
+      # measurable.
+      runner:
+        paths:
+          - internal/runner/src
+        target: 30%
+        flags:
+          - runner
 
 flags:
   core:
@@ -40,3 +57,7 @@ flags:
   chai_shim:
     paths:
       - shim/chai/src
+
+  runner:
+    paths:
+      - internal/runner/src

core/package.json

@@ -63,8 +63,8 @@
         "test:nodesrc": "cross-env TRIPWIRE_PREFER_SOURCE=1 nyc --cache node ./bin/tripwire.js --type-check -p ./test/tsconfig.test.json \"./test/src/!(browser|worker)/**/*.test.ts\" -- --trace-uncaught && node ./bin/tripwire.js coverage-report",
         "test:browser": "node ./bin/tripwire.js --target browser --coverage --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|worker)/**/*.test.ts\"",
         "test:worker": "node ./bin/tripwire.js --target worker --coverage --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|browser)/**/*.test.ts\"",
-        "debug:browser": "node ./bin/tripwire.js --target browser --browser-launch open --source-maps --show-files --verbose --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|worker)/**/*.test.ts\"",
-        "debug:worker": "node ./bin/tripwire.js --target worker --browser-launch open --source-maps --show-files --verbose --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|browser)/**/*.test.ts\"",
+        "debug:browser": "node ./bin/tripwire.js --target browser --browser-launch open --source-maps --show-files --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|worker)/**/*.test.ts\"",
+        "debug:worker": "node ./bin/tripwire.js --target worker --browser-launch open --source-maps --show-files --type-check -p ./test/tsconfig.test.json \"./test/src/!(node|browser)/**/*.test.ts\"",
         "lint": "grunt tripwire-lint",
         "coverage:report": "npm run coverage:nyc && npm run coverage:merge",
         "coverage:nyc": "nyc report --reporter=json",

core/rollup.config.js

@@ -179,6 +179,20 @@ const rollupConfigMainEntry = (srcPath, destPath, path, format = "umd") => {
     return taskRollupConfig;
 };
 
+// A circular import between this repository's own modules fails the build: which module's exports
+// exist when another reads them then depends on which file happens to load first. A cycle entirely
+// inside a third-party package is left as a warning, since it is not ours to fix.
+function failOnCircularImport(warning, warn) {
+    if (warning.code === "CIRCULAR_DEPENDENCY") {
+        const ids = warning.ids || [];
+        if (!ids.some((id) => /[\\/]node_modules[\\/]/.test(id))) {
+            throw new Error("Circular import: " + (ids.length ? ids.join(" -> ") : warning.message));
+        }
+    }
+
+    warn(warning);
+}
+
 export default [
     rollupConfigMainEntry("build/es5/mod", "dist/es5", "main", "umd"),
     rollupConfigMainEntry("build/es6/mod", "dist/es6", "main", "umd"),
@@ -199,4 +213,4 @@ export default [
     rollupConfigFactory("build/es6/mod", "bundle/es6", true, "iife", "iife"),
     rollupConfigFactory("build/es6/mod", "bundle/es6", false, "umd", "umd"),
     rollupConfigFactory("build/es6/mod", "bundle/es6", true, "umd", "umd")
-];
+].map((config) => Object.assign(config, { onwarn: failOnCircularImport }));

core/scripts/copyRunnerBin.js

@@ -70,29 +70,56 @@ function copyRecursive(src, dest) {
     }
 }
 
-function ensureLocalLinks() {
-    const coreDir = path.resolve(__dirname, "..");
-    const nevware21Dir = path.join(coreDir, "node_modules", "@nevware21");
-    const linkType = process.platform === "win32" ? "junction" : "dir";
+// Removes the link an older build put at `linkPath` -- only the link, never the folder it points to.
+function _removeLink(linkPath) {
+    let stat = null;
+    try {
+        stat = fs.lstatSync(linkPath);
+    } catch (e) {
+        return;
+    }
 
-    if (!fs.existsSync(nevware21Dir)) {
-        fs.mkdirSync(nevware21Dir, { recursive: true });
+    if (stat.isSymbolicLink()) {
+        // A Windows junction is reported as a link too, and unlink removes just the junction
+        fs.unlinkSync(linkPath);
+        console.log("\x1b[36mRemoved the old self-link: core/node_modules/@nevware21/tripwire\x1b[0m");
     }
+}
 
-    // Self-link so the copied runner binary can dynamically resolve "@nevware21/tripwire"
-    // (the package under test) when run against core's own dogfood test suite from within
-    // core/ -- the default UX/compile providers are bundled directly into the binary now
-    // (internal/runner/src/ux, internal/runner/src/compile), so no other local links are needed here.
-    const localLinks = {
-        "tripwire": coreDir
+function ensureSelfPackage() {
+    const coreDir = path.resolve(__dirname, "..");
+    const packageDir = path.join(coreDir, "node_modules", "@nevware21", "tripwire");
+
+    // Lets the copied runner binary, and core's own test files, resolve "@nevware21/tripwire" (the
+    // package under test) when core's dogfood test suite runs from within core/ -- the default
+    // UX/compile providers are bundled directly into the binary now (internal/runner/src/ux,
+    // internal/runner/src/compile), so nothing else needs resolving here.
+    //
+    // It is a folder holding just a package.json whose entry points lead back to core's build, and
+    // not a link to core itself: core contains this very folder, so such a link was a loop. Anything
+    // walking ignored folders through it -- `git clean -xdf` (the clean build), `git status --ignored`
+    // -- went round core/node_modules/@nevware21/tripwire/node_modules/... until the paths grew too
+    // long for the file system. Node resolves `main` to the real file under core/dist, so the module
+    // loaded is the same one either way.
+    _removeLink(packageDir);
+    fs.mkdirSync(packageDir, { recursive: true });
+
+    const corePackage = JSON.parse(fs.readFileSync(path.join(coreDir, "package.json"), "utf8"));
+    const redirect = {
+        name: corePackage.name,
+        version: corePackage.version,
+        private: true,
+        description: "Points back to this checkout's core build. Written by core/scripts/copyRunnerBin.js.",
+        main: "../../../" + corePackage.main,
+        module: "../../../" + corePackage.module,
+        types: "../../../" + corePackage.types
     };
+    const text = JSON.stringify(redirect, null, 4) + "\n";
+    const target = path.join(packageDir, "package.json");
 
-    for (const [name, target] of Object.entries(localLinks)) {
-        const linkPath = path.join(nevware21Dir, name);
-        if (!fs.existsSync(linkPath)) {
-            fs.symlinkSync(target, linkPath, linkType);
-            console.log("\x1b[36mCreated local link: core/node_modules/@nevware21/" + name + "\x1b[0m");
-        }
+    if (!fs.existsSync(target) || fs.readFileSync(target, "utf8") !== text) {
+        fs.writeFileSync(target, text, "utf8");
+        console.log("\x1b[36mWrote local package: core/node_modules/@nevware21/tripwire\x1b[0m");
     }
 }
 
@@ -105,7 +132,7 @@ function main() {
 
     console.log("\x1b[36mCopying runner bin artifacts...\x1b[0m");
     copyRecursive(srcDir, dstDir);
-    ensureLocalLinks();
+    ensureSelfPackage();
 
     console.log("\x1b[32mDone copying runner bin artifacts to core/bin\x1b[0m");
 }

core/test/src/browser/statusEndpoint.test.ts

@@ -7,14 +7,20 @@
  */
 
 import { assert } from "../../../src/assert/assertClass";
-import { assertStatusResponse } from "../support/statusEndpoint";
+import { ITestContext } from "../../../src/interfaces/suite/ITestContext";
+import { assertStatusResponse, isServedByTripwire } from "../support/statusEndpoint";
 
 describe("/api/status server endpoint", () => {
-    // it("returns running status from the browser server", async () => {
-    //     const response = await fetch("/api/status");
-    //     const body = await response.json();
+    it("returns running status from the browser server", async function (this: ITestContext) {
+        if (!isServedByTripwire()) {
+            // Another runner's page, with nothing serving /api/status (see isServedByTripwire)
+            this.skip();
+        }
 
-    //     assert.equal(typeof response.status, "number", "Validate that fetch returned a numeric status code");
-    //     assertStatusResponse(response.status, response.headers.get("content-type"), body);
-    // });
+        const response = await fetch("/api/status");
+        const body = await response.json();
+
+        assert.equal(typeof response.status, "number", "Validate that fetch returned a numeric status code");
+        assertStatusResponse(response.status, response.headers.get("content-type"), body);
+    });
 });

core/test/src/support/statusEndpoint.ts

@@ -8,6 +8,19 @@
 
 import { assert } from "../../../src/assert/assertClass";
 
+/**
+ * Whether this page or worker was loaded by tripwire's own browser runner, whose server answers
+ * `/api/status` on the same origin. The browser and worker suites also run under other runners -- the
+ * benchmark's karma arms load these same folders -- where nothing answers that path, so the tests that
+ * fetch it skip there rather than fail.
+ *
+ * The marker is the module registry tripwire's runner installs before any test module loads:
+ * `__tripwireDefine__`, on the test frame's window (frame.html) or on the worker's global scope.
+ */
+export function isServedByTripwire(): boolean {
+    return typeof (globalThis as any).__tripwireDefine__ === "function";
+}
+
 export function assertStatusResponse(statusCode: number, contentType: string | null, body: any): void {
     assert.equal(statusCode, 200, "Validate that /api/status returns HTTP 200");
     assert.equal(contentType, "application/json; charset=utf-8", "Validate that /api/status returns JSON content");

core/test/src/worker/statusEndpoint.test.ts

@@ -7,14 +7,20 @@
  */
 
 import { assert } from "../../../src/assert/assertClass";
-import { assertStatusResponse } from "../support/statusEndpoint";
+import { ITestContext } from "../../../src/interfaces/suite/ITestContext";
+import { assertStatusResponse, isServedByTripwire } from "../support/statusEndpoint";
 
 describe("/api/status server endpoint", () => {
-    // it("returns running status from the worker server", async () => {
-    //     const response = await fetch("/api/status");
-    //     const body = await response.json();
+    it("returns running status from the worker server", async function (this: ITestContext) {
+        if (!isServedByTripwire()) {
+            // Another runner's worker, with nothing serving /api/status (see isServedByTripwire)
+            this.skip();
+        }
 
-    //     assert.equal(typeof response.status, "number", "Validate that fetch returned a numeric status code");
-    //     assertStatusResponse(response.status, response.headers.get("content-type"), body);
-    // });
+        const response = await fetch("/api/status");
+        const body = await response.json();
+
+        assert.equal(typeof response.status, "number", "Validate that fetch returned a numeric status code");
+        assertStatusResponse(response.status, response.headers.get("content-type"), body);
+    });
 });

internal/runner/rollup.config.js

@@ -401,4 +401,18 @@ const mainBundle = {
     ]
 };
 
-module.exports = [clientBundle, workerBundle, mainBundle];
+// A circular import between this repository's own modules fails the build: which module's exports
+// exist when another reads them then depends on which file happens to load first. A cycle entirely
+// inside a third-party package is left as a warning, since it is not ours to fix.
+function failOnCircularImport(warning, warn) {
+    if (warning.code === "CIRCULAR_DEPENDENCY") {
+        const ids = warning.ids || [];
+        if (!ids.some((id) => /[\\/]node_modules[\\/]/.test(id))) {
+            throw new Error("Circular import: " + (ids.length ? ids.join(" -> ") : warning.message));
+        }
+    }
+
+    warn(warning);
+}
+
+module.exports = [clientBundle, workerBundle, mainBundle].map((config) => Object.assign(config, { onwarn: failOnCircularImport }));

internal/runner/src/codecov/nycReport.ts

@@ -7,6 +7,8 @@
  */
 
 import { spawnSync } from "child_process";
+import * as fs from "fs";
+import * as path from "path";
 
 /**
  * Builds one `--reporter <name>` pair per configured reporter (nyc's own convention for
@@ -23,6 +25,33 @@ export function _buildReporterArgs(reporters?: string[]): string[] {
     return args;
 }
 
+/**
+ * The project folder `nyc report` should work in: the nearest folder at or above `cwd` holding a
+ * package.json, or `cwd` itself when there is none -- the same folder nyc picks on its own.
+ *
+ * It is passed to nyc explicitly as `--cwd` because nyc otherwise starts that search from `NYC_CWD`
+ * when it is set, and an nyc run *above* this process sets it for every child (re-adding it to each
+ * spawn even if it is removed from the environment). The report would then resolve its paths and load
+ * its `.nycrc` include and exclude patterns from that outer project, and report none of this one's files.
+ */
+export function _nycCwd(cwd: string): string {
+    let dir = path.resolve(cwd);
+
+    for (;;) {
+        // eslint-disable-next-line security/detect-non-literal-fs-filename
+        if (fs.existsSync(path.join(dir, "package.json"))) {
+            return dir;
+        }
+
+        const parent = path.dirname(dir);
+        if (parent === dir) {
+            return path.resolve(cwd);
+        }
+
+        dir = parent;
+    }
+}
+
 /**
  * Shared `nyc report` invocation used by both the browser/worker coverage writer
  * ({@link NycCoverageReporter}) and the node-target {@link runCoverageReport} command --
@@ -42,7 +71,7 @@ export function _spawnNycReport(cwd: string, reportArgs: string[], label: string
 
         const result = spawnSync(
             process.execPath,
-            [nycPath, "report", ...reportArgs],
+            [nycPath, "report", "--cwd", _nycCwd(cwd), ...reportArgs],
             { cwd, stdio: "inherit" }
         );
 

internal/runner/src/compile/tsCompile.ts

@@ -157,7 +157,7 @@ export function getProjectFileNames(projectPath: string, cwd: string): string[]
     }
 
     const ts = loadTypescript(cwd);
-    const loadedConfig = ts.readConfigFile(fullProjectPath, ts.sys.readFile);
+    const loadedConfig = _readTsConfig(ts, fullProjectPath);
     if (loadedConfig.error) {
         return null;
     }
@@ -200,6 +200,15 @@ function _normCasePath(filePath: string, caseSensitive: boolean): string {
     return caseSensitive ? normalized : normalized.toLowerCase();
 }
 
+/**
+ * Reads a tsconfig, passing its path with forward slashes the way TypeScript keeps its own file
+ * names. Given a Windows path with backslashes, a syntax error in the config fails TypeScript's
+ * internal path assertion ("Debug Failure") rather than coming back as the config's `error`.
+ */
+function _readTsConfig(ts: any, configPath: string): any {
+    return ts.readConfigFile(_normCasePath(configPath, true), ts.sys.readFile);
+}
+
 /**
  * Per-process cache of TypeScript programs keyed by "<configPath>:<includeSourceMap>".
  * Creating a TypeScript program is expensive (it parses all project files). By caching the
@@ -320,7 +329,7 @@ function _getOrCreateTsProgramEntry(ts: any, configPath: string, includeSourceMa
         return _tsProgCache[cacheKey];
     }
 
-    const loadedConfig = ts.readConfigFile(configPath, ts.sys.readFile);
+    const loadedConfig = _readTsConfig(ts, configPath);
     if (loadedConfig.error) {
         return null;
     }
@@ -773,7 +782,7 @@ export function registerTypeScriptHook(projectPath: string, cwd: string, checkDi
 
     // eslint-disable-next-line security/detect-non-literal-fs-filename
     if (fs.existsSync(fullProjectPath)) {
-        const loadedConfig = ts.readConfigFile(fullProjectPath, ts.sys.readFile);
+        const loadedConfig = _readTsConfig(ts, fullProjectPath);
         if (!loadedConfig.error) {
             const parsed = ts.parseJsonConfigFileContent(loadedConfig.config, ts.sys, path.dirname(fullProjectPath));
             if (!parsed.errors || !parsed.errors.length) {