test(e2e): give each checkout its own ports and scratch directory

.github/workflows/e2e.yml

@@ -48,5 +48,5 @@ jobs:
         continue-on-error: true
         with:
           name: image_comparison_results_${{ matrix.os }}
-          path: .tmp
+          path: .tmp/runs
           retention-days: 5

CONTRIBUTING.md

@@ -85,7 +85,11 @@ npm run test
 npm run test:e2e:chrome
 ```
 
-When running end-to-end tests, baseline images are saved to `tests/baseline`. Baseline diffs and actual snapshots are saved to `.tmp`.
+When running end-to-end tests, baseline images are saved to `tests/baseline`. Baseline diffs and actual snapshots are saved to `.tmp/runs/<port>`, where `<port>` is the static server port this checkout uses. Locally, a passing run deletes that directory on the way out, so the snapshots stay around only after a failure. CI always keeps it, so the artifacts below are there whether the run passed or not.
+
+Downloaded test datasets are cached in `.tmp/datasets` and shared by every run, so deleting a run directory does not force a re-download.
+
+Two checkouts get different ports and different run directories, so their suites can run at the same time. Two suites out of one checkout cannot: they would share `dist/` and `tests/baseline`. Set `VOLVIEW_E2E_PORT` and `VOLVIEW_E2E_AUX_PORT` if something else already holds the ports a checkout picked.
 
 When adding a new baseline image and test, the image should be pulled from GitHub Actions. Every test run will upload artifacts containing the snapshots taken, and those should be used when verifying and committing the baseline images.
 

tests/e2ePorts.ts

@@ -0,0 +1,40 @@
+import { createHash } from 'crypto';
+import { projectRoot } from './e2eTestUtils';
+
+// Clear of the crowded low ports, and below the range operating systems hand
+// out for outbound connections (32768+ on Linux, 49152+ on Windows), so nothing
+// else on the machine is handed one while it sits unbound between runs.
+const RANGE_START = 20000;
+const RANGE_PAIRS = 5000;
+
+/**
+ * The same checkout path always derives the same ports and two checkouts derive
+ * different ones, so every process in a run agrees without coordinating.
+ */
+function derivePort() {
+  // Windows compares paths case-insensitively, so the same checkout reached
+  // through a differently cased path has to hash the same.
+  const root =
+    process.platform === 'win32' ? projectRoot().toLowerCase() : projectRoot();
+  const hashed = createHash('sha256').update(root).digest().readUInt32BE(0);
+  return RANGE_START + (hashed % RANGE_PAIRS) * 2;
+}
+
+function overridable(name: string, derived: number) {
+  const raw = process.env[name];
+  if (!raw) return derived;
+  const port = Number(raw);
+  if (!Number.isInteger(port) || port < 1 || port > 65535) {
+    throw new Error(`${name} must be a port number, got "${raw}"`);
+  }
+  return port;
+}
+
+const BASE = derivePort();
+
+export const TEST_PORT = overridable('VOLVIEW_E2E_PORT', BASE);
+// For a spec that needs a server of its own alongside the static one.
+export const AUX_PORT = overridable('VOLVIEW_E2E_AUX_PORT', BASE + 1);
+
+// The static server, which is also what the vite dev server proxies /tmp to.
+export const BASE_URL = `http://localhost:${TEST_PORT}`;

tests/server/content-disposition-server.ts

@@ -2,9 +2,8 @@ import express from 'express';
 import cors from 'cors';
 import { readFileSync } from 'fs';
 import { join } from 'path';
