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

Local visual regression test time out #47988

Closed
ciampo opened this issue Feb 11, 2023 · 7 comments · Fixed by #47991
Closed

Local visual regression test time out #47988

ciampo opened this issue Feb 11, 2023 · 7 comments · Fixed by #47991
Assignees
Labels
[Package] E2E Tests /packages/e2e-tests [Tool] E2E Test Utils /packages/e2e-test-utils [Type] Bug An existing feature does not function as intended

Comments

@ciampo
Copy link
Contributor

ciampo commented Feb 11, 2023

How to reproduce

  • npm run distclean && npm ci
  • In two separate tabs:
    • First run npm run storybook:e2e:dev
    • Once storybook has launched, in a separate tab run npm run test:e2e:storybook -- --update-snapshots

What should happen

The snapshots are generated correctly in a few seconds

What is currently happening

The tests time out and the snapshots are not generated

How to fix it

I traced the regression back to PR #46459

Removing the auto: true option from this file fixes the issue:

{ scope: 'worker', auto: true },

PR #46459 assumed that all Playwright e2e tests in the repository load the editor, but that assumption is not correct for visual regression tests

@ciampo ciampo added [Type] Bug An existing feature does not function as intended [Tool] E2E Test Utils /packages/e2e-test-utils [Package] E2E Tests /packages/e2e-tests labels Feb 11, 2023
@ciampo
Copy link
Contributor Author

ciampo commented Feb 11, 2023

@WunderBart @kevin940726 and @t-hamano , could you help with this one?

I'm not as well versed as you may be in Playwright.

Tests in test/storybook-playwright/specs (which import { test } from '@wordpress/e2e-test-utils-playwright') are currently timing out — is there any way we can refactor the code so that they can opt out from the code that tries to initialise the editor?

Thank you!

@ciampo
Copy link
Contributor Author

ciampo commented Feb 11, 2023

@bph we may want to delay advertising local visual regression testing until this issue isn't ironed out

@ciampo ciampo changed the title Local visual regression tests not working Local visual regression test time out Feb 11, 2023
@t-hamano
Copy link
Contributor

I think @wordpress/e2e-test-utils-playwright is E2E utility for WordPress. Can we solve this problem by importing directly from Playwright as follows? However, I'm not sure if this is the appropriate method, since the snapshot file name would change.

diff --git a/test/storybook-playwright/specs/font-size-picker.spec.ts b/test/storybook-playwright/specs/font-size-picker.spec.ts
index 6aa11e8926..c2f6fc3580 100644
--- a/test/storybook-playwright/specs/font-size-picker.spec.ts
+++ b/test/storybook-playwright/specs/font-size-picker.spec.ts
@@ -1,7 +1,7 @@
 /**
- * WordPress dependencies
+ * External dependencies
  */
-import { test, expect } from '@wordpress/e2e-test-utils-playwright';
+import { test, expect } from '@playwright/test';

Also, this may be an issue in my environment, but the Storybook now only shows components that include E2E testing 🤔

storybook

@ciampo
Copy link
Contributor Author

ciampo commented Feb 11, 2023

Can we solve this problem by importing directly from Playwright as follows?

That seems to solve it! I'm not sure if there are any implications, but if we ever encounter any I guess we can create a specific version of test and expect for Storybook visual regression testing

the snapshot file name would change.

This shouldn't be too bad, since VizReg for now is only supposed to be used locally (the snapshots should be gitignored)

Also, this may be an issue in my environment, but the Storybook now only shows components that include E2E testing 🤔

That is expected, so that only e2e-focused stories are compiled and served :) You can read here for more info

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Feb 11, 2023
@kevin940726
Copy link
Member

Oops, I didn't know that e2e-test-utils-playwright is used by storybook visual testing.

I agree with @t-hamano here though, the package is designed to work with a WordPress instance, we want to keep it opinionated for that specific use case. You can always just use the official @playwright/test for anything else though, expect is just a re-export, and test is extended in the test.ts file. You can go there and copy any part that's relevant to your setup to create your own test function. Most of the utils are re-exported as well, so you can also do new Editor( { page } ) for instance to create an editor POM.

@ciampo
Copy link
Contributor Author

ciampo commented Feb 12, 2023

Oops, I didn't know that e2e-test-utils-playwright is used by storybook visual testing.

It's a relatively recent addition that we did to help testing some changes in the components package, still a bit rough around the edges (and for now working only locally).

Awesome, thank you both for the extra context 🙏

@ciampo
Copy link
Contributor Author

ciampo commented Feb 12, 2023

we may want to delay advertising local visual regression testing until this issue isn't ironed out

@bph All solved, feel free to ignore this message :)

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] E2E Tests /packages/e2e-tests [Tool] E2E Test Utils /packages/e2e-test-utils [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants