test-runner icon indicating copy to clipboard operation
test-runner copied to clipboard

Wait for network idle on page.goto

Open shilman opened this issue 3 years ago • 19 comments

example: in CE project, some logos took time to load which resulted in flaky snapshot tests

shilman avatar Feb 14 '22 14:02 shilman

The following works when using @storybook/addon-storyshots and Playwright:

await Promise.all([
  page.waitForLoadState(`networkidle`),
  page.waitForLoadState(`domcontentloaded`),
])

However, with test-runner these events fire before the story started to render, making these typical ways to wait w/ playwright ineffective. Could you please share your guidance on how you plan to overcome this?

  • the bug should be related to the invocation order (rendering happens too late) of the test-runner

For now, we overcome this with page.waitForTimeout(NUM_OF_MS) for a list of stories that depend on the need for waiting

o-alexandrov avatar Mar 16 '23 06:03 o-alexandrov

We have the same problem here. Especially with image loading it can be very tricky to get stable snapshot tests.

webholics avatar Apr 13 '23 13:04 webholics

Same. I'm only using page.waitForLoadState(networkidle) and it's super flaky. It's also explicitly discouraged as an effective method to use in playwright docs.

slashwhatever avatar Apr 28 '23 10:04 slashwhatever

Hey there! We recently introduced a way to completely customize the way the test-runner prepares for running the tests. Can you check the documentation and examples here and see if it suits your needs? Sharing your findings would be great too! Thank you!

yannbf avatar May 01 '23 08:05 yannbf

Same here. The following helps a little, but not in a consistent manner:

    // Make sure assets (images, fonts) are loaded and ready
    await page.waitForLoadState('domcontentloaded')
    await page.waitForLoadState('networkidle');
    await page.waitForFunction(() => !!document.fonts.ready);

joriswitteman avatar Jul 11 '23 09:07 joriswitteman

Would label this as a bug by the way rather than a feature request.

joriswitteman avatar Jul 11 '23 09:07 joriswitteman

I believe I've found a workaround that works consistently for us:

    await page.waitForLoadState('domcontentloaded')
    await page.waitForLoadState('load')
    await page.waitForLoadState('networkidle');
    await page.waitForFunction(() =>
      document.readyState === 'complete'
      && document.fonts.ready
      // naturalWidth will be zero if image file is not yet loaded.
      && ![...document.images].some(({ naturalWidth }) => !naturalWidth)
    );

Let me know if it helps anyone.

joriswitteman avatar Jul 11 '23 11:07 joriswitteman

@joriswitteman it probably wouldn't work in a case when an img tag has loading=lazy.

  • have you tried it in a case when your story is scrollable w/ img lazy loading?

o-alexandrov avatar Jul 11 '23 11:07 o-alexandrov

I have not. Our components probably don't cover enough vertical real estate for that to occur.

But you could amend the code to take lazy loading into account like so:

    await page.waitForLoadState('domcontentloaded')
    await page.waitForLoadState('load')
    await page.waitForLoadState('networkidle');
    await page.waitForFunction(() =>
      document.readyState === 'complete'
      && document.fonts.ready
      // naturalWidth will be zero if image file is not yet loaded.
      && ![...document.images].some(
        ({ naturalWidth, loading }) => !naturalWidth && loading !== 'lazy'
      )
    );

joriswitteman avatar Jul 11 '23 12:07 joriswitteman

would it be possible to export defaultPrepare so that I can call it and then add my own additional awaits? In my case, I need to wait for certain CSS files to be loaded before executing tests, but I don't necessarily want to overwrite all of the original defaultPrepare logic, I just want to extend it

ashleyryan avatar Aug 14 '23 20:08 ashleyryan

Hey everyone! I dug into this issue and it seems like the best solution is to deal with it yourself in the postRender hook.

Let me try to explain:

The test-runner works in the following order:

  1. It runs the setup function. After this, every of the following processes happen to each of your components, in parallel.
  2. It uses the prepare function to open your Storybook URL, then navigates to /iframe.html. This allows it to load Storybook's APIs, but also this means whatever is loaded from your .storybook/preview.ts file, or .storybook/preview-head.html will also execute. So far, no stories have been loaded.
  3. After that, it uses Storybook's APIs to select a story for a component, then waits for it to finish rendering + executing the play function
  4. Upon failure, it reports errors, and upon success, it proceeds to invoke the postRender hook
  5. The postRender hook is invoked, with custom code defined by you. This could be the image snapshot step, for instance.
  6. Execution is done for a single test

So if you have flakiness happening in your stories, you actually want to act on step 5. If you were to add all these waiting utilities to the prepare function, it would only be effective to globally added fonts/stylesheets in .storybook/preview.ts or .storybook/preview-head.html. Therefore, you should improve your own postRender function with something like this:

// .storybook/test-runner.ts
import { waitForPageReady } from '@storybook/test-runner'
import { toMatchImageSnapshot } from 'jest-image-snapshot'

const customSnapshotsDir = `${process.cwd()}/__snapshots__`

export const setup = () => {
  expect.extend({ toMatchImageSnapshot })
}

export const postRender = async (page, context) => {
  await page.waitForLoadState('domcontentloaded');
  await page.waitForLoadState('load');
  await page.waitForLoadState('networkidle');
  await page.evaluate(() => document.fonts.ready);
  // + whatever extra loading you'd like

  const image = await page.screenshot()
  expect(image).toMatchImageSnapshot({
    customSnapshotsDir,
    customSnapshotIdentifier: context.id,
  })
}

I thought of adding that built-in to the test-runner, however this adds a few extra milliseconds of wait per test, which will affect every user that does not do image snapshot testing.

We could potentially export a utility from the test-runner itself that essentially executes those lines, something like

export const waitForPageReady = async (page: Page) => {
  await page.waitForLoadState('domcontentloaded');
  await page.waitForLoadState('load');
  await page.waitForLoadState('networkidle');
  await page.evaluate(() => document.fonts.ready);
}

and then you can use that in the postRender function. I might experiment with that, release a canary and you can test it. How does that sound? Once I have feedback that it actually has helped, then I can merge it into a stable release!

Edit: It's done! You can install the canary:

yarn add --save-dev @storybook/[email protected]

and use it like so:

// .storybook/test-runner.ts
import { waitForPageReady } from '@storybook/test-runner'
import { toMatchImageSnapshot } from 'jest-image-snapshot'

const customSnapshotsDir = `${process.cwd()}/__snapshots__`

export const setup = () => {
  expect.extend({ toMatchImageSnapshot })
}

export const postRender = async (page, context) => {
  // make sure everything is properly loaded before taking an image snapshot
  await waitForPageReady(page)

  const image = await page.screenshot()
  expect(image).toMatchImageSnapshot({
    customSnapshotsDir,
    customSnapshotIdentifier: context.id,
  })
}

yannbf avatar Oct 10 '23 12:10 yannbf

I just tested the waitForPageReady function using the exact example above on a simple button component, but it fails consistently with the following timeout error: "Exceeded timeout of 15000 ms for a test. Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

When I comment out the line await page.waitForLoadState("networkidle");, however, it works.

Any idea why this might be? Is there something else I'm missing?

skaindl avatar Oct 25 '23 14:10 skaindl

Thanks a lot for trying it out @skaindl!

You mention about a simple button component. Can you share a reproduction I can take a look at? It's possible that:

  • There's an ongoing network event that won't resolve
  • There are some javascript errors going on

I'm not sure, I would love to take a look at a reproduction to understand it further. I tested this in a medium sized project with about 80 tests, from simple buttons to complex full page stories that fetch data, and even a story for the entire app with some internal navigation, and it all worked correctly.

yannbf avatar Nov 01 '23 10:11 yannbf

@skaindl Have you got Hot Module Reload running? If I remember correctly, I've worked on SB projects before where that was an issue with network idle.

slashwhatever avatar Nov 01 '23 16:11 slashwhatever

One element that still seems to struggle; HTML elements with a background image don't seem to be caught by networkidle or by checking for images on the page (obvs). Anyone know of a clean way we can detect those load events?

EDIT: Also having issues with google fonts - seems like tiny changes in font rendering between test runs is causing issues that can amount to more than a 1% threshold.

slashwhatever avatar Nov 06 '23 18:11 slashwhatever

'networkidle' never happens if you run npx storybook dev with webpack due to an ongoing __webpack_hmr request

Hypnosphi avatar Mar 11 '24 19:03 Hypnosphi

Adding https://github.com/storybookjs/test-runner/issues/444

shajjhusein avatar Mar 22 '24 22:03 shajjhusein

I believe I've found a workaround that works consistently for us:

    await page.waitForLoadState('domcontentloaded')
    await page.waitForLoadState('load')
    await page.waitForLoadState('networkidle');
    await page.waitForFunction(() =>
      document.readyState === 'complete'
      && document.fonts.ready
      // naturalWidth will be zero if image file is not yet loaded.
      && ![...document.images].some(({ naturalWidth }) => !naturalWidth)
    );

Let me know if it helps anyone.

Works for me, thank you so much! Too bad this isn't included in waitForPageReady or in some other utility function in @storybook/test-runner

idosius avatar Mar 25 '24 10:03 idosius

We use MSW for image requests:

export const XSSnapshot: Story = {
    parameters: {
        cookie: {},
        viewport: {
            defaultViewport: 'xs',
        },
        msw: {
            handlers: [
                rest.get('**/*.svg', async (req, res, ctx) => {
                    const buffer = await fetch(
                        `/fixtures/mock-image-1.png`
                    ).then((response) => response.arrayBuffer())

                    return res(
                        ctx.set('Content-Length', buffer.byteLength.toString()),
                        ctx.set('Content-Type', 'image/png'),
                        ctx.body(buffer)
                    )
                }),
            ],
        },
    },
    render,
    play: async (context) => {
        await xsRendered.play?.(context)
    },
}

Snapshot tests are still flaky - images do/don't load - with this setup:

async postVisit(page, story) {
        // Run snapshot tests for stories with `snapshot` in the name.
        if (story.id.includes('snapshot')) {
            // Awaits for the page to be loaded and available including assets (e.g., fonts)
            await waitForPageReady(page)

            // Generates a snapshot file based on the story identifier
            const image = await page.screenshot({
                animations: 'disabled',
                fullPage: true,
            })

            expect(image).toMatchImageSnapshot({
                customSnapshotsDir,
                customSnapshotIdentifier: story.id,
                failureThreshold: 0.01, // 1% threshold for entire image.
                failureThresholdType: 'percent', // Percent of image or number of pixels.
            })
        }
    },

dacgray avatar Mar 29 '24 03:03 dacgray