feat(test): POC e2e tests #3998

yannickcr · 2019-07-30T11:56:31Z

Summary

This PR contains a sample End-2-End test and a configuration for each tested service.

See related gist for details and instructions about how to run tests on each service.

Haroenv · 2019-07-30T12:05:02Z

e2e/test.js

+    );
+
+    // Select "Apple" brand in list
+    const brand = await browser.$('span=Apple');


is there a reason this isn't querySelector?

We are not in the browser context here so we have to use the selector from WebdriverIO. It is a bit different than querySelector https://webdriver.io/docs/selectors.html

Haroenv · 2019-07-30T12:05:53Z

e2e/test.js

+    // Wait for the results list to be updated (wait for the "macbook" word to be highlighted)
+    await browser.waitUntil(
+      async () => (await browser.$$('mark=MacBook')).length > 0,
+      10000


can we not rely on the search callback instead of a magic number?

What do you mean by search callback? You can find the API documentation here https://webdriver.io/docs/api.html

We can wait for the result event of search for example. However that isn't very portable between frameworks. Since we know there's only one search done, the search client could be injected & waited for maybe

Since it is an E2E test we're not supposed to interact with InstantSearch differently from a user.

We should only wait for a visual feedback that the search is done, like a real user.

francoischalifour · 2019-07-30T12:38:32Z

e2e/test.js

+    );
+
+    // Select "Apple" brand in list
+    const brand = await browser.$('span=Apple');


Can we rely on something else than span? This seems flaky and we shouldn't describe where to get the information, but rather what information to get. In that case we want the label (this is what drives accessibility) element having as value "Apple".

The label element have the value Apple\n442 and the linebreak seems to trigger some issues with the selector (I did not take the time to try to fix it).

But we can also use CSS selectors if needed. https://webdriver.io/docs/selectors.html

Targeting the right elements is tricky in E2E test as the UI and/or CSS classes can change frequently.

IMHO in our case we should rely on CSS classes, since they are pretty stable in InstantSearch (any change would be a breaking change).

I wish to be able to use something like ByLabelText here :(

If span sounds flaky, IMO, the class names are less flaky since we try to maintain all the same class names across the flavors(which is far from a good way to test in accessibility-wise, and which is testing implementation details).

Since we use them for E2E tests we can imagine the usage for [test-id] attribute in the DOM.

Yes, that's also a possibility.

test- attributes could be nice by I don't see how we can do it without altering every the DOM of every widget.
We're pretty lucky our css classes are pretty stable across flavours, using them should be good enough

Maybe this can help: https://github.com/testing-library/testcafe-testing-library.

Sadly this is not compatible with WebdriverIO 😞

francoischalifour · 2019-07-30T12:40:48Z

e2e/test.js

+
+    // Get title for each result
+    const hits = await browser.$$('.hit h1');
+    const hitsText = await Promise.all(hits.map(hit => hit.getText()));


Why do we extract only the text and not the HTML for also the highlighted parts? This seems to cover better the actual usage.

Here I'm only testing that I got the right list of results, I do not test that the text is correctly highlighted.

francoischalifour · 2019-07-30T12:42:18Z

wdio.browserstack.config.js

+  hostname: 'hub.browserstack.com',
+  port: 443,
+  baseUrl: '',
+  user: process.env.BROWSERSTACK_USER,


Not sure yet if these E2E tests are also meant to be run on our local machines. If so, we can consider adding dotenv to our stack.

When the e2e tests fail on CI server, we'll probably want to try it again on our local machines. So let's consider adding it :)

francoischalifour · 2019-07-30T12:43:39Z

wdio.browserstack.config.js