-
-const PORT = 4568;
-const TEMP_DIR = '.tmp';
+import { TEMP_DIR } from '../../wdio.shared.conf';
+import { AUX_PORT } from '../e2ePorts';
 
 export function createContentDispositionServer() {
   const app = express();
@@ -33,8 +32,8 @@ export function createContentDispositionServer() {
 
 export function startServer() {
   const app = createContentDispositionServer();
-  return app.listen(PORT, () => {
-    console.log(`Content-Disposition test server running on port ${PORT}`);
+  return app.listen(AUX_PORT, () => {
+    console.log(`Content-Disposition test server running on port ${AUX_PORT}`);
   });
 }
 

tests/specs/content-disposition.e2e.ts

@@ -1,14 +1,13 @@
 import { volViewPage } from '../pageobjects/volview.page';
 import { downloadFile } from './utils';
 import { startServer, stopServer } from '../server/content-disposition-server';
+import { AUX_PORT as SERVER_PORT } from '../e2ePorts';
 
 const CT_ELECTRODES = {
   url: 'https://raw.githubusercontent.com/neurolabusc/niivue-images/main/CT_Electrodes.nii.gz',
   name: 'CT_Electrodes.nii.gz',
 };
 
-const SERVER_PORT = 4568;
-
 describe('Content-Disposition header handling', () => {
   let server: ReturnType<typeof startServer>;
 

tests/specs/utils.ts

@@ -3,13 +3,17 @@ import * as fs from 'fs';
 import { z } from 'zod';
 import { cleanuptotal } from 'wdio-cleanuptotal-service';
 import JSZip from 'jszip';
-import { TEMP_DIR } from '../../wdio.shared.conf';
+import {
+  DATASET_CACHE,
+  TEMP_DIR,
+  linkCachedDataset,
+} from '../../wdio.shared.conf';
 import { volViewPage } from '../pageobjects/volview.page';
 import { RemoteResource } from '../../src/io/manifest';
 
-// File is not automatically deleted
+// Cached across runs, and not automatically deleted
 export const downloadFile = async (url: string, fileName: string) => {
-  const savePath = path.join(TEMP_DIR, fileName);
+  const savePath = path.join(DATASET_CACHE, fileName);
   if (!fs.existsSync(savePath)) {
     // Download to a temporary file first to avoid race conditions
     const tempPath = `${savePath}.${process.pid}.${Date.now()}.tmp`;
@@ -34,7 +38,7 @@ export const downloadFile = async (url: string, fileName: string) => {
       }
     }
   }
-  return savePath;
+  return linkCachedDataset(fileName);
 };
 
 export async function writeManifestToFile(manifest: unknown, fileName: string) {

vite.config.ts

@@ -12,7 +12,7 @@ import { visualizer } from 'rollup-plugin-visualizer';
 import { sentryVitePlugin } from '@sentry/vite-plugin';
 import replace from '@rollup/plugin-replace';
 
-import { config } from './wdio.shared.conf';
+import { BASE_URL } from './tests/e2ePorts';
 
 function resolveNodeModulePath(moduleName: string) {
   const require = createRequire(import.meta.url);
@@ -224,7 +224,7 @@ export default defineConfig({
   server: {
     // so `npm run test:e2e:dev` can access the webdriver static server temp directory
     proxy: {
-      '/tmp': config.baseUrl!,
+      '/tmp': BASE_URL,
       // Local Girder stack, so girder-launched sessions (urls=/api/v1/...)
       // work same-origin against the dev server.
       '/api': 'http://localhost:8080',

wdio.shared.conf.ts

@@ -1,7 +1,10 @@
 import * as path from 'path';
 import * as fs from 'fs';
+import { createServer } from 'net';
 import type { Options, Capabilities } from '@wdio/types';
+import { SevereServiceError } from 'webdriverio';
 import { projectRoot } from './tests/e2eTestUtils';
+import { AUX_PORT, BASE_URL, TEST_PORT } from './tests/e2ePorts';
 
 const TEST_DATASETS = [
   {
@@ -38,7 +41,6 @@ export const CONTENT_VIEWPORT = { width: 1280, height: 720 } as const;
 export const applyTestViewport = (browser: any) =>
   browser.setViewport({ ...CONTENT_VIEWPORT, devicePixelRatio: 1 });
 
-export const TEST_PORT = 4567;
 // for slow connections try:
 // DOWNLOAD_TIMEOUT=60000 && npm run test:e2e:dev
 export const DOWNLOAD_TIMEOUT = Number(process.env.DOWNLOAD_TIMEOUT ?? 30000);
@@ -47,13 +49,81 @@ const IS_CI = !!(process.env.CI || process.env.GITHUB_ACTIONS);
 
 const ROOT = projectRoot();
 const TMP = '.tmp/';
-// TEMP_DIR is also browser downloads directory
-export const TEMP_DIR = path.resolve(ROOT, TMP);
+// Fixtures are downloaded once and shared by every run.
+export const DATASET_CACHE = path.resolve(ROOT, TMP, 'datasets');
+// Everything a run generates or downloads through the browser, including the
+// fixture links it serves. Also the browser downloads directory. Keyed by port
+// so a checkout, or an overridden port, gets scratch space of its own.
+export const TEMP_DIR = path.resolve(ROOT, TMP, 'runs', String(TEST_PORT));
 const FIXTURES_DIR = 'tests/fixtures/';
 export const FIXTURES = path.resolve(ROOT, FIXTURES_DIR);
 
+// The static server and the browser's download directory both point into
+// TEMP_DIR, and Windows refuses to unlink a file while a handle is open, so give
+// their teardown a moment to catch up.
+const removeDir = (dir: string) =>
+  fs.rmSync(dir, {
+    recursive: true,
+    force: true,
+    maxRetries: 5,
+    retryDelay: 200,
+  });
+
+/**
+ * Exposes a cached fixture under this run's directory, so specs can reach it at
+ * `/tmp/<name>` without every run holding its own copy.
+ */
+export function linkCachedDataset(name: string) {
+  const runPath = path.join(TEMP_DIR, name);
+  if (fs.existsSync(runPath)) return runPath;
+
+  fs.mkdirSync(TEMP_DIR, { recursive: true });
+  try {
+    fs.linkSync(path.join(DATASET_CACHE, name), runPath);
+  } catch {
+    // A hard link needs one filesystem; a copy always works.
+    fs.copyFileSync(path.join(DATASET_CACHE, name), runPath);
+  }
+  return runPath;
+}
+
+const inUse = (port: number) =>
+  new Promise<boolean>((resolve) => {
+    const server = createServer()
+      .once('error', (err: NodeJS.ErrnoException) =>
+        resolve(err.code === 'EADDRINUSE')
+      )
+      .once('listening', () => server.close(() => resolve(false)));
+    server.listen(port);
+  });
+
+const NAMED_PORTS = [
+  ['VOLVIEW_E2E_PORT', TEST_PORT],
+  ['VOLVIEW_E2E_AUX_PORT', AUX_PORT],
+] as const;
+
+/**
+ * These ports are stable, not reserved, so a suite already running out of this
+ * checkout, a checkout that hashed the same way, or an unrelated process can be
+ * holding one.
+ */
+async function assertPortsAvailable() {
+  const checks = await Promise.all(
+    NAMED_PORTS.map(async ([name, port]) =>
+      (await inUse(port)) ? `${port} (${name})` : null
+    )
+  );
+  const taken = checks.filter(Boolean);
+  if (taken.length) {
+    // Anything less severe is logged and the run carries on without a server.
+    throw new SevereServiceError(
+      `E2E ports already in use: ${taken.join(', ')}. Set the named variables to free ports to run anyway.`
+    );
+  }
+}
+
 export const config: Options.Testrunner = {
-  baseUrl: `http://localhost:${TEST_PORT}`,
+  baseUrl: BASE_URL,
   // ====================
   // Runner Configuration
   // ====================
@@ -89,7 +159,7 @@ export const config: Options.Testrunner = {
             mount: '/',
             path: './dist',
           },
-          { mount: '/tmp', path: `./${TMP}` },
+          { mount: '/tmp', path: TEMP_DIR },
         ],
         port: TEST_PORT,
       },
@@ -118,6 +188,13 @@ export const config: Options.Testrunner = {
   //
 
   async onPrepare() {
+    // Bail before the wipe below, which would otherwise take out the scratch
+    // directory of whichever suite is already holding the port.
+    await assertPortsAvailable();
+
+    fs.mkdirSync(DATASET_CACHE, { recursive: true });
+    // Start empty, so whatever is in here afterwards came from this run.
+    removeDir(TEMP_DIR);
     fs.mkdirSync(TEMP_DIR, { recursive: true });
 
     const RETRIES = 3;
@@ -140,29 +217,40 @@ export const config: Options.Testrunner = {
     };
 
     const downloads = TEST_DATASETS.map(async ({ url, name }) => {
-      const savePath = path.join(TEMP_DIR, name);
-      if (fs.existsSync(savePath)) {
-        return;
-      }
-      for (let attempt = 1; attempt <= RETRIES; attempt += 1) {
-        try {
-          await downloadOnce(url, savePath);
-          return;
-        } catch (err) {
-          if (attempt === RETRIES) {
-            throw new Error(
-              `Failed to download ${name} after ${RETRIES} attempts: ${
-                (err as Error).message
-              }`
-            );
+      const savePath = path.join(DATASET_CACHE, name);
+      if (!fs.existsSync(savePath)) {
+        for (let attempt = 1; attempt <= RETRIES; attempt += 1) {
+          try {
+            await downloadOnce(url, savePath);
+            break;
+          } catch (err) {
+            if (attempt === RETRIES) {
+              throw new Error(
+                `Failed to download ${name} after ${RETRIES} attempts: ${
+                  (err as Error).message
+                }`
+              );
+            }
+            await delay(RETRY_DELAY_MS);
           }
-          await delay(RETRY_DELAY_MS);
         }
       }
+      linkCachedDataset(name);
     });
     await Promise.all(downloads);
   },
 
+  async onComplete(exitCode, completedConfig) {
+    // A failed run keeps its directory: the screenshots and downloads in it are
+    // what there is to look at. Watch mode reports 0 whatever the tests did,
+    // since quitting it is not a failure. CI keeps its directory either way,
+    // because a passing run is exactly where a new baseline image gets picked
+    // up from the uploaded artifact.
+    if (exitCode === 0 && !completedConfig.watch && !IS_CI) {
+      removeDir(TEMP_DIR);
+    }
+  },
+
   async before(
     _capabilities:
       | Capabilities.RequestedStandaloneCapabilities