feat(toHaveScreenshot): align screenshot generation & comparison (#12812)

This patch aligns the strategies that are used to generate new
screnshot expectations and to compare screenshot expectations against
baseline.

With this patch, `toHaveScreenshot` will:
- when generating a new expectation: will wait for 2 consecutive
  screenshots to match and accept the last one as expectation.
- when given an expectation:
  * will compare first screenshot against expectation. If matches,
    resolve successfully
  * if first screenshot doesn't match, then wait for 2 consecutive
    screenshots to match and then compare last screenshot with the
    expectation.

An example of a new detailed call log:

```
  1) a.spec.ts:3:1 › should work ===================================================================

    Error: Screenshot comparison failed:

      20000 pixels (ratio 0.03 of all image pixels) are different

    Call log:
      - expect.toHaveScreenshot with timeout 5000ms
      -   verifying given screenshot expectation
      - fast-path: checking first screenshot to match expectation
      - taking page screenshot
      -   disabled all CSS animations
      -   waiting for fonts to load...
      -   fonts in all frames are loaded
      - fast-path failed: first screenshot did not match expectation - 20000 pixels (ratio 0.03 of all image pixels) are different
      - waiting for 2 consecutive screenshots to match
      - waiting 100ms before taking screenshot
      - taking page screenshot
      -   disabled all CSS animations
      -   waiting for fonts to load...
      -   fonts in all frames are loaded
      - 2 consecutive screenshots matched
      - final screenshot did not match expectation - 20000 pixels (ratio 0.03 of all image pixels) are different
      - 20000 pixels (ratio 0.03 of all image pixels) are different

    Expected: /Users/andreylushnikov/tmp/test-results/a-should-work/should-work-1-expected.png
    Received: /Users/andreylushnikov/tmp/test-results/a-should-work/should-work-1-actual.png
        Diff: /Users/andreylushnikov/tmp/test-results/a-should-work/should-work-1-diff.png

      3 | test('should work', async ({ page }) => {
      4 |   await page.goto('file:///Users/andreylushnikov/prog/playwright/tests/assets/rotate-z.html');
    > 5 |   await expect(page).toHaveScreenshot();
        |                      ^
      6 | });
      7 |
```
This commit is contained in:
Andrey Lushnikov 2022-03-21 16:10:33 -06:00 committed by GitHub
parent 67e754f6b5
commit c18077c0de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 48 additions and 30 deletions

View File

@ -31,7 +31,7 @@ import { Progress, ProgressController } from './progress';
import { assert, isError } from '../utils/utils';
import { ManualPromise } from '../utils/async';
import { debugLogger } from '../utils/debugLogger';
import { mimeTypeToComparator, ImageComparatorOptions, ComparatorResult } from '../utils/comparators';
import { mimeTypeToComparator, ImageComparatorOptions } from '../utils/comparators';
import { SelectorInfo, Selectors } from './selectors';
import { CallMetadata, SdkObject } from './instrumentation';
import { Artifact } from './artifact';
@ -458,53 +458,72 @@ export class Page extends SdkObject {
const comparator = mimeTypeToComparator['image/png'];
const controller = new ProgressController(metadata, this);
const isGeneratingNewScreenshot = !options.expected;
if (isGeneratingNewScreenshot && options.isNot)
if (!options.expected && options.isNot)
return { errorMessage: '"not" matcher requires expected result' };
let intermediateResult: {
actual?: Buffer,
previous?: Buffer,
errorMessage?: string,
errorMessage: string,
diff?: Buffer,
} | undefined = undefined;
const areEqualScreenshots = (actual: Buffer | undefined, expected: Buffer | undefined, previous: Buffer | undefined) => {
const comparatorResult = actual && expected ? comparator(actual, expected, options.comparatorOptions) : undefined;
if (comparatorResult !== undefined && !!comparatorResult === !!options.isNot)
return true;
if (comparatorResult)
intermediateResult = { errorMessage: comparatorResult.errorMessage, diff: comparatorResult.diff, actual, previous };
return false;
};
const callTimeout = this._timeoutSettings.timeout(options);
return controller.run(async progress => {
let actual: Buffer | undefined;
let previous: Buffer | undefined;
const pollIntervals = [0, 100, 250, 500];
progress.log(`${metadata.apiName}${callTimeout ? ` with timeout ${callTimeout}ms` : ''}`);
if (isGeneratingNewScreenshot)
progress.log(` generating new screenshot expectation: waiting for 2 consecutive screenshots to match`);
if (options.expected)
progress.log(` verifying given screenshot expectation`);
else
progress.log(` waiting for screenshot to match expectation`);
progress.log(` generating new stable screenshot expectation`);
let isFirstIteration = true;
while (true) {
progress.throwIfAborted();
if (this.isClosed())
throw new Error('The page has closed');
let comparatorResult: ComparatorResult | undefined;
const screenshotTimeout = pollIntervals.shift() ?? 1000;
if (screenshotTimeout)
progress.log(`waiting ${screenshotTimeout}ms before taking screenshot`);
if (isGeneratingNewScreenshot) {
previous = actual;
actual = await rafrafScreenshot(progress, screenshotTimeout).catch(e => undefined);
comparatorResult = actual && previous ? comparator(actual, previous, options.comparatorOptions) : undefined;
} else {
actual = await rafrafScreenshot(progress, screenshotTimeout).catch(e => undefined);
comparatorResult = actual ? comparator(actual, options.expected!, options.comparatorOptions) : undefined;
}
if (comparatorResult !== undefined && !!comparatorResult === !!options.isNot)
previous = actual;
actual = await rafrafScreenshot(progress, screenshotTimeout).catch(e => {
progress.log(`failed to take screenshot - ` + e.message);
return undefined;
});
if (!actual)
continue;
// Compare against expectation for the first iteration.
const expectation = options.expected && isFirstIteration ? options.expected : previous;
if (areEqualScreenshots(actual, expectation, previous))
break;
if (comparatorResult) {
if (isGeneratingNewScreenshot)
progress.log(`2 last screenshots do not match: ${comparatorResult.errorMessage}`);
else
progress.log(`screenshot does not match expectation: ${comparatorResult.errorMessage}`);
intermediateResult = { errorMessage: comparatorResult.errorMessage, diff: comparatorResult.diff, actual, previous };
}
if (intermediateResult)
progress.log(intermediateResult.errorMessage);
isFirstIteration = false;
}
return isGeneratingNewScreenshot ? { actual } : {};
if (!isFirstIteration)
progress.log(`captured a stable screenshot`);
if (!options.expected)
return { actual };
if (isFirstIteration) {
progress.log(`screenshot matched expectation`);
return {};
}
if (areEqualScreenshots(actual, options.expected, previous)) {
progress.log(`screenshot matched expectation`);
return {};
}
throw new Error(intermediateResult!.errorMessage);
}, callTimeout).catch(e => {
// Q: Why not throw upon isSessionClosedError(e) as in other places?
// A: We want user to receive a friendly diff between actual and expected/previous.

View File

@ -24,7 +24,7 @@ import { diff_match_patch, DIFF_INSERT, DIFF_DELETE, DIFF_EQUAL } from '../third
const { PNG } = require(require.resolve('pngjs', { paths: [require.resolve('pixelmatch')] })) as typeof import('pngjs');
export type ImageComparatorOptions = { threshold?: number, maxDiffPixels?: number, maxDiffPixelRatio?: number };
export type ComparatorResult = { diff?: Buffer; errorMessage?: string; } | null;
export type ComparatorResult = { diff?: Buffer; errorMessage: string; } | null;
export type Comparator = (actualBuffer: Buffer | string, expectedBuffer: Buffer, options?: any) => ComparatorResult;
export const mimeTypeToComparator: { [key: string]: Comparator } = {
'application/octet-string': compareBuffersOrStrings,

View File

@ -52,7 +52,7 @@ test('should fail to screenshot a page with infinite animation', async ({ runInl
expect(result.exitCode).toBe(1);
expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded while generating screenshot because page kept changing`);
expect(stripAnsi(result.output)).toContain(`expect.toHaveScreenshot with timeout 2000ms`);
expect(stripAnsi(result.output)).toContain(`generating new screenshot expectation: waiting for 2 consecutive screenshots to match`);
expect(stripAnsi(result.output)).toContain(`generating new stable screenshot expectation`);
expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(true);
expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(false);
expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-previous.png'))).toBe(true);
@ -370,8 +370,8 @@ test('should fail when screenshot is different size', async ({ runInlineTest })
`
});
expect(result.exitCode).toBe(1);
expect(stripAnsi(result.output)).toContain(`Timeout 2000ms exceeded`);
expect(stripAnsi(result.output)).toContain(`waiting for screenshot to match expectation`);
expect(stripAnsi(result.output)).toContain(`verifying given screenshot expectation`);
expect(stripAnsi(result.output)).toContain(`captured a stable screenshot`);
expect(result.output).toContain('Expected an image 22px by 33px, received 1280px by 720px.');
});
@ -411,7 +411,6 @@ test('should fail when screenshot is different pixels', async ({ runInlineTest }
});
expect(result.exitCode).toBe(1);
expect(result.output).toContain('Screenshot comparison failed');
expect(result.output).toContain(`Timeout 2000ms exceeded`);
expect(result.output).toContain('12345 pixels');
expect(result.output).toContain('Call log');
expect(result.output).toContain('ratio 0.02');