Merge pull request #39 from happyprime/task/tests-and-validation

Add config validation and a test suite

Author: jeremyfelt

.github/workflows/test.yml

@@ -0,0 +1,29 @@
+name: Lint and test
+
+on:
+  push:
+    branches:
+      - trunk
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        node-version:
+          - 22
+          - 24
+    env:
+      # The unit tests run against in-memory PNGs and config files, so the
+      # Chromium download from the postinstall script is not needed here.
+      PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+          cache: npm
+      - run: npm ci
+      - run: npm run lint
+      - run: npm test

.gitignore

@@ -3,6 +3,9 @@ node_modules
 # Reglance output directory.
 .reglance
 
+# Multi-persona code review artifacts.
+.reviews
+
 # Legacy output directories (pre-2.0), kept ignored for leftover local data.
 captures
 compares

README.md

@@ -52,11 +52,17 @@ self-ignored `.reglance/` directory — nothing to add to `.gitignore`.
 | `viewports`         | no       | `[{ name, width, height }]`. Defaults to `desktop` (1920×1080), `mobile` (390×844). |
 | `output`            | no       | Output directory. Defaults to `.reglance`.                                         |
 | `pixelmatchOptions` | no       | [pixelmatch](https://github.com/mapbox/pixelmatch) options, e.g. `{ "threshold": 0.1 }`. |
+| `timeouts`          | no       | `{ goto, settle }` in ms. Navigation and post-scroll network-idle waits. Defaults `{ goto: 15000, settle: 8000 }`. Raise `settle` for slow, lazy-loading pages. |
 
 `domain` is only needed by `capture`; `control` and `compare` work on the files
 already captured. See [`reglance.example.json`](reglance.example.json) for a full
 example.
 
+A `paths` value may be a full URL pointing at a different host than `domain`.
+This is supported, but `capture` prints a warning listing such paths so
+off-domain navigation is a conscious choice — keep configs from trusted
+sources, since the report renders captured content.
+
 ## Commands
 
 | Command             | Description                                                          |
@@ -70,14 +76,17 @@ Append path keys to limit a command to specific pages:
 
 ## Options
 
-| Flag                | Command | Description                                            |
-| ------------------- | ------- | ------------------------------------------------------ |
-| `--domain=<host>`   | capture | Override the configured domain for this run.           |
-| `--concurrency=<n>` | capture | Parallel browser contexts (default: 4).                |
-| `--stagger=<ms>`    | capture | Delay between starting contexts (default: 500).        |
-| `--skip-reload`     | capture | Reuse the page between viewports instead of reloading. |
-| `--no-open`         | compare | Don't open the report when finished.                   |
-| `--config=<path>`   | any     | Path to the config file (default: `reglance.json`).    |
+| Flag                       | Command | Description                                                                       |
+| -------------------------- | ------- | --------------------------------------------------------------------------------- |
+| `--domain=<host>`          | capture | Override the configured domain for this run.                                      |
+| `--concurrency=<n>`        | capture | Parallel browser contexts (default: 4). Must be a positive integer.               |
+| `--stagger=<ms>`           | capture | Delay between starting contexts (default: 500). `0` disables staggering.          |
+| `--skip-reload`            | capture | Reuse the page between viewports instead of reloading.                            |
+| `--fail-on-degraded`       | capture | Exit non-zero if any page failed to load cleanly (for CI). Default: warn, exit 0. |
+| `--insecure`               | capture | Ignore TLS certificate errors for non-local hosts (already ignored for `.test`/localhost). |
+| `--compare-concurrency=<n>`| compare | Parallel diff workers (default: CPU count − 1). Lower it for very tall pages.      |
+| `--no-open`                | compare | Don't open the report when finished.                                              |
+| `--config=<path>`          | any     | Path to the config file (default: `reglance.json`).                               |
 
 ### Per-developer domains
 
@@ -88,12 +97,25 @@ their own local site without editing the config:
 npx reglance capture --domain=site2.test
 ```
 
+### Trustworthy baselines
+
+A baseline is only useful if it reflects pages that actually loaded. reglance
+guards against silently baselining bad data:
+
+- If a page never loads cleanly (after retries), `capture` reports it as
+  degraded instead of treating it as a success. Add `--fail-on-degraded` to
+  make the run exit non-zero in CI.
+- `control` records each promotion in `.reglance/controls/manifest.json` and
+  warns when it promoted fewer captures than expected (so the untouched
+  controls are now stale). `compare` warns when the baseline mixes controls
+  from more than one `control` run.
+
 ## Output
 
 ```
 .reglance/
 	captures/   Latest screenshots + HTML snapshots
-	controls/   Baseline screenshots + HTML
+	controls/   Baseline screenshots + HTML (+ manifest.json)
 	compares/   Diff images and HTML diffs
 	reports/    The report — open reports/index.html
 ```

bin/reglance.mjs

@@ -1,5 +1,6 @@
 #!/usr/bin/env node
 import { parseArgs } from 'node:util';
+import { toInt } from '../src/args.mjs';
 import { loadConfig, ensureOutputDir } from '../src/config.mjs';
 import { capture } from '../src/capture.mjs';
 import { control } from '../src/control.mjs';
@@ -26,6 +27,11 @@ Options:
   --concurrency=<n>   Parallel browser contexts for capture (default: 4).
   --stagger=<ms>      Delay between starting capture contexts (default: 500).
   --skip-reload       Reuse the page between viewports during capture.
+  --insecure          Ignore TLS certificate errors for non-local hosts
+                      (already ignored for .test/localhost).
+  --fail-on-degraded  Exit non-zero if any capture failed to load cleanly.
+  --compare-concurrency=<n>  Parallel diff workers for compare
+                      (default: CPU count - 1; lower it for very tall pages).
   --no-open           Don't open the report automatically after compare.
   -h, --help          Show this help.
 
@@ -43,6 +49,9 @@ const { values, positionals } = parseArgs({
 		concurrency: { type: 'string' },
 		stagger: { type: 'string' },
 		'skip-reload': { type: 'boolean' },
+		insecure: { type: 'boolean' },
+		'fail-on-degraded': { type: 'boolean' },
+		'compare-concurrency': { type: 'string' },
 		'no-open': { type: 'boolean' },
 		help: { type: 'boolean', short: 'h' },
 	},
@@ -56,23 +65,7 @@ if (values.help || !command) {
 }
 
 /**
- * Parse an integer flag, exiting with an error when it is not a number.
- *
- * @param {string} value - The raw flag value.
- * @param {string} name  - The flag name, for error messages.
- * @returns {number} The parsed integer.
- */
-function toInt(value, name) {
-	const parsed = parseInt(value, 10);
-	if (Number.isNaN(parsed)) {
-		console.error(`Invalid value for --${name}: ${value}`);
-		process.exit(1);
-	}
-	return parsed;
-}
-
-/**
- *
+ * Dispatch the parsed command.
  */
 async function main() {
 	const config = loadConfig({
@@ -96,9 +89,11 @@ async function main() {
 					? toInt(values.concurrency, 'concurrency')
 					: undefined,
 				staggerDelay: values.stagger
-					? toInt(values.stagger, 'stagger')
+					? toInt(values.stagger, 'stagger', { min: 0 })
 					: undefined,
 				skipReload: values['skip-reload'],
+				failOnDegraded: values['fail-on-degraded'],
+				insecure: values.insecure,
 			});
 			break;
 
@@ -107,7 +102,16 @@ async function main() {
 			break;
 
 		case 'compare':
-			await compare(config, { only, open: !values['no-open'] });
+			await compare(config, {
+				only,
+				open: !values['no-open'],
+				concurrency: values['compare-concurrency']
+					? toInt(
+							values['compare-concurrency'],
+							'compare-concurrency'
+						)
+					: undefined,
+			});
 			break;
 
 		default:

package-lock.json

@@ -22,6 +22,7 @@
 	"files": [
 		"bin",
 		"src",
+		"scripts",
 		"templates",
 		"reglance.example.json"
 	],
@@ -31,13 +32,14 @@
 	"scripts": {
 		"lint": "eslint .",
 		"fix": "eslint . --fix",
-		"postinstall": "playwright install chromium"
+		"test": "node --test",
+		"postinstall": "node scripts/postinstall.mjs"
 	},
 	"dependencies": {
 		"diff": "^8.0.3",
 		"open": "^11.0.0",
 		"pixelmatch": "^7.2.0",
-		"playwright": "^1.60.0",
+		"playwright": "1.60.0",
 		"pngjs": "^7.0.0"
 	},
 	"devDependencies": {

package.json

@@ -15,5 +15,9 @@
 		"includeAA": false,
 		"alpha": 0.1,
 		"diffColor": [255, 0, 0]
+	},
+	"timeouts": {
+		"goto": 15000,
+		"settle": 8000
 	}
 }

reglance.example.json

@@ -0,0 +1,32 @@
+#!/usr/bin/env node
+import { spawnSync } from 'node:child_process';
+
+// reglance is installed as a dev dependency, so this hook runs inside every
+// consuming project's install. It must never fail that install: a blocked CDN,
+// an air-gapped runner, or a proxy should degrade gracefully, not abort
+// `npm ci` for the whole project.
+
+// Honor the standard Playwright skip flag (CI that doesn't capture, or that
+// provisions Chromium separately). The explicit `playwright install` command
+// ignores this variable, so we check it ourselves.
+if (process.env.PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD) {
+	process.exit(0);
+}
+
+const result = spawnSync('playwright', ['install', 'chromium'], {
+	stdio: 'inherit',
+	// shell:true so the `playwright` bin resolves on Windows (.cmd) too.
+	shell: true,
+});
+
+if (result.status !== 0) {
+	console.error(
+		'\n⚠️  reglance: Chromium download failed. reglance is installed, but ' +
+			'`reglance capture` needs a browser — run `npx playwright install ' +
+			'chromium` before capturing.'
+	);
+}
+
+// Always succeed: a missing browser is recoverable and is reported again at
+// capture time; it must not break the consumer's install.
+process.exit(0);

scripts/postinstall.mjs

@@ -0,0 +1,24 @@
+/**
+ * Parse an integer CLI flag, throwing an actionable error when it is invalid.
+ *
+ * Uses `Number` rather than `parseInt` so trailing garbage ("8x") is rejected
+ * instead of silently truncated, and enforces a minimum so a value like
+ * `--concurrency=0` cannot reach the capture loop and hang it.
+ *
+ * @param {string} value     - The raw flag value.
+ * @param {string} name      - The flag name, for error messages.
+ * @param {object} [options] - Parse options.
+ * @param {number} [options.min] - The smallest allowed value (default 1).
+ * @returns {number} The parsed integer.
+ */
+export function toInt(value, name, { min = 1 } = {}) {
+	const parsed = Number(value);
+
+	if (!Number.isInteger(parsed) || parsed < min) {
+		throw new Error(
+			`Invalid value for --${name}: ${value} (expected an integer >= ${min}).`
+		);
+	}
+
+	return parsed;
+}