Skip to content

Commit 1c3af3d

Browse files
OrKoNDevtools-frontend LUCI CQ
authored and
Devtools-frontend LUCI CQ
committed
Handle screenshot errors in unit tests
Recovers image diff view in test failures: https://screenshot.googleplex.com/9dKe3HUNzbj6Zpz Fixed: 408112810 Change-Id: Ieb588c8cfb18d63549b179620809a70a1010a753 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6426041 Reviewed-by: Philip Pfaffe <pfaffe@chromium.org> Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
1 parent be53255 commit 1c3af3d

File tree

4 files changed

+25
-5
lines changed

4 files changed

+25
-5
lines changed

front_end/testing/DOMHelpers.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ export async function assertScreenshot(filename: string) {
331331
}
332332
await raf();
333333
// @ts-expect-error see karma config.
334-
const result = await window.assertScreenshot(`#${TEST_CONTAINER_ID}`, filename);
335-
if (result) {
336-
throw new Error(result);
334+
const errorMessage = await window.assertScreenshot(`#${TEST_CONTAINER_ID}`, filename);
335+
if (errorMessage) {
336+
throw new Error(errorMessage);
337337
}
338338
}

test/conductor/karma-resultsdb-reporter.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
/* eslint-disable @typescript-eslint/no-explicit-any */
66

77
import * as ResultsDb from './resultsdb.js';
8+
import {ScreenshotError} from './screenshot-error.js';
89

910
const chalk = require('chalk');
1011
const diff = require('diff');
@@ -113,6 +114,19 @@ export const ResultsDBReporter = function(
113114
}
114115

115116
const testResult: ResultsDb.TestResult = {testId, duration, status, expected, summaryHtml};
117+
118+
if (result.log?.[0]?.startsWith('Error: ScreenshotError')) {
119+
const screenshotError = ScreenshotError.errors.shift();
120+
if (screenshotError) {
121+
// Assert that the screenshot error matches the log.
122+
// If it does not, it means something is wrong
123+
// with the order of assertions and tests.«
124+
if (!result.log?.[0]?.includes(screenshotError.message)) {
125+
throw new Error('Unexpected screenshot assertion error');
126+
}
127+
[testResult.artifacts, testResult.summaryHtml] = screenshotError.toMiloArtifacts();
128+
}
129+
}
116130
ResultsDb.sendTestResult(testResult);
117131
};
118132
this.specSuccess = specComplete;

test/conductor/screenshot-error.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export class ScreenshotError extends Error {
1717
// The max length of the summary is 4000, but we need to leave some room for
1818
// the rest of the HTML formatting (e.g. <pre> and </pre>).
1919
static readonly SUMMARY_LENGTH_CUTOFF = 3900;
20+
static errors: ScreenshotError[] = [];
2021
readonly screenshots: ArtifactGroup;
2122

2223
private constructor(screenshots: ArtifactGroup, message?: string, cause?: Error) {

test/unit/karma.conf.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {formatAsPatch, resultAssertionsDiff, ResultsDBReporter} from '../../test
1010
import {CHECKOUT_ROOT, GEN_DIR, SOURCE_ROOT} from '../../test/conductor/paths.js';
1111
import * as ResultsDb from '../../test/conductor/resultsdb.js';
1212
import {loadTests, TestConfig} from '../../test/conductor/test_config.js';
13+
import {ScreenshotError} from '../conductor/screenshot-error.js';
1314
import {assertElementScreenshotUnchanged} from '../shared/screenshots.js';
1415

1516
const puppeteer = require('puppeteer-core');
@@ -61,8 +62,12 @@ const CustomChrome = function(this: any, _baseBrowserDecorator: unknown, args: B
6162
await assertElementScreenshotUnchanged(element, filename, {
6263
captureBeyondViewport: false,
6364
});
64-
} catch (err) {
65-
return err.message;
65+
return undefined;
66+
} catch (error) {
67+
if (error instanceof ScreenshotError) {
68+
ScreenshotError.errors.push(error);
69+
}
70+
return `ScreenshotError: ${error.message}`;
6671
}
6772
});
6873

0 commit comments

Comments
 (0)