e2e test with selenium + browserstack for legacy browsers like IE. #21528

sainthkh · 2020-04-10T01:34:01Z

Closes Testing: Include IE11 "Canary" test in Travis build #16509 ?

Description

Experimental implementation of e2e testing with selenium + browserstack for legacy browsers like Internet Explorer 11.
I tested the package setup by writing e2e test code for IE11: browser crashes on pressing Enter to switch to edit mode #21276. I know it's more suitable for a separate PR. But I want to be sure if it's a worthwhile attempt.

How has this been tested?

Turn on WordPress dev environment by npx wp-env start.
Build plugin by npm run dev or npm run build.
Go to packages/e2e-selenium folder.
Copy .env.template file and rename it to .env. Fill the content of .env. You can get the name and key by signing up browserstack. If you have any problem signing up or don't want to sign up for this, I'll share my username and key. Contact sainthkh at WordPress Slack.
Run ../../node_modules/.bin/jest --config=jest.config.js.

After a few minutes, you can see the message like below:

Then, you can watch the result by visiting the link. (Login is not required.)

Screenshots

Check above.

Types of changes

New testing feature.

Things to discuss.

1. How to show the result link.

Currently, I used console.log() to log it. But it's against the Gutenberg convention. process.stdout.write() is overwritten by the jest result message. It cannot be used. I also used process.stdout.on('data') and process.on('exit'), but they didn't work.

My last option is to use child_process.fork() and IPC. But I hope there is simpler answer than this. If there's no other way than this, I need to use it.

2. Moving the selenium+browserstack configuartion to packages/scripts?

I'm not sure this setup is useful for WordPress developers.

3. How to organize tests.

I created a folder, ie, under test folder. And create a test file with an issue number. It's because it takes really long to finish a test (10-20s a test or even longer). By breaking down a test file per issue, we can test them separately.

Downside: It's hard to see what's going on from the name of the file. But I guess issue page contains more info than the file name. So, it don't think it's important.

4. Setting up BrowserStack and Travis CI.

To make this work with CI, we need an official BrowserStack account for Gutenberg team and set up environment variables.

I created CI_BROWSERSTACK_USER, CI_BROWSERSTACK_KEY for them.

Checklist:

My code is tested.
My code follows the WordPress code style.
[N/A] My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
[N/A] I've updated all React Native files affected by any refactorings/renamings in this PR.

aduth · 2020-04-10T19:06:21Z

To make this work with CI, we need an official BrowserStack account for Gutenberg team and set up environment variables.

Have you considered feasibility of this, from the perspective of pricing?

Do you assume we can apply for free open-source testing ? Mentioned also in #16509 (comment) . If we need to apply and it's a reasonable-enough process, we should probably get this sorted .

I see most value in having this as part of the Travis build. It's good if people can run it locally, but I don't think we can reliably assume people are going to that.

Edit: I've started the process to apply for an open source testing account.

package.json

packages/e2e-selenium/.env.template

packages/e2e-selenium/index.d.ts

package.json

aduth · 2020-04-10T19:33:50Z

I submitted an application for an open source testing account through BrowserStack, and was approved rather quickly. In considering how we'd want to implement this into our continuous integration, I'm especially conscious of how best to be careful with the credential keys.

We can encrypt variables which, as noted in the documentation, would not be available to pull requests from forked repositories.

If we're expecting these test runs to be quite slow, and they're not urgent in the sense of needing to block individual pull requests, we might be content to allow these to run only on commits to master, not on pull requests. This would address both (a) not blocking pull requests by poor performance of these tests and (b) avoid worrying about encrypted variables only being available in "trusted" pull requests (assuming that all master commits are "trusted").

sainthkh · 2020-04-13T04:34:42Z

As WordPress is huge, I was quite sure that BrowserStack will grant a free open source account. (I guess most bosses and markerters want WordPress logo on their "trusted by" list.) But I couldn't believe that they also gave me one when I applied with my cloned Gutenberg repo.

I totally agree with running these tests on master branch only. Actually, I was wondering how we should add other edge cases with other browsers like #20507 (safari), #21031(firefox + macOS). But by adding them on master branch only, we can group them with a name like "Travis CI - Browser issues" and add a job for each environment. (It's not considered in this PR.)

p.s. I cannot understand why lint fails with a mysterious message, 'Cannot read property 'toUpperCase' of undefined'. It passes on the local Windows 10.

sainthkh · 2020-04-13T04:37:08Z

packages/e2e-selenium/README.md

@@ -0,0 +1,13 @@
+# E2E Selenium
+
+End-To-End (E2E) tests for WordPress written for legacy browsers.


I guess legacy is a bit wrong here, this package might test edge cases in safari, firefox, or edge. Right?

I guess legacy is a bit wrong here, this package might test edge cases in safari, firefox, or edge. Right?

Related to #21528 (comment) , I think it might depend on the scope of the issues we want to cover here. If we don't think we'll need it to be used for Firefox and Safari, then I think it's quite reasonable to document this as targeted toward legacy browsers. In fact, I think it could be preferable to further clarify the limited scope this package is intended to serve.

I would be perfectly happy to have these tests running on IE only. None of the other browsers have caused (or are likely to cause) anywhere near as much trouble as IE.

chrisvanpatten · 2020-04-13T04:43:25Z

What would be the impact of this on the Puppeteer-based e2e tests and tooling in the short and long term?

sainthkh · 2020-04-13T04:45:34Z

@chrisvanpatten It's a new toolset. It doesn't affect any Puppeteer-based e2e tests.

It's created for some edge cases that cannot be tested with Puppeteer like IE11, safari, etc.

aduth · 2020-04-13T13:02:01Z

I totally agree with running these tests on master branch only. Actually, I was wondering how we should add other edge cases with other browsers like #20507 (safari), #21031(firefox + macOS). But by adding them on master branch only, we can group them with a name like "Travis CI - Browser issues" and add a job for each environment. (It's not considered in this PR.)

I like the line of thinking here. I'm especially conscious of the potential developer experience impact in bringing a second set of end-to-end testing tools, in addition the current Puppeteer workflow. I think we'd want to be very explicit in outlining the scope of this package as intending to cover the limited set of cases that we can't cover through those tools.

But I'd also want to highlight Playwright, which is being developed by many of the original developers of Puppeteer, and which aims to increase coverage to additional browsers (including Firefox and Safari). It'd been discussed in one of the previous JavaScript meetings, and it might be something to keep an eye on and consider as a replacement for Puppeteer at some point in the future.

So to try to "future-proof" the introduction of this module in light of potential future changes, I consider:

Internet Explorer might still be the primary target?
We could frame it around Browserstack and their automation APIs
- I'm also not opposed to keeping it as it is, as far as naming around "Selenium"
- I guess it depends whether we consider Browserstack or Selenium as being merely an implementation detail

I'm also trying to consider its relationship amongst e2e-tests, e2e-test-utils, scripts. As proposed, this is a combination of the runner configuration, the test cases themselves, and some utilities. I think I could be okay with this along the line of thinking that this should be relatively focused in its purpose and thus self-contained. I might wonder if the naming captures all of this. I'm not sure if I have any good alternatives. Maybe e2e-tests-selenium (clarify that tests are included), e2e-tests-ie (deemphasize Selenium, focus on IE), e2e-tests-legacy (focus on legacy browsers). I think the current naming could be fine as well.

aduth · 2020-04-13T13:26:05Z

p.s. I cannot understand why lint fails with a mysterious message, 'Cannot read property 'toUpperCase' of undefined'. It passes on the local Windows 10.

Puzzling indeed. I can try to check and see if I can reproduce it locally. Based on how the error is presented, it seems like it's an issue with one of the linting tools itself, not necessarily a problem in your code (i.e. not a reported lint issue, but an error occurring in the process of performing the lint).

aduth · 2020-04-13T13:34:33Z

packages/e2e-selenium/tsconfig.json

+		// commented out because src folder doesn't exist in this package.
+		//"rootDir": "src",


There was some prior discussion at #18942 (comment) about folder structures for packages which aren't transpiled, using lib as a substitute for src. I believe this package in its current state is not prone to the same problems which were aimed to be addressed in having a lib folder (files in the root of the project), but it would be good to have the consistency all the same.

But this breaks compressed size action. Should we roll back?

I decided to use rootDirs.

aduth · 2020-04-13T13:50:32Z

I've added this as a topic to be discussed during tomorrow's JavaScript meeting. I think it'll be good to get some extra attention on this 👍

sainthkh · 2020-04-14T02:07:50Z

1.

I didn't know that playwright support safari. But it does. Then, it seems that focusing on IE is the better choice. When I was writing it, I thought there was no library that supports e2e test for safari except selenium.

My idea was to e2e test everything with Chrome and other browser issues with selenium. But playwright can cover 3 major evergreen browsers. Then, following that way is better than my original thought.

Then, changing the name to e2e-tests-ie or e2e-ie isn't a bad idea.

2.

AFAIK, selenium is the only option to e2e test IE. And locally setting up selenium IE driver is really tedious. (check here) And I guess we cannot manipulate the Internet options in CIs. As

That's why I didn't include this in scripts. Maybe some developers need to use BrowserStack and Selenium for their own IE e2e tests. Then, they can set up their own projects. Setup isn't that hard. And the current e2e-selenium is good as an example project or a boilerplate, and is not as a script.

And these tests won't be written in Puppeteer or Playwright. In addition, they cannot be run together. So, they are not in e2e-tests.

And utils for these tests should be written for Selenium. They need different set of dependencies and structures. Therefore, they're not written in e2e-test-utils.

But I admit that I read those packages and borrowed ideas to create e2e-selenium.

3.

When I was implementing this package, an idea that was bugging me was "Do we really need this?"

IE is a legacy browser. It's slow and doesn't support ES2015+ and it won't. It's slowly losing market share.

But that also made me think "when should we safely say that IE11 is dead" or "when is the safe time to say that it is better to use our time for other things than to maintain this ie test package?"

As we all know, Microsoft won't release the next version of Windows 10 in the near future. (Their goal is to make it evergreen.) And IE is part of Windows 10. In other words, IE'll be always with us until Windows 10 is gone. It's really unlikely that Microsoft will announce the official death of IE.

I've checked 2 browser market share sites and the market share of IE is 5.87% and 3.77%. It lost about 1.7-3% of market share in 2019. As browserslist default is >0.5%, we can safely drop IE in 2022~2024 assuming that IE loses its market share linearly.

But the problem is that firefox is losing share at the same time. I hope when IE becomes under .5%, Firefox becomes something like 1.12%.

tellthemachines

This is working well! Just a few comments below. Then once it's merged, we'll be faced with the task of figuring out everything we should test for 😄

tellthemachines · 2020-04-14T01:35:16Z

packages/e2e-selenium/config/browserstack.js

+const stopBrowserStackLocal = async () => {
+	return new Promise( ( resolve ) => {
+		browserstackLocal.stop( function() {
+			resolve();


Shouldn't we add a reject here in case there's an error?

class Local { start(options: Partial<Options>, callback: (error?: Error) => void): void; isRunning(): boolean; stop(callback: () => void): void; }

As you can see, error is not returned as an argument. That's why there is no reject.

I added this as a comment.

tellthemachines · 2020-04-14T01:52:35Z

packages/e2e-selenium/jest.config.js

@@ -0,0 +1,12 @@
+module.exports = {
+	setupFilesAfterEnv: [
+		//'@wordpress/jest-console',


Can this be removed?

In Gutenberg, console.log is not allowed by jest-console. But I needed console.log to print out the link to the result.

That's why it's commented out. When I solve the problem, it'll be uncommented.

tellthemachines · 2020-04-14T03:27:19Z

packages/e2e-selenium/util/close-welcome-guide.js

+const { login } = require( './login' );
+const { createNewPost } = require( './create-new-post' );
+
+const closeWelcomeGuide = async () => {


This function name is confusing because three different things happen here, and "close welcome guide" is probably the least significant of them. It would be clearer to name it something like loginAndCreateNewPost or even navigateToEditor.

AFAIK, selenium doesn't have an evaluate in Puppeteer. Because of that, we cannot write code like below in create-new-post in test-utils.

const isWelcomeGuideActive = await page.evaluate( () => wp.data.select( 'core/edit-post' ).isFeatureActive( 'welcomeGuide' ) ); if ( showWelcomeGuide !== isWelcomeGuideActive ) { await page.evaluate( () => wp.data.dispatch( 'core/edit-post' ).toggleFeature( 'welcomeGuide' ) ); await page.reload(); }

All we can do is open up a new page in the beginning and close it programmatically by sending ESC key.

That's what it does: closing welcome guide for the first time.

Note. I tried to mimic the code above with driver.executeJavaScript but failed. Because it tries to find an element right after reloading started.

That's why createNewPost and closeWelcomeGuide are separate functions.

Perhaps I wasn't clear enough. My issue is with the function being called closeWelcomeGuide, when what it is doing is:

Login;

create new post;

close welcome guide.

It would be better to give this function a name that reflects all three of the things it does, instead of just the last one.

How about fixing that in this way?

Remove login() and createNewPost().

Call them in beforeAll() instead.

It's clearer than simply calling closeWelcomeGuide() inside the beforeAll().

Sure, that would work!

tellthemachines · 2020-04-14T04:27:21Z

packages/e2e-selenium/util/create-new-post.js

@@ -0,0 +1,7 @@
+const createNewPost = async () => {
+	await driver.get( 'http://localhost:8888/wp-admin/post-new.php' );


It might be better to use localhost:8889 instead, as it's specifically set up for testing. 8888 being the environment used for development, tests might be affected by currently active plugins or other local changes.

tellthemachines · 2020-04-14T04:31:07Z

packages/e2e-selenium/README.md

@@ -0,0 +1,13 @@
+# E2E Selenium
+
+End-To-End (E2E) tests for WordPress written for legacy browsers.


I would be perfectly happy to have these tests running on IE only. None of the other browsers have caused (or are likely to cause) anywhere near as much trouble as IE.

tellthemachines · 2020-04-14T04:40:28Z

But that also made me think "when should we safely say that IE11 is dead" or "when is the safe time to say that it is better to use our time for other things than to maintain this ie test package?"

This has been discussed recently, in several places. It's a pretty popular topic 😅
There's some more context on why we need to continue support for IE here.

aduth · 2020-04-14T14:43:00Z

I've felt a lot of uncertainty around "Which browsers do we test?", "How is this distinct from existing testing?", "Should we invest in this if Internet Explorer is on its way out?"

One of the advantages of using a service like BrowserStack is that it supports many different combinations of operating system and browser. With that in mind, I think modeling this tooling as targeted toward testing regressions of specific issues under specific configuration of browser not otherwise covered by end-to-end tests may be a nice way to address all of the above questions:

Which browsers do we test?
- It's not really targeted toward specific browsers, so much as driven by the issues wherein a specific configuration had previously resulted in buggy behavior.
How is this distinct from existing testing?
- @wordpress/e2e-tests are meant to cover a predictable set of standard behaviors which behave relatively consistently across all browsers.
Should we invest in this if Internet Explorer is on its way out?
- As noted above, under this model, it's not necessarily targeted at Internet Explorer alone, even if the majority of the current set of test cases might incidentally be covering Internet Explorer issues.

As far as impact that has on the proposal:

The way you've structured the files as associated with a specific GitHub issue (e.g. 21276.js) is a nice pattern to emphasize the fact that tests within this package are explicitly tied with specific issues, and not with general behavior.
- This feels good to me. It also helps encourage that issues be created to track bugs.
Organizing test cases in browser-specific directories (e.g. ie/21276.js) may also help set a convention for how this could be expanded to additional browsers as well.
- Alternatively, we could consider making this more of a configuration of the test itself, especially if we need to support pretty granular configurations (specific versions of browsers, specific operating systems, or versions of operating systems including 32 bit vs. 64 bit).
Naming: We'd not want to change to e2e-tests-ie or e2e-ie under this model. The current naming may be fine enough, or even emphasizing Browserstack and the fact these tests would cover many different possible browser combinations.

global.d.ts doesn't work.

sainthkh · 2020-04-15T03:24:48Z

1.

After reading your comment, I realized that I forgot there are still differences between browsers. I agree with that this package is necessary even after IE is dead.

2.

I think that the better name for this package should go without selenium. Because selenium is the web browser automation tool. It emphasizes automation, not problems we met in various browsers.

It seems that unclear name made us discuss "what is the difference between e2e-selenium and e2e-test."

e2e-specific-browsers or e2e-browserstack might be better.

3.

It would be ideal if we can customize it function a bit and write tests like this:

it('tests something', { preset: 'iphone-safari' }, async () => {
  // test code.
})

it('test other thing', { 
  browserName: 'firefox',
  os: 'Windows',
  os_version: '8.1',
}, async () => {
  // test code
})

And organize test files like below:

test/
- ie/
  - 21276.js
- 12345.js
- 33313.js

(I believe IE needs a special folder because most of them would be related with IE. And for others, let's start by adding them in a single list. When the list becomes big, we'll see the pattern and we'll naturally know how to organize them.)

Maybe we can run the test like:

npm run test-browserstack # test all
npm run test-browserstack --preset=iphone-safari # specific preset
npm run test-browserstack --issue=12345 # specific issue
npm run test-browserstack --browser=safari #specific browser
npm run test-browserstack --os=mac #specific os

sainthkh · 2020-04-15T07:30:49Z

I could reproduce the lint failure on Ubuntu 18.04. Found out that the problem was on npm-package-json-lint. I opened a new PR at #21597.

sainthkh · 2020-04-30T04:08:57Z

Reference to the meeting about this PR: click here.

gziolo · 2021-03-30T10:13:22Z

WordPress is dropping support for IE 11 later this year so this work is no longer necessary:

https://make.wordpress.org/core/2021/03/25/discussion-summary-dropping-support-for-ie11/

Thank you for all the work done to explore this approach 🙇🏻

sainthkh requested review from ajitbohra and chrisvanpatten as code owners April 10, 2020 01:34

sainthkh mentioned this pull request Apr 10, 2020

Testing: Include IE11 "Canary" test in Travis build #16509

Closed

gziolo added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. Needs Technical Feedback Needs testing from a developer perspective. labels Apr 10, 2020

gziolo requested a review from aduth April 10, 2020 04:38

gziolo added the Browser Issues Issues or PRs that are related to browser specific problems label Apr 10, 2020

aduth reviewed Apr 10, 2020

View reviewed changes

package.json Show resolved Hide resolved

aduth reviewed Apr 10, 2020

View reviewed changes

packages/e2e-selenium/.env.template Outdated Show resolved Hide resolved

packages/e2e-selenium/index.d.ts Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

aduth requested a review from tellthemachines April 10, 2020 19:12

sainthkh force-pushed the try/e2e-selenium-webdriver branch from 70cbb01 to c00d48f Compare April 13, 2020 04:21

sainthkh commented Apr 13, 2020

View reviewed changes

sainthkh requested a review from aduth April 13, 2020 04:48

aduth reviewed Apr 13, 2020

View reviewed changes

sainthkh force-pushed the try/e2e-selenium-webdriver branch from c00d48f to eea3352 Compare April 14, 2020 02:57

tellthemachines reviewed Apr 14, 2020

View reviewed changes

sainthkh added 3 commits April 15, 2020 11:44

Initial update.

5fffe54

Set up example test.

a0bef92

Set up jest.

52969d8

sainthkh added 18 commits April 15, 2020 11:44

Set up browserstack.local.

6450dbb

global.d.ts -> index.d.ts.

f6fac7f

global.d.ts doesn't work.

Write test for local test.

bec8247

Show link to the result page.

9b29c01

Add .env template.

337eac2

Add NAME and KEY constants.

a03d962

Add example test.

b6009b2

Update structure.

f1a99f7

Remove unnecessary test file.

e6f1144

Editorconfig.

8ce04fc

Optin typescript type checking.

ec2aabe

Add e2e-selenium to package.json

af7636b

roodir to lib.

c4e7734

Explain why test folders are not excluded.

3149b94

Add comment why reject is unnecessary.

7c81496

8888 -> 8889

a877909

rootDir -> rootDirs to avoid tsc failures.

c56cb73

Show error message when the test website is not launched.

5a7cbe6

sainthkh force-pushed the try/e2e-selenium-webdriver branch from eea3352 to 5a7cbe6 Compare April 15, 2020 02:46

Add description to package.json.

c7156c2

sainthkh force-pushed the try/e2e-selenium-webdriver branch from 5b03a17 to c7156c2 Compare April 15, 2020 07:19

sainthkh mentioned this pull request Apr 15, 2020

Update npm-package-json-lint from 4.0.3 to 5.0.0 #21597

Merged

1 task

sainthkh mentioned this pull request May 7, 2020

Experimenting Chromatic #22155

Open

gziolo mentioned this pull request Jan 7, 2021

Update linting and formatting for test plugin files. #28033

Merged

Base automatically changed from master to trunk March 1, 2021 15:43

gziolo closed this Mar 30, 2021

		@@ -0,0 +1,13 @@
		# E2E Selenium

		End-To-End (E2E) tests for WordPress written for legacy browsers.

		// commented out because src folder doesn't exist in this package.
		//"rootDir": "src",

		@@ -0,0 +1,7 @@
		const createNewPost = async () => {
		await driver.get( 'http://localhost:8888/wp-admin/post-new.php' );

e2e test with selenium + browserstack for legacy browsers like IE. #21528

e2e test with selenium + browserstack for legacy browsers like IE. #21528

Conversation

sainthkh commented Apr 10, 2020

Description

How has this been tested?

Screenshots

Types of changes

Things to discuss.

1. How to show the result link.

2. Moving the selenium+browserstack configuartion to packages/scripts?

3. How to organize tests.

4. Setting up BrowserStack and Travis CI.

Checklist:

aduth commented Apr 10, 2020 • edited Loading

aduth commented Apr 10, 2020

sainthkh commented Apr 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisvanpatten commented Apr 13, 2020 • edited Loading

sainthkh commented Apr 13, 2020

aduth commented Apr 13, 2020

aduth commented Apr 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sainthkh Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Apr 13, 2020

sainthkh commented Apr 14, 2020 • edited Loading

tellthemachines left a 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

sainthkh Apr 15, 2020 • edited Loading

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

tellthemachines commented Apr 14, 2020

aduth commented Apr 14, 2020

sainthkh commented Apr 15, 2020 • edited Loading

sainthkh commented Apr 15, 2020

sainthkh commented Apr 30, 2020

gziolo commented Mar 30, 2021

aduth commented Apr 10, 2020 •

edited

Loading

sainthkh commented Apr 13, 2020 •

edited

Loading

chrisvanpatten commented Apr 13, 2020 •

edited

Loading

sainthkh Apr 14, 2020 •

edited

Loading

sainthkh commented Apr 14, 2020 •

edited

Loading

sainthkh Apr 15, 2020 •

edited

Loading

sainthkh commented Apr 15, 2020 •

edited

Loading