Skip to content

Commit 10350e2

Browse files
authored
chore: split process for optimize and compare (#2168)
1 parent cc489c2 commit 10350e2

File tree

12 files changed

+461
-300
lines changed

12 files changed

+461
-300
lines changed

.gitignore

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ dist
2525
types
2626

2727
node_modules
28-
regression-fixtures
29-
regression-diffs
3028
test/cli/output
3129
coverage
3230
.DS_Store

CONTRIBUTING.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ Parameters must have types declared in a [`@typedef`](https://jsdoc.app/tags-typ
5959

6060
### Testing
6161

62-
Our regression test suite includes larger SVGs that may take a long time to render and optimize, especially on older machines. The default timeout is 20 minutes, but can be increased in [`test/regression.js`](https://github.com/svg/svgo/blob/main/test/regression.js) by modifying `NAVIGATION_TIMEOUT_MS`. Setting the value to `0` will disable the timeout entirely.
63-
64-
If an SVG can not be optimized within 10 minutes in CI, then that indicates a significant performance problem that must be addressed.
62+
Our regression test suite includes larger SVGs that may take a long time to optimize and render, especially on older machines. If an SVG can not be optimized within 10 minutes in CI, then that indicates a significant performance problem that must be addressed.
6563

6664
> [!IMPORTANT]
6765
> Regression test results vary between hosts. It's not known why yet, but it's likely related to the host environment, such as system packages, drivers, or hardware.

eslint.config.mjs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ export default [
99
'.yarnrc.yml',
1010
'node_modules/**',
1111
'dist/**',
12-
'test/regression-fixtures/**',
13-
'test/regression-diffs/**',
1412
'test/cli/output/**',
1513
'coverage/**',
1614
],

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
"test": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --maxWorkers=4 --coverage",
8484
"test:bundles": "yarn build:bundles && node ./test/svgo.cjs && node ./test/browser.js",
8585
"test:types": "yarn build:types && tsc && tsd",
86-
"test:regression": "node ./test/regression/extract.js && cross-env NO_DIFF=1 node ./test/regression/regression.js",
86+
"test:regression": "node ./test/regression/extract.js && node ./test/regression/optimize.js && cross-env NO_DIFF=1 node ./test/regression/compare.js",
8787
"qa": "yarn lint && yarn test:types && yarn test && yarn test:bundles && yarn test:regression",
8888
"clean": "yarn clean:build && yarn clean:types",
8989
"clean:build": "rimraf dist",

test/regression/compare.js

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import fs from 'node:fs/promises';
2+
import os from 'node:os';
3+
import path from 'node:path';
4+
import pixelmatch from 'pixelmatch';
5+
import { chromium } from 'playwright';
6+
import { PNG } from 'pngjs';
7+
import { expectMismatch, ignore, skip } from './file-lists.js';
8+
import { pathToPosix, printReport } from './lib.js';
9+
import {
10+
readReport,
11+
readVersion,
12+
REGRESSION_DIFFS_PATH,
13+
REGRESSION_FIXTURES_PATH,
14+
REGRESSION_OPTIMIZED_PATH,
15+
writeReport,
16+
} from './regression-io.js';
17+
18+
const NAVIGATION_TIMEOUT_MS = 0;
19+
const WIDTH = 960;
20+
const HEIGHT = 720;
21+
22+
/** @type {import('playwright').PageScreenshotOptions} */
23+
const screenshotOptions = {
24+
omitBackground: true,
25+
clip: { x: 0, y: 0, width: WIDTH, height: HEIGHT },
26+
animations: 'disabled',
27+
};
28+
29+
/**
30+
* @param {ReadonlyArray<string>} list
31+
* @returns {Promise<Omit<import('./regression-io.js').TestReport, 'metrics' | 'checksums'>>}
32+
*/
33+
const runTests = async (list) => {
34+
const version = await readVersion();
35+
const listCopy = [...list];
36+
37+
/** @type {Omit<import('./regression-io.js').TestReport, 'metrics' | 'checksums'>} */
38+
const report = {
39+
version,
40+
files: {
41+
toMatch: listCopy.length - expectMismatch.length - ignore.length,
42+
toMismatch: expectMismatch.length,
43+
toIgnore: ignore.length,
44+
toSkip: skip.length,
45+
},
46+
results: {
47+
match: 0,
48+
expectMismatch: 0,
49+
ignored: 0,
50+
},
51+
errors: {
52+
shouldHaveMatched: [],
53+
shouldHaveMismatched: [],
54+
},
55+
};
56+
57+
const totalFiles = listCopy.length;
58+
let tested = 0;
59+
60+
/**
61+
* @param {import('playwright').Page} page
62+
* @param {string} name
63+
*/
64+
const processFile = async (page, name) => {
65+
await page.goto(`file://${path.join(REGRESSION_FIXTURES_PATH, name)}`);
66+
const originalBuffer = await page.screenshot(screenshotOptions);
67+
68+
await page.goto(`file://${path.join(REGRESSION_OPTIMIZED_PATH, name)}`);
69+
const optimizedBufferPromise = page.screenshot(screenshotOptions);
70+
71+
const writeDiffs = process.env.NO_DIFF == null;
72+
const diff = writeDiffs ? new PNG({ width: WIDTH, height: HEIGHT }) : null;
73+
const originalPng = PNG.sync.read(originalBuffer);
74+
const optimizedPng = PNG.sync.read(await optimizedBufferPromise);
75+
const matched = pixelmatch(
76+
originalPng.data,
77+
optimizedPng.data,
78+
diff?.data,
79+
WIDTH,
80+
HEIGHT,
81+
);
82+
83+
// ignore small aliasing issues
84+
const isMatch = matched <= 4;
85+
const namePosix = pathToPosix(name);
86+
const expectedToMismatch = expectMismatch.includes(namePosix);
87+
88+
if (isMatch) {
89+
if (expectedToMismatch) {
90+
report.errors.shouldHaveMismatched.push(namePosix);
91+
} else if (ignore.includes(namePosix)) {
92+
report.results.ignored++;
93+
} else {
94+
report.results.match++;
95+
}
96+
} else {
97+
if (expectedToMismatch) {
98+
report.results.expectMismatch++;
99+
} else if (!ignore.includes(namePosix)) {
100+
report.errors.shouldHaveMatched.push(namePosix);
101+
}
102+
103+
if (diff) {
104+
const file = path.join(REGRESSION_DIFFS_PATH, `${name}.diff.png`);
105+
await fs.mkdir(path.dirname(file), { recursive: true });
106+
await fs.writeFile(file, PNG.sync.write(diff));
107+
}
108+
}
109+
110+
if (process.stdout.isTTY) {
111+
process.stdout.clearLine(0);
112+
process.stdout.write(
113+
`\rCompared ${(++tested).toLocaleString()} of ${totalFiles.toLocaleString()}…`,
114+
);
115+
}
116+
};
117+
118+
const worker = async () => {
119+
let item;
120+
const page = await context.newPage();
121+
while ((item = listCopy.pop())) {
122+
await processFile(page, item);
123+
}
124+
await page.close();
125+
};
126+
127+
const browser = await chromium.launch();
128+
const context = await browser.newContext({
129+
javaScriptEnabled: false,
130+
viewport: { width: WIDTH, height: HEIGHT },
131+
});
132+
context.setDefaultTimeout(NAVIGATION_TIMEOUT_MS);
133+
134+
await Promise.all(
135+
Array.from(new Array(os.cpus().length * 2), () => worker()),
136+
);
137+
await browser.close();
138+
139+
if (process.stdout.isTTY) {
140+
console.log();
141+
}
142+
143+
return report;
144+
};
145+
146+
(async () => {
147+
try {
148+
const filesPromise = fs.readdir(REGRESSION_FIXTURES_PATH, {
149+
recursive: true,
150+
});
151+
const list = (await filesPromise).filter((name) => name.endsWith('.svg'));
152+
153+
const report = await runTests(list);
154+
const metrics = await readReport();
155+
const combinedReport = {
156+
...report,
157+
...metrics,
158+
};
159+
160+
printReport(
161+
/** @type {import('./regression-io.js').TestReport}*/ (combinedReport),
162+
);
163+
await writeReport(combinedReport);
164+
165+
const failed =
166+
report.results.match !== report.files.toMatch ||
167+
report.results.expectMismatch !== report.files.toMismatch;
168+
169+
if (failed) {
170+
process.exit(1);
171+
}
172+
} catch (error) {
173+
console.error(error);
174+
process.exit(1);
175+
}
176+
})();

test/regression/extract.js

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
1-
import fs from 'fs';
2-
import path from 'path';
3-
import stream from 'stream';
4-
import util from 'util';
5-
import zlib from 'zlib';
1+
/**
2+
* @fileoverview Download and extracts the latest version of SVGO Test Suite.
3+
*/
4+
5+
import fs from 'node:fs';
6+
import path from 'node:path';
7+
import stream from 'node:stream';
8+
import util from 'node:util';
9+
import zlib from 'node:zlib';
610
import tarStream from 'tar-stream';
7-
import { fileURLToPath } from 'url';
811
import { skip, validateFileLists } from './file-lists.js';
9-
10-
const __dirname = path.dirname(fileURLToPath(import.meta.url));
12+
import {
13+
readPrevEtag,
14+
REGRESSION_FIXTURES_PATH,
15+
writeEtag,
16+
TEMP_DIR_PATH,
17+
} from './regression-io.js';
1118

1219
const pipeline = util.promisify(stream.pipeline);
1320

@@ -49,22 +56,43 @@ const extractTarGz = async (url, baseDir) => {
4956
next();
5057
});
5158

52-
const { body } = await fetch(url);
59+
const etag = await readPrevEtag();
60+
61+
/** @type {Record<string, string>} */
62+
const headers = {};
5363

54-
if (!body) {
55-
throw Error('No body returned when fetching SVGO Test Suite.');
64+
if (etag) {
65+
headers['If-None-Match'] = etag;
5666
}
5767

58-
await pipeline(body, zlib.createGunzip(), extract);
59-
await validateFileLists(svgs);
68+
const response = await fetch(url, {
69+
headers,
70+
});
71+
72+
if (response.status === 200 && response.body) {
73+
console.info('Downloading SVGO Test Suite and extracting files…');
74+
await fs.promises.rm(REGRESSION_FIXTURES_PATH, {
75+
recursive: true,
76+
force: true,
77+
});
78+
await pipeline(response.body, zlib.createGunzip(), extract);
79+
await validateFileLists(svgs);
80+
81+
const etag = response.headers.get('ETag');
82+
if (etag) {
83+
await writeEtag(etag);
84+
}
85+
} else if (response.status === 304) {
86+
console.info('Reusing local copy of SVGO Test Suite');
87+
}
6088
};
6189

6290
(async () => {
6391
try {
64-
console.info('Downloading SVGO Test Suite and extracting files');
92+
console.info('Using temporary directory: %s\n', TEMP_DIR_PATH);
6593
await extractTarGz(
6694
'https://svg.github.io/svgo-test-suite/svgo-test-suite.tar.gz',
67-
path.join(__dirname, 'regression-fixtures'),
95+
REGRESSION_FIXTURES_PATH,
6896
);
6997
} catch (error) {
7098
console.error(error);

test/regression/file-lists.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
/**
2+
* @fileoverview
3+
* Helpers to interact with file lists. File lists are used to configure
4+
* exceptions in SVGO Test Suite.
5+
*
6+
* All file lists are defined in `./lists`.
7+
*
8+
* Syntax:
9+
* * All entires must be written with POSIX file separators.
10+
* * Comments can be written by putting `#` at the start of the line.
11+
* * Blank lines are ignored.
12+
*/
13+
114
import path from 'path';
215
import { fileURLToPath } from 'url';
316
import { readFileList, toBulletPointList } from './lib.js';

test/regression/lib.js

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import crypto from 'node:crypto';
6+
import path from 'node:path';
67
import fs from 'node:fs/promises';
78
import picocolors from 'picocolors';
89

@@ -37,18 +38,18 @@ export function md5sum(value) {
3738
}
3839

3940
/**
40-
* @param {import('./regression.js').TestReport} report
41+
* @param {import('./regression-io.js').TestReport} report
4142
*/
4243
export async function printReport(report) {
4344
const { shouldHaveMatched, shouldHaveMismatched } = report.errors;
4445

4546
console.log(`SVGO Test Suite Version: ${report.version}
4647
4748
▶ Test Results
48-
Match: ${report.results.match.toLocaleString()} / ${report.suite.toMatch.toLocaleString()}
49-
Expected Mismatch: ${report.results.expectMismatch.toLocaleString()} / ${report.suite.toMismatch.toLocaleString()}
50-
Ignored: ${report.results.ignored.toLocaleString()} / ${report.suite.toIgnore.toLocaleString()}
51-
Skipped: ${report.suite.toSkip.toLocaleString()}
49+
Match: ${report.results.match.toLocaleString()} / ${report.files.toMatch.toLocaleString()}
50+
Expected Mismatch: ${report.results.expectMismatch.toLocaleString()} / ${report.files.toMismatch.toLocaleString()}
51+
Ignored: ${report.results.ignored.toLocaleString()} / ${report.files.toIgnore.toLocaleString()}
52+
Skipped: ${report.files.toSkip.toLocaleString()}
5253
5354
▶ Metrics
5455
Bytes Saved: ${bytesToHumanReadable(report.metrics.bytesSaved)}
@@ -110,7 +111,7 @@ export function secsToHumanReadable(secs) {
110111
arr.push(`${minutes.toString().padStart(2, '0')}m`);
111112
}
112113

113-
arr.push(`${secs.toString().padStart(2, '0')}s`);
114+
arr.push(`${Math.round(secs).toString().padStart(2, '0')}s`);
114115
return arr.join('');
115116
}
116117

@@ -122,3 +123,14 @@ export function secsToHumanReadable(secs) {
122123
export function toBulletPointList(arr, bullet = '*') {
123124
return arr.map((s) => `${bullet} ${s}`).join('\n');
124125
}
126+
127+
/**
128+
* @param {string} filepath
129+
* Path that uses file separators for the current operating system.
130+
* ({@link path.sep})
131+
* @returns {string}
132+
* Same path but with POSIX file separators. ({@link path.posix.sep})
133+
*/
134+
export function pathToPosix(filepath) {
135+
return filepath.replaceAll(path.sep, path.posix.sep);
136+
}

test/regression/lists/expect-mismatch.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,16 @@ oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/application-javascript.svg
138138
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/application-pdf.svg
139139
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/application-x-javascript.svg
140140
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/application-x-srt.svg
141+
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/application-x-srtrip.svg
141142
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/text-csharp.svg
142143
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/text-css.svg
143144
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/text-plain.svg
144-
oxygen-icons-5.116.0/scalable/mimetypes/small/22x22/text-x-changelog.svg
145145
oxygen-icons-5.116.0/scalable/mimetypes/small/32x32/application-x-applix-word.svg
146146
oxygen-icons-5.116.0/scalable/mimetypes/small/32x32/application-x-lyx.svg
147147
oxygen-icons-5.116.0/scalable/mimetypes/small/48x48/application-x-cd-image.svg
148+
oxygen-icons-5.116.0/scalable/mimetypes/small/48x48/application-x-cda.svg
148149
oxygen-icons-5.116.0/scalable/mimetypes/small/48x48/application-x-cue.svg
149150
oxygen-icons-5.116.0/scalable/mimetypes/small/48x48/help.svg
150-
oxygen-icons-5.116.0/scalable/mimetypes/small/48x48/text-xmcd.svg
151151
oxygen-icons-5.116.0/scalable/mimetypes/small/64x64/application-x-cue.svg
152152
oxygen-icons-5.116.0/scalable/places/small/64x64/folder-tar.svg
153153
oxygen-icons-5.116.0/scalable/places/small/64x64/network-server-database.svg

0 commit comments

Comments
 (0)