-
Notifications
You must be signed in to change notification settings - Fork 72
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
[Bug]ReferenceError: __test is not defined #68
Comments
Hey @VictorGlindasPaf thanks for opening this issue! Is that the only failure in your tests, or do previous tests fail too? |
These errors were definitely mixed with other ones, but I'm not sure in which order they came. I'll see if I have time to get some more info about this when back at work on Monday. |
@yannbf The error is flaky and happens occasionally. Yesterday, even in this repository, it happened once when I was running the tests. Reexecuting the tests made the error disappear. It seems to be a kind of timing issue. This happens occasionally on my private Gitlab pipeline as well (flaky). Locally it just happens rarely. |
Interesting! Can we chat more about it? Can you reach out on discord at Yann#9096 or twitter? |
@VictorGlindasPaf I had the same issue. In my case, the issue was a redirection happening in the components by using a decorator, redirecting to an SSO page when a Token was not available in the localstorage. Maybe you have a similar issue and a redirection happens in special cases or you are triggering a page reload. This would explain, why the initial script, which contains the |
I'm having the same problem. The error seems to occur at random. I'm using the test-runner to take snapshots of stories of web components. This is my module.exports = {
async postRender(page, context) {
await page.waitForFunction(
() => {
const inoElements = Array.from(
document.getElementsByTagName('*')
).filter((el) => el.tagName.startsWith('INO'));
return inoElements.every((el) => el.classList.contains('hydrated'));
},
{ polling: 50 }
);
await page.$('#root');
const elementHandler = await page.$('#root');
const innerHTML = await elementHandler.innerHTML();
expect(innerHTML).toMatchSnapshot();
},
}; The introduction of this snippet led to this error. Any ideas why this is happening? |
I can reproduce this by setting |
For me, this is caused when I navigate to a different URL trying to capture snapshots during (The use case for navigating for me, is to put different arg values in the URL and as such capture different permutations of the Story). |
getting same error, following the docs, it says zero config, but do we need extra config? |
I'm also getting this error while testing web components. Now tests will also fail at random, probably because vite takes too long for the test-runner to render components. |
Hey everyone! If you can share a minimal reproduction of this issue, I'd love to take a look at. @AnnaHoege given you have hit this issue recently, could you share a reproduction? Thank you! |
I wish I had a repro to share, but unfortunately I don't. I can only add another anecdote that this happens randomly when using storybook 7 and vite. I get this error in the CI every now and then when making arbitrary changes, and every time it fails I can make the same build pass by simply retrying in the CI without making any code changes. Feels like something is flaky inside of the storybook + vite integration. |
I have an optimistic update: I seem to have found a consistent (but unforunately not minimal) repro. I seem to get this error consistently for .stories files that are using the play function. In the code snippet below, if I keep the
import { expect } from "@storybook/jest";
import type { Meta, StoryObj } from "@storybook/react";
import { userEvent, within } from "@storybook/testing-library";
import { useState } from "react";
import { Modal } from "./Modal";
export default {
title: "atoms/Modal",
component: Modal,
tags: ["autodocs"],
} satisfies Meta<typeof Modal>;
export const Default: StoryObj<Meta<typeof Modal>> = {
args: { children: "Hello World!", open: true },
};
export const CanOnlySeeContentWhenOpen: StoryObj<Meta<typeof Modal>> = {
args: { children: "Hello World!" },
render() {
const [open, setOpen] = useState(false);
return (
<>
<Modal open={open}>Hello World</Modal>
<button onClick={() => setOpen(true)}>Show</button>
</>
);
},
async play({ canvasElement }) {
const canvas = within(canvasElement);
expect(canvas.getByText("Hello World")).not.toBeVisible();
await userEvent.click(canvas.getByRole("button"));
expect(canvas.getByText("Hello World")).toBeVisible();
},
}; Here's my stack trace in case it helps:
My CI is github actions using ubuntu-latest |
Hey @ksandin! Given the error message:
Are your stories doing any navigation on click? Navigations break the test-runner. |
@yannbf I have no navigations in the code that is being tested. As to why that warning appears, My guess is that something under the hood in storybook is causing a navigation. |
Any update on this issue? Unfortunately, we just added storybook testing to our pipeline and ran into this issue multiple times today. I can't say much about a repro, as the issue only started when to started running the tests automatically - and we haven't written many tests yet. We are using storybook version 7.5.3 and test-runner 0.14. |
Hey everyone, it's really difficult to fix an issue when there is no reproduction available. I have been trying to reproduce this but without any success of. We use the test-runner daily in about 40 different projects, and so far have not seen this error. I recall this happening in very early versions of the test-runner, but not anymore. Please do share a reproduction if you have one, I'd be incredibly thankful for that! |
I can reproduce this when using the workaround in mswjs/msw-storybook-addon#82 // Yuck...
Loading.decorators = [
(Story) => {
useEffect(() => {
return () => window.location.reload();
}, []);
return <Story />;
},
]; |
FYI: this happened to me when storybook was running in another terminal... |
Looking at the source code, this smells like a race condition to me. The test-runner/src/playwright/transformPlaywright.ts Lines 28 to 30 in 6624c5e
And the test-runner/src/setup-page-script.ts Line 266 in 6624c5e
Which in turn is loaded in Lines 64 to 75 in 6624c5e
Could the explanation be that there is a race between the invocation and loading of the setup page? |
This issue appears randomly for us also. |
It seems the race condition is resolved by running that initialization script not with diff --git a/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.js b/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.js
index f6b3bdf..328350c 100644
--- a/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.js
+++ b/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.js
@@ -57,7 +57,7 @@ const testPrefixer = (0, import_template.default)(`
await globalThis.__sbPreRender(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
});
diff --git a/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.mjs b/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.mjs
index 08d0006..dfaec94 100644
--- a/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.mjs
+++ b/node_modules/@storybook/test-runner/dist/playwright/transformPlaywright.mjs
@@ -25,7 +25,7 @@ const testPrefixer = template(`
await globalThis.__sbPreRender(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
}); |
Hey there @throw5way thanks for the suggestion! I made a canary with the fix at #445, please try it out and give some feedback if you can. Thanks!
|
@yannbf that fixes my issue! I have been scratching my head over this for a week. |
for us, after upgrading the the canary version, all test pass even though they should be failing. also we are trying todo some individual retries to deflake test runs. We start to see this error after we add the
|
Hey @hans-lizihan could you please share a reproduction? I'd like to take a deeper look if possible. For the others, could you please let me know if the canary solves your issues? |
Hey @yannbf The canary build solves our issues as reproduced by #68 (comment) |
Glad to hear that @blcoyote! It seems that it causes issues when the test-runner is used for visual regression testing though. I will have to take a deeper look into that. In the meantime, everyone else, please give the canary a try and give me feedback if possible. Thanks! |
I notice this still occurs on 0.18.1. Is the canary change included in 0.18? And I'm running visual snapshot testing. |
Hey @unional it is not, it's only in that canary version so far. I need more feedback whether it introduces any issues. Is the canary working without issues for you? |
been using it for a couple of weeks and it is working with no issues for me |
@yannbf We have a large private repo, |
just tested this I'm also getting false positives 😟 |
FYI just hit this issue. Its been tricky to repro, because it only happens for us when using Vite and the dependency optimization step has not run. Only in Storybook 8 in our case (even though it looks like this issue predates it by a fair bit). Eg, setting the vite config to have:
Otherwise, first run fails, second run pass. But using a production build of storybook to test against bypasses this optimize dep step, so the tests always work. Not a great solution when running locally, though. I added code in a preVisit step to wait until the setup-page-script functions are available on the global object. In the event of dep optimization, they NEVER become available (but on second run they are). It's really strange. |
I encountered this issue at 0.18.2 and got a workaround with this patch-package file.
diff --git a/node_modules/@storybook/test-runner/dist/index.js b/node_modules/@storybook/test-runner/dist/index.js
index e1bc5a2..1aba18e 100644
--- a/node_modules/@storybook/test-runner/dist/index.js
+++ b/node_modules/@storybook/test-runner/dist/index.js
@@ -13556,7 +13556,7 @@ var testPrefixer = /* @__PURE__ */ __name((context) => {
await globalThis.__sbPreVisit(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
});
diff --git a/node_modules/@storybook/test-runner/dist/index.mjs b/node_modules/@storybook/test-runner/dist/index.mjs
index c32e886..b8ca561 100644
--- a/node_modules/@storybook/test-runner/dist/index.mjs
+++ b/node_modules/@storybook/test-runner/dist/index.mjs
@@ -13539,7 +13539,7 @@ var testPrefixer = /* @__PURE__ */ __name((context) => {
await globalThis.__sbPreVisit(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
});
diff --git a/node_modules/@storybook/test-runner/dist/test-storybook.js b/node_modules/@storybook/test-runner/dist/test-storybook.js
index 32bbbd8..c59380a 100755
--- a/node_modules/@storybook/test-runner/dist/test-storybook.js
+++ b/node_modules/@storybook/test-runner/dist/test-storybook.js
@@ -17621,7 +17621,7 @@ var testPrefixer = /* @__PURE__ */ __name((context) => {
await globalThis.__sbPreVisit(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
});
diff --git a/node_modules/@storybook/test-runner/dist/test-storybook.mjs b/node_modules/@storybook/test-runner/dist/test-storybook.mjs
index da7fd85..0ac585b 100755
--- a/node_modules/@storybook/test-runner/dist/test-storybook.mjs
+++ b/node_modules/@storybook/test-runner/dist/test-storybook.mjs
@@ -17627,7 +17627,7 @@ var testPrefixer = /* @__PURE__ */ __name((context) => {
await globalThis.__sbPreVisit(page, context);
}
- const result = await page.evaluate(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
+ const result = await page.addInitScript(({ id, hasPlayFn }) => __test(id, hasPlayFn), {
id: %%id%%,
}); |
[TEST] In my msal.ts when I add the below code a group of test cases break with the above error |
This solution works for me. However, not if I'm running |
I was able to solve my error. The error was in errorredirectpromise which was causing a redirect. I just had to call it before msalinstance was instance was initialized and check for any pending promises so it did not cause an redirect. Which fixed my issue. |
@yannbf Hello there! The problem with __test is not defined still exist in 0.19.0 and 0.19.1 during Github Actions. I've tried to install your canary, but it provides me a lot of errors with coverage report. Files are tested correctly but for coverage report receives files like mocks or preview.js. There is a way to fix it? When I back to f.e. 0.19.0 with no canary, coverage files looks good, but I receive _test error again. |
I solved the issue on my end. The reasons were bad decisions made by previous developers. They did reloads to get rid of cache. I made proper redux query invalidators in decorators i preview.tsx instead, so reloads werent neccessay. |
the release 0.17.1--canary.445.4041ab4.0 from this PR fixes this issue for me. It'd be nice to see this fix in main releases |
Same issue here on
For us, it only happens in the first test that runs. It does not have any navigation in the test itself. I tried applying the patch directly from #68 (comment) or installing the canary version. |
@valentinpalkovic Thanks for the hint, turned out that we used a window.location.reload in a decorator function which resulted in |
I can confirm that the attempted patch causes all tests to pass, even those that should fail. From a quick reading of the code, that canary version tries to make the test function run earlier, which is the opposite of what you would want; you want the setup function to run earlier, and/or the actual test to run later. It looks to me like if you have too many stories, you will inevitably start seeing tests fail with "An error occurred in the following story, most likely because of a navigation." It looks like a lot of people who have been reporting this problem have large repos with a lot of stories. At work, where we have a lot of stories, we have been hitting this error (and some random timeouts, and occasionally the I have a repo here that reliably reproduces these errors for me; it has a simple test page and 400 copies of that test page's story. In this repo, I reliably get "An error occurred in the following story, most likely because of a navigation" when running tests. (There is definitely no navigation taking place in the stories.) This is kind of a contrived example, but like I said, I (and presumably other people in this thread) have been hitting the exact same problems in real life. In general, I don't see any real reason why there should be a limit for the number of stories the test runner should be able to support; it should be possible to run each of the tests more or less in isolation, so if you can run each test, you should be able to run them all (it's not like there's any shared state between them.) I notice that the memory and threads used by the test runner reliably increase as the test runner progresses in a large project. It seems likely to me that the underlying cause of the issue is some kind of memory leak and resource starvation that's taking place; probably the execution context for the tests is being destroyed because the underlying browser is killing tabs that take too much memory, or something along those lines. |
We are mitigating this issue by running Storybook tests in batches. We have a script that runs the storybooks whose names start with A-D, then E-M, then N-Z. If you're using a Unix shell, you can use FILES=$(find src -type f -regex ".*\/[A-D][A-Za-z0-9_-]*\.stories\.[tj]sx$") Then just run the test runner with those names after it: npx test-storybook $FILES This makes running tests and CI/CD slightly more complicated, but it eliminates the issue. |
FYI, while big project with a lot of stories are more likely to trigger it, it also happens to small project. I have a project with < 100 stories in a monorepo, and I also run into this when I run storybook against it. |
@vanessayuenn @yannbf seems like a good thing to fix if we can squeeze it in. WDYT? |
I seem to get this error seemingly at random when running the tests.
Following the stack trace just takes me to the very bottom of my story file.
The text was updated successfully, but these errors were encountered: