chore: speedier regression testing #1897
Conversation
KTibow
changed the title
test/regression.js
Outdated
| // setup server | ||
| const server = http.createServer((req, res) => { | ||
| if (req.url.startsWith('/original/')) { | ||
| const name = req.url.slice('/original/'.length); | ||
| if (originalFiles.has(name)) { | ||
| res.setHeader('Content-Type', 'image/svg+xml'); | ||
| res.end(originalFiles.get(name)); | ||
| const server = http.createServer(async (req, res) => { | ||
| const name = req.url.startsWith('/original/') | ||
| ? req.url.slice('/original/'.length) | ||
| : req.url.startsWith('/optimized/') | ||
| ? req.url.slice('/optimized/'.length) | ||
| : undefined; | ||
| let file; | ||
| if (name) { | ||
| try { | ||
| file = await fs.promises.readFile( | ||
| path.join(fixturesDir, name), | ||
| 'utf-8', | ||
| ); | ||
| } catch (error) { | ||
| res.statusCode = 404; | ||
| res.end(); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (req.url.startsWith('/original/')) { | ||
| res.setHeader('Content-Type', 'image/svg+xml'); | ||
| res.end(file); | ||
| return; | ||
| } | ||
| if (req.url.startsWith('/optimized/')) { | ||
| const name = req.url.slice('/optimized/'.length); | ||
| if (optimizedFiles.has(name)) { | ||
| res.setHeader('Content-Type', 'image/svg+xml'); | ||
| res.end(optimizedFiles.get(name)); | ||
| return; | ||
| 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; |
There was a problem hiding this comment.
I really wanted to get rid of the whole HTTP server, since we should be able to just use setContent in Playwright. Unfortunately, there appears to be a bug that stops us from doing that right now.
Usage would be as follows, so much simpler:
const fixturesDir = path.join(__dirname, 'regression-fixtures');
const runTests = async ({ list }) => {
let mismatched = 0;
let passed = 0;
const processFile = async (page, name) => {
const original = await fs.promises.readFile(
path.join(fixturesDir, name),
'utf-8',
);
const optimized = optimize(original, {
path: name,
floatPrecision: 4,
});
await page.setContent(original);
const originalBuffer = await page.screenshot({
omitBackground: true,
clip: { x: 0, y: 0, width, height },
});
await page.setContent(optimized.data);
// …
|
I see… that would explain that logic. We had the fallback to empty response because some of the SVGs try to load other resources, and we want to ignore them. Perhaps you can just put that back for now. Now that the issue with the Sorry for the bad call on my part! |
|
I'm responding to this comment here, since it's more to do with our usage than the bug report itself: microsoft/playwright#28778 (comment) I initially didn't try it deliberately, since inlining an SVG and loading it via an However, I did just try it now and it does work… kind of. Failing tests went from 262 to 1,000+, and it's much slower. Possibly because of the URI encoding to make it work in a URL format, plus the browser has to load more data since the encoding makes the SVG fat. I'll come back to that later, but for now, we can keep the HTTP server. 🤔 |
Goes from 76 seconds to 61 seconds in my tests.
Two main changes:
Bonus: this means that they go in alphabetical order
This means no optimizing skipped tests
Admittedly won't be as useful once the PR that moves skipping tests into the extractor gets merged, but hey, it'll still decrease initial startup time
*I was going to make it even faster with
node-canvas, but the w3 tests are a bit eccentric and fail to render properly there, and I don't know of any other convenient rendering libs