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

feat: ability to generate multiple sets of autotests #28

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

KuznetsovRoman
Copy link
Member

@KuznetsovRoman KuznetsovRoman commented Nov 6, 2024

What is done

  • Added autoScreenshotStorybookGlobals option (can be specified in storyfile and in plugin config).

It can be specified in order to create multiple sets of autoscreenshot tests with different storybook global values

For example, with autoScreenshotStorybookGlobals is set to:

{
  "default": {},
  "light theme": {
    "theme": "light"
  },
  "dark theme": {
    "theme": "dark"
  }
}

there would be 3 screenshot tests for each story, with each having corresponding storybook globals value:

  • ... Autoscreenshot default
  • ... Autoscreenshot light theme
  • ... Autoscreenshot dark theme

  • Increased error readability in rare case when opened url is not storybook url. Now it shows an error with following message: "Couldn't find storybook channel. Looks like the opened page is not storybook preview"

  • Added instruction to ignore "couldn't read test file" warning in the warning itself (can be done by setting "TESTPLANE_STORYBOOK_DISABLE_STORY_REQUIRE_WARNING" env var, was implemented a while ago)

@KuznetsovRoman KuznetsovRoman force-pushed the TESTPLANE-320.autoscreenshot_globals branch 2 times, most recently from 6364eca to 05b5365 Compare November 6, 2024 15:41
Comment on lines +40 to +45
const result = await openStoryStep(ctx.browser, story, globals);

await autoScreenshotStep(ctx.browser, story.autoscreenshotSelector || result.rootSelector);
});
await autoScreenshotStep(ctx.browser, story.autoscreenshotSelector || result.rootSelector);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont want to separate "update globals" to different step because it is extra http request and we would need to re-render the story itself. Because of this, globals are updated right before story setting, so we would have to do this only once.

Comment on lines 25 to 26
? story.autoScreenshotStorybookGlobals
: autoScreenshotStorybookGlobals;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picking "which autoScreenshotStorybookGlobals to use" betweenautoScreenshotStorybookGlobals from testplane.config.ts and autoScreenshotStorybookGlobals from storyfile, preferring the storyfile one.

If storyfile globals are defined, using it without merging with testplane.config.ts version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we merge them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we might encounter situation, when user does not need to test some component with multiple themes.

On the other hand, if this case will happen, we could bring another PR and make story.autoScreenshotStorybookGlobals to be type of either Record, either function, where config defaults are passed as first argument, so user will be able to do it.

So, i will merge them

done: (result: string) => void,
) => void;
__TESTPLANE_STORYBOOK_INITIAL_GLOBALS__?: Record<string, unknown>;
__STORYBOOK_PREVIEW__?: { storeInitializationPromise?: Promise<void> };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storybook preview. Here is storeInitializationPromise is stored. We can't emit "updateGlobals" until storybook preview is initialized

remountOnly: boolean,
done: (result: string) => void,
) => void;
__TESTPLANE_STORYBOOK_INITIAL_GLOBALS__?: Record<string, unknown>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storybook does not save initial (default) globals, so we have to do it ourselves

Comment on lines +179 to +192
const storybookPreview = (window as StorybookWindow).__STORYBOOK_PREVIEW__;
const isStorybookPreviewAvailable = storybookPreview && storybookPreview.storeInitializationPromise;
const shouldUpdateStorybookGlobals = storybookGlobals && isStorybookPreviewAvailable;

if (shouldUpdateStorybookGlobals) {
(storybookPreview.storeInitializationPromise as Promise<void>).then(function () {
let defaultGlobals = (window as StorybookWindow).__TESTPLANE_STORYBOOK_INITIAL_GLOBALS__;

if (!defaultGlobals) {
const setGlobalCalls = (window as StorybookWindow).__STORYBOOK_ADDONS_CHANNEL__.data?.setGlobals;
const initValue = (setGlobalCalls && setGlobalCalls[0].globals) || {};

defaultGlobals = (window as StorybookWindow).__TESTPLANE_STORYBOOK_INITIAL_GLOBALS__ = initValue;
}
Copy link
Member Author

@KuznetsovRoman KuznetsovRoman Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving first setGlobal call in order to handle these cases:

  • test1 sets globals {a: "b"}
  • test2 sets globals {c: "d"}

In this scenario, test2 has globals {a:"b"} already set and he only updates {c:"d"}. So, in order to make tests independent from order, we are merging default globals with user-called globals

In other tests in this session we can reuse these default globals from "TESTPLANE_STORYBOOK_INITIAL_GLOBALS" as long as we don't reload the page. And in case if we do, we could pull initial globals from setGlobalCalls

Comment on lines 173 to 177
if (!channel) {
result.loadError = "Couldn't find storybook channel. Looks like the opened page is not storybook preview";

if (shouldRemount) {
channel.emit("setCurrentStory", { storyId: "" });
doneJson(result);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is thrown if loaded url is not storybook url (for example, if our baseUrl was invalidly changed by some other plugin)

channel.emit("setCurrentStory", { storyId });
const storybookPreview = (window as StorybookWindow).__STORYBOOK_PREVIEW__;
const isStorybookPreviewAvailable = storybookPreview && storybookPreview.storeInitializationPromise;
const shouldUpdateStorybookGlobals = storybookGlobals && isStorybookPreviewAvailable;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if storybookGlobals is empty, we should updateGlobals, because previous test might set something, and we need to reset it

Comment on lines 25 to 26
? story.autoScreenshotStorybookGlobals
: autoScreenshotStorybookGlobals;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we merge them?

@KuznetsovRoman KuznetsovRoman force-pushed the TESTPLANE-320.autoscreenshot_globals branch 5 times, most recently from a5dd1d0 to 432ac7a Compare November 7, 2024 21:24
@KuznetsovRoman KuznetsovRoman force-pushed the TESTPLANE-320.autoscreenshot_globals branch from 432ac7a to 63eb93c Compare November 7, 2024 21:28
@KuznetsovRoman KuznetsovRoman merged commit c1359eb into master Nov 7, 2024
2 checks passed
@KuznetsovRoman KuznetsovRoman deleted the TESTPLANE-320.autoscreenshot_globals branch November 7, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants