microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.67k stars 3.65k forks source link

[Bug]: toHaveScreenshot changes page style and changes font family #29968

Open muhammad-saleh opened 7 months ago

muhammad-saleh commented 7 months ago

Version

1.42.1

Steps to reproduce

  1. Clone my repo at https://github.com/muhammad-saleh/playwright-screenshot-bug-repro
  2. npm install
  3. npx playwright test --headed (or headless)

Expected behavior

I expected that the full page screenshot should match the actual page

Actual behavior

I found that at the time toHaveScreenshot is executed, it changes the page styles, and the screenshot has a different font than the actual rendered page.

Running in headed mode, I noticed that the font is properly loaded and rendered, but it's exactly when toHaveScreenshot is executed that this problem happens.

Note: This doesn't happen when I set fullpage: false

Expected font: image

Incorrect font in screenshot: image

Same behavior also happens in CI with mcr.microsoft.com/playwright:v1.42.1-jammy but it's flaky.

Additional context

No response

Environment

System:
    OS: Linux 6.7 Fedora Linux 39 (Workstation Edition)
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i7-13650HX
    Memory: 22.30 GB / 30.97 GB
    Container: Yes
  Binaries:
    Node: 20.10.0 - /usr/bin/node
    npm: 10.2.3 - /usr/bin/npm
    pnpm: 8.15.4 - ~/.local/share/pnpm/pnpm
  IDEs:
    VSCode: 1.87.2 - /usr/bin/code
  Languages:
    Bash: 5.2.26 - /usr/bin/bash
yury-s commented 7 months ago

Apparently in some cases Chromium triggers font update which interferes with the capturing screenshots. Most likely it is because of this code. The behavior is flaky.

yury-s commented 7 months ago

Investigation notes: after unsuccessful screenshot browser requests the fonts again:

request finished https://tempo.fit/fonts/Moderat/Moderat-Medium.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Regular.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Regular.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Medium.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Regular.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Medium.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Bold.woff2
will screenshot
{
  format: 'png',
  quality: undefined,
  clip: { x: 0, y: 0, width: 1280, height: 3622, scale: 1 },
  captureBeyondViewport: true
}
request finished https://tempo.fit/fonts/Moderat/Moderat-Regular.woff2
request finished https://tempo.fit/fonts/Moderat/Moderat-Medium.woff2

We didn't see such requests in DevTools though.

This was from the following test:

import { test, expect } from '@playwright/test';
import { Page } from "@playwright/test";

export const delay = (ms: number) =>
  new Promise((resolve) => setTimeout(resolve, ms));

test.setTimeout(180000);
test('capture screenshot', async ({ page }) => {
  page.on('requestfinished', (request) => {
    if (request.url().includes('fonts/Moderat'))
      console.log('request finished', request.url());
  });
  await page.goto(`https://tempo.fit`, {
    waitUntil: "networkidle",
  });

  await page.evaluate(() => document.fonts.ready);
  await delay(10000);
  await page.keyboard.down('Escape');
  await page.keyboard.down('Escape');
  await page.keyboard.down('Escape');
  await delay(1000);

  await page.evaluate(() => document.fonts.ready);

  console.log('will screenshot');
  await expect(page).toHaveScreenshot({
    fullPage: true,
    animations: 'allow',
    caret: 'initial',
    scale: 'device',
  });
  console.log('did screenshot');
});
muhammad-saleh commented 7 months ago

@yury-s Thank you for looking into this. Is there a way to prevent Chromium from requesting the fonts again? Any fix or workaround do you recommend?

yury-s commented 7 months ago

No easy workaround unfortunately. You can intercept requests to **/*fonts/Moderat* and abort them but that will change the look of the page.

Rafiot commented 7 months ago

I'm having the same kind of issue and I added (in python) that line before calling the screenshot:

await page.route("**/*", lambda route: route.abort())

But it doesn't seem to change anything. What do you mean by intercept requests?

muhammad-saleh commented 7 months ago

I was able to solve this by this workaround:

  1. As @yury-s suggested, I blocked this font family requests
  2. In our test code I added the font files
  3. In the CI, I copied the font files to the system and cleared the font cache
  4. Run the tests
  5. Chromium will fallback to system fonts since the font family requests are blocked
Rafiot commented 7 months ago

@muhammad-saleh how did you block the requests?

muhammad-saleh commented 7 months ago

@Rafiot

  test.beforeEach(async ({ context }) => {
    await context.route(/Moderat/, (route) => route.abort());
  });
Rafiot commented 7 months ago

alright, I'll try that again, but it still seemed to get stuck (with the equivalent call in Python).

sameeryoussef-kb commented 2 months ago

expect.toHaveScreenshot('expected.png', { fullPage: true, }) is causing my page to trigger the mobile responsive breakpoints momentarily - causing flakiness. It sounds like we need another argument that allows the page to settle (whether it before UI updates or font loads etc).

rustyy commented 3 weeks ago

Same here, I worked around this by setting fullpage to false and instead updating the viewport-size by the actual document height, right before executing toHaveScreenshot. This solved flaky tests for now.

async function scrollPage(page: Page, scrollInterval = 100, distance = 100) {
  return page.evaluate(
    async ([scrollInterval, distance]) => {
      return new Promise<number>((resolve) => {
        let totalHeight = 0;
        const timer = setInterval(() => {
          const scrollHeight = document.body.scrollHeight;
          window.scrollBy(0, distance);
          totalHeight += distance;

          if (totalHeight >= scrollHeight) {
            clearInterval(timer);
            window.scrollTo(0, 0);
            resolve(totalHeight);
          }
        }, scrollInterval);
      });
    },
    [scrollInterval, distance],
  );
}

test("Page Screenshot", async ({ page, viewport }) => {
  await page.goto("https://www.example.com");

  // need to scroll through the entire site anyways, so scrollPage collects the total height and returns it
  const height = await scrollPage(page);

  if (viewport) {
    await page.setViewportSize({
      width: viewport?.width,
      height,
    });
  }

  await expect(page).toHaveScreenshot({
    fullPage: false,
  });
});

As an alternative:

test("Page Screenshot", async ({ page, viewport }) => {
  await page.goto("https://www.example.com");

  const height = await page.evaluate(() => {
    const body = document.body;
    const html = document.documentElement;

    return Math.max(
      body.scrollHeight,
      body.offsetHeight,
      html.clientHeight,
      html.scrollHeight,
      html.offsetHeight,
    );
  });

  if (viewport) {
    await page.setViewportSize({
      width: viewport?.width,
      height,
    });
  }

  await expect(page).toHaveScreenshot({
    fullPage: false,
  });
});