-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Use Puppeteer API to simulate reduced motion in e2e tests #18453
Conversation
@@ -213,7 +212,7 @@ beforeAll( async () => { | |||
|
|||
await trashExistingPosts(); | |||
await setupBrowser(); | |||
await activatePlugin( 'gutenberg-test-plugin-disables-the-css-animations' ); | |||
await page.emulateMediaFeatures( [ { name: 'prefers-reduced-motion', value: 'reduce' } ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How nice 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still waiting for the confirmation from Travis :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should move it to each page load.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this might be required for every page load 🙃 It doesn't work at the moment according to Travis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool usage of new capabilities of Puppeteer, nice one 👍
9f73a0d
to
dd184cd
Compare
For some reason, this is not working as intended, I'll need to debug this a bit later. |
@@ -206,6 +206,7 @@ describe( 'Preview with Custom Fields enabled', () => { | |||
beforeEach( async () => { | |||
await createNewPost(); | |||
await toggleCustomFieldsOption( true ); | |||
await page.emulateMediaFeatures( [ { name: 'prefers-reduced-motion', value: 'reduce' } ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it only take effect in specific test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toggleCustomFieldsOption
called before reloads the page. My thinking was maybe we need this on each page load, but Travis doesn't not agree with me anyway.
I'm not sure yet why this PR is not working as intended.
dd184cd
to
9e62143
Compare
My difficulty here is that I'm having a hard time reproducing the travis issues locally. I tried CPU throttling without success. @aduth since you worked on simulation APIs too, maybe you have an idea about the issues here. |
Are you doing a full run of tests locally, or just specific test files? I wonder if there's something we need to be doing as "clean-up" where we enact the emulation, otherwise risk lingering effects bleeding between tests? |
Ideally, this is enabled for the whole suite of tests though. So even if it lingers, that's the desired behavior. |
At least for how the pull request is currently implemented, it's only targeting specific tests. I'm not saying it should be one way or the other, but I'm wondering if that might explain the differences between what you're seeing locally and in Travis. |
originally, I tried activating this in the setup before each test but I had the same failures, I thought that maybe it's because every time a page is refreshed the simulation resets, so I tried doing it after every page refresh (like the current state), but none of these worked. |
Oh, I missed that it's now in If it's really intended to take effect for the whole test, then I think it should be in the |
Right now, neither of these options work though. I moved it to createNewPage because I assumed that maybe the "page" instance change when we navigate to a new page, forcing us to call the emulation again but this is was a wrong assumption I think, since it doesn't work anyway. So there must be something else we're missing. |
Unfortunately, it's not clear how to fix this, I'm going to close this PR for now. |
I might like to pick this up again at some point. Some rough thoughts off the top of my head, for my future self or whoever looks at it again:
|
This new API in Puppeteer 2.0 removes the need for the disable animations plugin and the env variable we were using.