Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for network idle on page.goto #50

Open
shilman opened this issue Feb 14, 2022 · 19 comments
Open

Wait for network idle on page.goto #50

shilman opened this issue Feb 14, 2022 · 19 comments
Labels
feature request New feature or request

Comments

@shilman
Copy link
Member

shilman commented Feb 14, 2022

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

@shilman shilman added the feature request New feature or request label Feb 14, 2022
@o-alexandrov
Copy link

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

@webholics
Copy link

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

@slashwhatever
Copy link

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.

@yannbf
Copy link
Member

yannbf commented May 1, 2023

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!

@joriswitteman
Copy link

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
Copy link

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

@joriswitteman
Copy link

joriswitteman commented Jul 11, 2023

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.

@o-alexandrov
Copy link

@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?

@joriswitteman
Copy link

joriswitteman commented Jul 11, 2023

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'
      )
    );

@ashleyryan
Copy link

ashleyryan commented Aug 14, 2023

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

@yannbf
Copy link
Member

yannbf commented Oct 10, 2023

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/test-runner@0.14.0--canary.361.e0c0bcf.0

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,
  })
}

@skaindl
Copy link

skaindl commented Oct 25, 2023

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?

@yannbf
Copy link
Member

yannbf commented Nov 1, 2023

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.

@slashwhatever
Copy link

@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
Copy link

slashwhatever commented Nov 6, 2023

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.

@Hypnosphi
Copy link
Member

Hypnosphi commented Mar 11, 2024

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

@shajjhusein
Copy link

Adding #444

@idosius
Copy link

idosius commented Mar 25, 2024

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

@dacgray
Copy link

dacgray commented Mar 29, 2024

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.
            })
        }
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests