E2E Tests: Adding helpers to install/activate/deactivate and remove plugins #5906

youknowriad · 2018-03-30T13:48:00Z

In order to test extensibility APIs (meta boxes, templates etc...) we need a way to install and uninstall plugins.

This PR adds these helpers. I'm planning to use these in #5902

Testing instructions

You can add something like this to any e2e test

import { installPlugin, uninstallPlugin, activatePlugin, deactivatePlugin } from '../support/plugins';
await installPlugin( 'edit-flow', 'Edit Flow' );
await activatePlugin( 'edit-flow' );
await deactivatePlugin( 'edit-flow' );
await uninstallPlugin( 'edit-flow' );

…lugins

youknowriad · 2018-03-30T13:49:27Z

test/e2e/support/utils.js

+ *
+ * @return {Promise}        Promise.
+ */
+export async function waitForRequests( minDelay = 1000, maxDelay = 20000 ) {


This function is exactly the reason I loved Cypress. This was handled automatically. Can't find the equivalent in Puppeteer. This is trying to be as reliable as I can.

While not intuitively named, I wonder if this is equivalent:

page.waitForNavigation( { waitUntil: 'networkidle0' } );

https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#pagewaitfornavigationoptions

(There are various methods which have networkidle0 as well)

Alternatively, instead of waiting for requests, you could wait for the DOM to be in a state you expect once the request completes:

https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#pagewaitforselectorselector-options

If anything, this highlights that Cypress gives us crutches to poor ways of expressing intent 😄

If anything, this highlights that Cypress gives us crutches to poor ways of expressing intent

I don't agree with this, the idea in Cypress is when you do an action (click, move, anything) you wait for all the side effects to happen before continuing. I think it's great in terms of e2e maintainability.

page.waitForNavigation( { waitUntil: 'networkidle0' } );

This is not the same thing, this expects a page load and then waits for the network.

Alternatively, instead of waiting for requests, you could wait for the DOM to be in a state you expect once the request completes:

Yes, but this is not generic enough for me, we'd have to recreate the same thing over and over again because the UI change is not the same and also, sometimes there's no UI change, just a request that you need to wait for before quitting a page for instance which will cut the request.

I'm fine updating the PR to use selectors but I still disagree with you. Why waitForNavigation exist in the first place, you could also replace it with waiting for UI

What do you mean "waiting for UI" ? I am fine (enough) with methods like waitForSelector.

I was thinking: what's the difference between . page.waitForNavivation and waitForRequests there's none IMO, both are helpers to avoid the hassle of testing Async stuff.

Why would we be fine with waitForNavigation and not waitForRequests since both could be replaced with waitForSelector

It depends in what context you mean to use waitForNavigation.

If you mean for here, to wait for XHR requests to complete, then I would agree, it's no different and is not a good tool to use.

If you mean to its intended behavior to wait for a page to load, then I think it's slightly different since for a full page load, the user does use more of a judgment of network activity and page readiness to gauge when to continue (screen turns white, then loads). None of these are perfect, sure, and I suppose this is why Puppeteer provides options for waitUntil, including load, domcontentloaded, networkidle0, and networkidle2.

But I do agree it's only slightly different. In reality, while page load process plays some role, in the end the user ultimately waits for the thing they expect to see on the page (waitForSelector).

I updated using waitForSelector let me know what you think :)

danielbachhuber · 2018-03-30T13:53:50Z

In order to test extensibility APIs (meta boxes, templates etc...) we need a way to install and uninstall plugins.

For the purposes of testing extensibility APIs, I still think we should create isolated equivalent plugins housed in our own test suite instead of installing seemingly random plugins from WordPress.org. Ultimately, the challenge is that one of aforementioned seemingly random plugins could change anything at any point and cause the Gutenberg test suite to fail unexpectedly.

youknowriad · 2018-03-30T13:56:30Z

We still need APIs to test installing plugins and we need APIs to activate/deactivate plugins (local or remote) so I I think this PR is still valuable.

danielbachhuber · 2018-03-30T14:02:29Z

We still need APIs to test installing plugins and we need APIs to activate/deactivate plugins (local or remote) so I I think this PR is still valuable.

Fair enough.

aduth

Beautiful 👍

aduth · 2018-04-03T01:30:41Z

test/e2e/support/plugins.js

+ */
+import { visitAdmin } from './utils';
+
+export async function installPlugin( name, searchTerm ) {


What would be the purpose of the second argument here? Not clear by the included documentation 😉

aduth · 2018-04-03T01:32:22Z

test/e2e/support/plugins.js

+	} );
+	await Promise.all( [
+		page.click( 'tr[data-slug="' + name + '"] .delete a' ),
+		confirmPromise,


Not sure if it'll suffer the same race conditions as waitForNavigation, but in that case, the suggested workaround has the "waiting" (event handler binding) arranged to take place before the click itself: https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#pageclickselector-options

gziolo · 2018-04-03T07:47:24Z

Nice 👍 It might be tricky to ensure that plugin gets properly uninstalled when the test fails because of random issues, but in general this will open a few interesting possibilities. 💯

E2E Tests: Adding helpers to install/activate/deactivate and remove p…

b537846

…lugins

youknowriad added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Mar 30, 2018

youknowriad self-assigned this Mar 30, 2018

youknowriad requested review from danielbachhuber and gziolo March 30, 2018 13:48

youknowriad commented Mar 30, 2018

View reviewed changes

danielbachhuber mentioned this pull request Mar 30, 2018

Add e2e tests for "MetaBoxes: Fix meta boxes toggling" #5909

Closed

3 tasks

Replace waitForRequest by page.waitForSelector usage

974245c

youknowriad mentioned this pull request Apr 2, 2018

E2E Tests: Adding a test to check CPT templates initialization. #5929

Merged

danielbachhuber approved these changes Apr 2, 2018

View reviewed changes

aduth approved these changes Apr 3, 2018

View reviewed changes

E2E: Document the plugin helpers

e53297e

youknowriad merged commit 096ecec into master Apr 3, 2018

youknowriad deleted the add/e2e-plugins-helpers branch April 3, 2018 07:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2E Tests: Adding helpers to install/activate/deactivate and remove plugins #5906

E2E Tests: Adding helpers to install/activate/deactivate and remove plugins #5906

youknowriad commented Mar 30, 2018

youknowriad Mar 30, 2018

aduth Mar 30, 2018

aduth Mar 30, 2018

aduth Mar 30, 2018

youknowriad Mar 30, 2018

youknowriad Mar 30, 2018

aduth Mar 30, 2018

youknowriad Mar 30, 2018

aduth Mar 30, 2018

youknowriad Apr 2, 2018

danielbachhuber commented Mar 30, 2018

youknowriad commented Mar 30, 2018

danielbachhuber commented Mar 30, 2018

aduth left a comment

aduth Apr 3, 2018

aduth Apr 3, 2018

gziolo commented Apr 3, 2018

E2E Tests: Adding helpers to install/activate/deactivate and remove plugins #5906

E2E Tests: Adding helpers to install/activate/deactivate and remove plugins #5906

Conversation

youknowriad commented Mar 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielbachhuber commented Mar 30, 2018

youknowriad commented Mar 30, 2018

danielbachhuber commented Mar 30, 2018

aduth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Apr 3, 2018