test: separate inspector timing from snapshots

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: philippfromme

tasks/inspect.mjs

@@ -9,7 +9,11 @@ if (!npmExecPath) {
   throw new Error('npm_execpath is required to run the inspector suite.');
 }
 
-const testExitCode = await run(process.execPath, [ npmExecPath, 'test' ]);
+const testExitCode = await run(
+  process.execPath,
+  [ npmExecPath, 'test' ],
+  { ...process.env, INSPECTOR_TIMINGS: 'true' }
+);
 
 if (!existsSync(inspectorPath)) {
   console.error(`Inspector report was not created: ${inspectorPath}`);
@@ -20,9 +24,12 @@ if (!existsSync(inspectorPath)) {
   process.exitCode = testExitCode || openExitCode;
 }
 
-function run(command, args) {
+function run(command, args, environment = process.env) {
   return new Promise(resolve => {
-    const child = spawn(command, args, { stdio: 'inherit' });
+    const child = spawn(command, args, {
+      env: environment,
+      stdio: 'inherit'
+    });
 
     child.on('exit', code => resolve(code ?? 1));
     child.on('error', () => resolve(1));

test/InspectorTimingSpec.js

@@ -0,0 +1,22 @@
+import assert from 'node:assert';
+
+import {
+  INSPECTOR_LAYOUT_TIMING_RUNS,
+  shouldMeasureLayoutTimings
+} from './inspector/Timing.js';
+
+describe('Inspector timing', function() {
+
+  it('should only measure timings when the inspector enables them', function() {
+    assert.strictEqual(shouldMeasureLayoutTimings({}), false);
+    assert.strictEqual(
+      shouldMeasureLayoutTimings({ INSPECTOR_TIMINGS: 'false' }),
+      false
+    );
+    assert.strictEqual(
+      shouldMeasureLayoutTimings({ INSPECTOR_TIMINGS: 'true' }),
+      true
+    );
+    assert.strictEqual(INSPECTOR_LAYOUT_TIMING_RUNS, 5);
+  });
+});

test/LayoutSpec.js

@@ -23,6 +23,10 @@ import {
 } from '../lib/layout/geometry/index.js';
 import { calculateStatistics } from '../tasks/benchmark-util.mjs';
 import { writeInspectorReport } from './inspector/Report.js';
+import {
+  INSPECTOR_LAYOUT_TIMING_RUNS,
+  shouldMeasureLayoutTimings
+} from './inspector/Timing.js';
 import {
   EXTERNAL_LABEL_CLEARANCE,
   EXPANDED_SUBPROCESS_ANNOTATION_CLEARANCE,
@@ -46,9 +50,9 @@ const snapshotsDirectory = path.join(__dirname, 'snapshots');
 const metricsBaselineFile = path.join(__dirname, 'metrics', 'baseline.json');
 
 const UPDATE_SNAPSHOTS = process.env.UPDATE_SNAPSHOTS === 'true';
-const INSPECTOR_LAYOUT_TIMING_RUNS = 5;
 const layoutTimingsByFixture = new Map();
 const layoutWarningsByFixture = new Map();
+const MEASURE_INSPECTOR_TIMINGS = shouldMeasureLayoutTimings();
 
 async function layoutProcess(xml) {
   return (await layoutProcessResult(xml)).xml;
@@ -3245,32 +3249,23 @@ describe('Layout', function() {
         const xml = fs.readFileSync(path.join(fixturesDirectory, fileName), 'utf8');
 
         // when
-        await layoutProcessResult(xml);
-
-        const timings = [];
-        let output;
-        let warnings;
-
-        for (let index = 0; index < INSPECTOR_LAYOUT_TIMING_RUNS; index++) {
-          const startedAt = performance.now();
-          const result = await layoutProcessResult(xml);
-
-          timings.push(performance.now() - startedAt);
-
-          if (index === 0) {
-            output = result.xml;
-            warnings = result.warnings;
-          }
-        }
+        const result = await layoutProcessResult(xml);
+        const output = result.xml;
 
-        layoutTimingsByFixture.set(fileName, timings);
-        layoutWarningsByFixture.set(fileName, warnings.map(warning => ({
+        layoutWarningsByFixture.set(fileName, result.warnings.map(warning => ({
           code: warning.code,
           elementId: warning.elementId,
           message: warning.message,
           relatedElementIds: warning.relatedElementIds
         })));
 
+        if (MEASURE_INSPECTOR_TIMINGS) {
+          layoutTimingsByFixture.set(
+            fileName,
+            await measureLayoutTimings(xml)
+          );
+        }
+
         fs.writeFileSync(path.join(outputDirectory, fileName), output, 'utf8');
 
         if (UPDATE_SNAPSHOTS) {
@@ -3335,21 +3330,23 @@ describe('Layout', function() {
     assert.ok(index.includes('createMetricsPanel'));
     assert.ok(index.includes('createLayoutTiming'));
     assert.ok(index.includes('createWarningsPanel'));
-    assert.ok(results.every(result => {
-      return Number.isFinite(result.layoutTiming?.averageMs) &&
-        result.layoutTiming.averageMs >= 0 &&
-        Number.isFinite(result.layoutTiming.p50Ms) &&
-        result.layoutTiming.p50Ms >= 0 &&
-        Number.isFinite(result.layoutTiming.p90Ms) &&
-        result.layoutTiming.p90Ms >= 0 &&
-        result.layoutTiming.runs === INSPECTOR_LAYOUT_TIMING_RUNS;
-    }));
-    assert.deepStrictEqual(
-      results
-        .map(result => result.layoutTiming.rank)
-        .sort((first, second) => first - second),
-      results.map((result, index) => index + 1)
-    );
+    if (MEASURE_INSPECTOR_TIMINGS) {
+      assert.ok(results.every(result => {
+        return Number.isFinite(result.layoutTiming?.averageMs) &&
+          result.layoutTiming.averageMs >= 0 &&
+          Number.isFinite(result.layoutTiming.p50Ms) &&
+          result.layoutTiming.p50Ms >= 0 &&
+          Number.isFinite(result.layoutTiming.p90Ms) &&
+          result.layoutTiming.p90Ms >= 0 &&
+          result.layoutTiming.runs === INSPECTOR_LAYOUT_TIMING_RUNS;
+      }));
+      assert.deepStrictEqual(
+        results
+          .map(result => result.layoutTiming.rank)
+          .sort((first, second) => first - second),
+        results.map((result, index) => index + 1)
+      );
+    }
     const groupWarningFixture = results.find(result => {
       return result.name === 'artifact.group-without-members.bpmn';
     });
@@ -3428,6 +3425,21 @@ function iit(fileName) {
   return it;
 }
 
+async function measureLayoutTimings(xml) {
+  await layoutProcessResult(xml);
+
+  const timings = [];
+
+  for (let index = 0; index < INSPECTOR_LAYOUT_TIMING_RUNS; index++) {
+    const startedAt = performance.now();
+
+    await layoutProcessResult(xml);
+    timings.push(performance.now() - startedAt);
+  }
+
+  return timings;
+}
+
 function summarizeLayoutTimings(timingsByFixture) {
   const timings = [ ...timingsByFixture.entries() ].map(([ fileName, durations ]) => {
     return [ fileName, calculateStatistics(durations) ];

test/README.md

@@ -14,9 +14,11 @@ improves or regresses layout quality. The visual inspector supports review of
 the snapshot suite. Each fixture header also shows its generated layout metrics
 and the delta from the recorded metrics baseline; green and orange metric cards
 indicate an improvement or regression for metrics with a preferred direction.
-It also shows average, p50, and p90 `layoutProcess` durations from five measured
-runs after one warm-up, and ranks fixtures by p50. Timing is informational only:
-it has no persisted baseline and never affects the test result.
+When generated with `npm run test:inspect`, it also shows average, p50, and p90
+`layoutProcess` durations from five measured runs after one warm-up, and ranks
+fixtures by p50. `npm test` runs each fixture once and does not collect timing
+measurements. Timing is informational only: it has no persisted baseline and
+never affects the test result.
 Non-fatal layout warnings are shown below the metrics for the fixture that
 produced them.
 

test/inspector/Timing.js

@@ -0,0 +1,5 @@
+export const INSPECTOR_LAYOUT_TIMING_RUNS = 5;
+
+export function shouldMeasureLayoutTimings(environment = process.env) {
+  return environment.INSPECTOR_TIMINGS === 'true';
+}