+    'browserstack.networkLogs': true,
+  },
+
+  capabilities: [


What is the time impact of adding multiple other combinations?

It depends of the offer on the test service.

With an offer like Automate Pro on BrowserStack we can run 5 tests in parallel, so 5 combinations in total without impacting the test duration.

https://www.browserstack.com/automate/parallel-calculator

tkrugg · 2019-08-01T08:27:47Z

e2e/test.js

+    browser.maximizeWindow();
+  });
+
+  it('must work', async () => {


For end to end test, I'd tend to use it in a slightly different way that in a unit test, for readability of the report and to clarify the intent of the commands that failed.
Example in this case:

it('navigates to the e-commerce demo', () => { await browser.url('http://localhost:5000/examples/e-commerce/'); }); it('selects Apple in the brands refinement list', () => { const brand = await browser.$('span=Apple'); await brand.click(); });

I'm just suggesting. This test is small enough that it's doesn't beg for it 😄

I do not have any opinion on this, it makes the test a bit longer but one advantage of this is that it annotates the test, for example on Sauce Labs you'll get:

(this is similar to the use of sauce:context)

tkrugg · 2019-08-01T08:29:57Z

e2e/test.js

+
+    // Compare them to expected titles
+    expect(hitsText).to.deep.equal([
+      'Apple - MacBook Air® (Latest Model) - 13.3" Display - Intel Core i5 - 8GB Memory - 128GB Flash Storage - Silver',


Here I think what we expect is to have Apple in all the hits text so maybe we should just check that?
There are less changes the test will fail if for some reason the ranking a or hit changed.

Mmm, I prefer to test that we get exactly the results we expect, and checking for only "Apple" is too vague IMO.

I find this as a good thing to have the test to fail if the ranking or a hit change 😇

This is something we can do with a dedicated demo: we can show the value of the attribute that will be filtered in the UI so we can test it

That would be equivalent to exposing some internals for testing, not a good practice and it would ruin the purpose of having end-2-end tests.

Not really true, the attribute is what proves that it's filtered, since the filter isn't related to the search query per se, and can be in multiple attributes

Let's not forget that we are supposed to test from a end user POV, so what matter at the end is that the result list is correct.

examples/calendar-widget/package.json

yannickcr · 2019-09-06T09:41:25Z

Implementation:

feat(test): POC e2e tests

d3221db

yannickcr added the ⚠️ DO NOT MERGE label Jul 30, 2019

yannickcr requested review from eunjae-lee, tkrugg, francoischalifour, Haroenv and samouss July 30, 2019 11:56

Haroenv reviewed Jul 30, 2019

View reviewed changes

francoischalifour reviewed Jul 30, 2019

View reviewed changes

yannickcr added 2 commits July 30, 2019 17:00

Fix ESLint errors

9ecb579

Fix tests

ef78b20

yannickcr force-pushed the poc/e2e-tests branch from cd1502d to ef78b20 Compare July 30, 2019 15:00

Add E2E tests on CircleCI with SauceLabs

6dc0b3a

yannickcr force-pushed the poc/e2e-tests branch from 9f1b116 to 6dc0b3a Compare July 31, 2019 17:04

tkrugg reviewed Aug 1, 2019

View reviewed changes

examples/calendar-widget/package.json Show resolved Hide resolved

Haroenv removed the request for review from samouss September 2, 2019 08:54

yannickcr closed this Sep 6, 2019

yannickcr deleted the poc/e2e-tests branch September 6, 2019 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test): POC e2e tests #3998

feat(test): POC e2e tests #3998

yannickcr commented Jul 30, 2019

Haroenv Jul 30, 2019

yannickcr Jul 30, 2019

Haroenv Jul 30, 2019

yannickcr Jul 30, 2019

Haroenv Jul 30, 2019

yannickcr Jul 30, 2019

francoischalifour Jul 30, 2019 •

edited

yannickcr Jul 30, 2019 •

edited

eunjae-lee Jul 30, 2019 •

edited

samouss Jul 31, 2019

yannickcr Jul 31, 2019

tkrugg Aug 1, 2019

francoischalifour Aug 10, 2019

yannickcr Aug 12, 2019

francoischalifour Jul 30, 2019

yannickcr Jul 30, 2019

francoischalifour Jul 30, 2019

eunjae-lee Jul 30, 2019

francoischalifour Jul 30, 2019 •

edited

yannickcr Jul 30, 2019 •

edited

tkrugg Aug 1, 2019

yannickcr Aug 1, 2019

tkrugg Aug 1, 2019

yannickcr Aug 1, 2019

Haroenv Aug 21, 2019

yannickcr Aug 21, 2019

Haroenv Aug 21, 2019

yannickcr Aug 21, 2019

yannickcr commented Sep 6, 2019

feat(test): POC e2e tests #3998

feat(test): POC e2e tests #3998

Conversation

yannickcr commented Jul 30, 2019

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

francoischalifour Jul 30, 2019 • edited

Choose a reason for hiding this comment

yannickcr Jul 30, 2019 • edited

Choose a reason for hiding this comment

eunjae-lee Jul 30, 2019 • edited

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

francoischalifour Jul 30, 2019 • edited

Choose a reason for hiding this comment

yannickcr Jul 30, 2019 • edited

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

yannickcr commented Sep 6, 2019

francoischalifour Jul 30, 2019 •

edited

yannickcr Jul 30, 2019 •

edited

eunjae-lee Jul 30, 2019 •

edited

francoischalifour Jul 30, 2019 •

edited

yannickcr Jul 30, 2019 •

edited