refactor: improve regression testing (#1898)

Author: SethFalco

.github/workflows/main.yml

@@ -36,6 +36,7 @@ jobs:
           node-version: ${{ env.NODE }}
           cache: yarn
       - run: yarn install
+      - run: yarn playwright install --with-deps chromium
       - run: yarn test-regression
   test:
     name: ${{ matrix.os }} Node.js ${{ matrix.node-version }}
@@ -58,5 +59,6 @@ jobs:
           node-version: ${{ matrix.node-version }}
           cache: yarn
       - run: yarn install
+      - run: yarn playwright install --with-deps chromium
       - run: yarn test
       - run: yarn test-browser

package.json

@@ -96,9 +96,9 @@
     "eslint": "^8.55.0",
     "jest": "^29.5.5",
     "node-fetch": "^2.7.0",
-    "pixelmatch": "^5.2.1",
-    "playwright": "^1.14.1",
-    "pngjs": "^6.0.0",
+    "pixelmatch": "^5.3.0",
+    "playwright": "^1.40.1",
+    "pngjs": "^7.0.0",
     "prettier": "^3.1.1",
     "rollup": "^2.79.1",
     "rollup-plugin-terser": "^7.0.2",

test/browser.js

@@ -1,6 +1,6 @@
-const fs = require('fs');
-const http = require('http');
 const assert = require('assert');
+const fs = require('node:fs/promises');
+const http = require('http');
 const { chromium } = require('playwright');
 
 const fixture = `<svg xmlns="http://www.w3.org/2000/svg">
@@ -33,14 +33,14 @@ globalThis.result = result.data;
 </script>
 `;
 
-const server = http.createServer((req, res) => {
+const server = http.createServer(async (req, res) => {
   if (req.url === '/') {
     res.setHeader('Content-Type', 'text/html');
     res.end(content);
   }
   if (req.url === '/svgo.browser.js') {
     res.setHeader('Content-Type', 'application/javascript');
-    res.end(fs.readFileSync('./dist/svgo.browser.js'));
+    res.end(await fs.readFile('./dist/svgo.browser.js'));
   }
   res.end();
 });

test/regression-extract.js

@@ -2,14 +2,33 @@
 
 const fs = require('fs');
 const path = require('path');
+const stream = require('stream');
 const util = require('util');
 const zlib = require('zlib');
-const stream = require('stream');
 const { default: fetch } = require('node-fetch');
 const tarStream = require('tar-stream');
 
 const pipeline = util.promisify(stream.pipeline);
 
+const exclude = [
+  // animated
+  'svg/filters-light-04-f.svg',
+  'svg/filters-composite-05-f.svg',
+  // messed gradients
+  'svg/pservers-grad-18-b.svg',
+  // removing wrapping <g> breaks :first-child pseudo-class
+  'svg/styling-pres-04-f.svg',
+  // rect is converted to path which matches wrong styles
+  'svg/styling-css-08-f.svg',
+  // complex selectors are messed because of converting shapes to paths
+  'svg/struct-use-10-f.svg',
+  'svg/struct-use-11-f.svg',
+  'svg/styling-css-01-b.svg',
+  'svg/styling-css-04-f.svg',
+  // strange artifact breaks inconsistently  breaks regression tests
+  'svg/filters-conv-05-f.svg',
+];
+
 /**
  * @param {string} url
  * @param {string} baseDir
@@ -18,15 +37,21 @@ const pipeline = util.promisify(stream.pipeline);
 const extractTarGz = async (url, baseDir, include) => {
   const extract = tarStream.extract();
   extract.on('entry', async (header, stream, next) => {
+    const name = header.name;
+
     try {
-      if (include == null || include.test(header.name)) {
-        if (header.name.endsWith('.svg')) {
-          const file = path.join(baseDir, header.name);
+      if (include == null || include.test(name)) {
+        if (
+          name.endsWith('.svg') &&
+          !exclude.includes(name) &&
+          !name.startsWith('svg/animate-')
+        ) {
+          const file = path.join(baseDir, name);
           await fs.promises.mkdir(path.dirname(file), { recursive: true });
           await pipeline(stream, fs.createWriteStream(file));
-        } else if (header.name.endsWith('.svgz')) {
+        } else if (name.endsWith('.svgz')) {
           // .svgz -> .svg
-          const file = path.join(baseDir, header.name.slice(0, -1));
+          const file = path.join(baseDir, name.slice(0, -1));
           await fs.promises.mkdir(path.dirname(file), { recursive: true });
           await pipeline(
             stream,
@@ -48,7 +73,7 @@ const extractTarGz = async (url, baseDir, include) => {
 
 (async () => {
   try {
-    console.info('Download W3C SVG 1.1 Test Suite and extract svg files');
+    console.info('Download W3C SVG 1.1 Test Suite and extract SVG files');
     await extractTarGz(
       'https://www.w3.org/Graphics/SVG/Test/20110816/archives/W3C_SVG_11_TestSuite.tar.gz',
       path.join(__dirname, 'regression-fixtures', 'w3c-svg-11-test-suite'),

test/regression.js

@@ -1,57 +1,49 @@
 'use strict';
 
-const fs = require('fs');
-const path = require('path');
+/**
+ * @typedef {import('playwright').Page} Page
+ * @typedef {import('playwright').PageScreenshotOptions} PageScreenshotOptions
+ */
+
+const fs = require('node:fs/promises');
 const http = require('http');
 const os = require('os');
+const path = require('path');
+const pixelmatch = require('pixelmatch');
 const { chromium } = require('playwright');
 const { PNG } = require('pngjs');
-const pixelmatch = require('pixelmatch');
 const { optimize } = require('../lib/svgo.js');
 
-const runTests = async ({ list }) => {
-  let skipped = 0;
+const width = 960;
+const height = 720;
+
+/** @type {PageScreenshotOptions} */
+const screenshotOptions = {
+  omitBackground: true,
+  clip: { x: 0, y: 0, width, height },
+  animations: 'disabled',
+};
+
+/**
+ * @param {string[]} list
+ * @returns {Promise<boolean>}
+ */
+const runTests = async (list) => {
   let mismatched = 0;
   let passed = 0;
-  list.reverse();
-  console.info('Start browser...');
+  console.info('Start browser…');
+  /**
+   * @param {Page} page
+   * @param {string} name
+   */
   const processFile = async (page, name) => {
-    if (
-      // animated
-      name.startsWith('w3c-svg-11-test-suite/svg/animate-') ||
-      name === 'w3c-svg-11-test-suite/svg/filters-light-04-f.svg' ||
-      name === 'w3c-svg-11-test-suite/svg/filters-composite-05-f.svg' ||
-      // messed gradients
-      name === 'w3c-svg-11-test-suite/svg/pservers-grad-18-b.svg' ||
-      // removing wrapping <g> breaks :first-child pseudo-class
-      name === 'w3c-svg-11-test-suite/svg/styling-pres-04-f.svg' ||
-      // rect is converted to path which matches wrong styles
-      name === 'w3c-svg-11-test-suite/svg/styling-css-08-f.svg' ||
-      // complex selectors are messed because of converting shapes to paths
-      name === 'w3c-svg-11-test-suite/svg/struct-use-10-f.svg' ||
-      name === 'w3c-svg-11-test-suite/svg/struct-use-11-f.svg' ||
-      name === 'w3c-svg-11-test-suite/svg/styling-css-01-b.svg' ||
-      name === 'w3c-svg-11-test-suite/svg/styling-css-04-f.svg' ||
-      // strange artifact breaks inconsistently  breaks regression tests
-      name === 'w3c-svg-11-test-suite/svg/filters-conv-05-f.svg'
-    ) {
-      console.info(`${name} is skipped`);
-      skipped += 1;
-      return;
-    }
     await page.goto(`http://localhost:5000/original/${name}`);
-    await page.setViewportSize({ width, height });
-    const originalBuffer = await page.screenshot({
-      omitBackground: true,
-      clip: { x: 0, y: 0, width, height },
-    });
+    const originalBuffer = await page.screenshot(screenshotOptions);
     await page.goto(`http://localhost:5000/optimized/${name}`);
-    const optimizedBuffer = await page.screenshot({
-      omitBackground: true,
-      clip: { x: 0, y: 0, width, height },
-    });
+    const optimizedBufferPromise = page.screenshot(screenshotOptions);
+
     const originalPng = PNG.sync.read(originalBuffer);
-    const optimizedPng = PNG.sync.read(optimizedBuffer);
+    const optimizedPng = PNG.sync.read(await optimizedBufferPromise);
     const diff = new PNG({ width, height });
     const matched = pixelmatch(
       originalPng.data,
@@ -63,24 +55,25 @@ const runTests = async ({ list }) => {
     // ignore small aliasing issues
     if (matched <= 4) {
       console.info(`${name} is passed`);
-      passed += 1;
+      passed++;
     } else {
-      mismatched += 1;
+      mismatched++;
       console.error(`${name} is mismatched`);
       if (process.env.NO_DIFF == null) {
         const file = path.join(
           __dirname,
           'regression-diffs',
           `${name}.diff.png`,
         );
-        await fs.promises.mkdir(path.dirname(file), { recursive: true });
-        await fs.promises.writeFile(file, PNG.sync.write(diff));
+        await fs.mkdir(path.dirname(file), { recursive: true });
+        await fs.writeFile(file, PNG.sync.write(diff));
       }
     }
   };
   const worker = async () => {
     let item;
     const page = await context.newPage();
+    await page.setViewportSize({ width, height });
     while ((item = list.pop())) {
       await processFile(page, item);
     }
@@ -93,44 +86,21 @@ const runTests = async ({ list }) => {
     Array.from(new Array(os.cpus().length * 2), () => worker()),
   );
   await browser.close();
-  console.info(`Skipped: ${skipped}`);
   console.info(`Mismatched: ${mismatched}`);
   console.info(`Passed: ${passed}`);
   return mismatched === 0;
 };
 
-const readdirRecursive = async (absolute, relative = '') => {
-  let result = [];
-  const list = await fs.promises.readdir(absolute, { withFileTypes: true });
-  for (const item of list) {
-    const itemAbsolute = path.join(absolute, item.name);
-    const itemRelative = path.join(relative, item.name);
-    if (item.isDirectory()) {
-      const itemList = await readdirRecursive(itemAbsolute, itemRelative);
-      result = [...result, ...itemList];
-    } else if (item.name.endsWith('.svg')) {
-      result = [...result, itemRelative];
-    }
-  }
-  return result;
-};
-
-const width = 960;
-const height = 720;
 (async () => {
   try {
     const start = process.hrtime.bigint();
     const fixturesDir = path.join(__dirname, 'regression-fixtures');
-    const list = await readdirRecursive(fixturesDir);
-    // setup server
+    const filesPromise = fs.readdir(fixturesDir, { recursive: true });
     const server = http.createServer(async (req, res) => {
       const name = req.url.slice(req.url.indexOf('/', 1));
       let file;
       try {
-        file = await fs.promises.readFile(
-          path.join(fixturesDir, name),
-          'utf-8',
-        );
+        file = await fs.readFile(path.join(fixturesDir, name), 'utf-8');
       } catch (error) {
         res.statusCode = 404;
         res.end();
@@ -144,14 +114,8 @@ const height = 720;
       }
       if (req.url.startsWith('/optimized/')) {
         const optimized = optimize(file, {
-          path: name,
           floatPrecision: 4,
         });
-        if (optimized.error) {
-          throw new Error(`Failed to optimize ${name}`, {
-            cause: optimized.error,
-          });
-        }
         res.setHeader('Content-Type', 'image/svg+xml');
         res.end(optimized.data);
         return;
@@ -161,9 +125,9 @@ const height = 720;
     await new Promise((resolve) => {
       server.listen(5000, resolve);
     });
-    const passed = await runTests({ list });
+    const list = (await filesPromise).filter((name) => name.endsWith('.svg'));
+    const passed = await runTests(list);
     server.close();
-    // compute time
     const end = process.hrtime.bigint();
     const diff = (end - start) / BigInt(1e6);
     if (passed) {