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.

Sign up for GitHub

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

Jump to bottom

feat(test): POC e2e tests #3998

Closed
wants to merge 4 commits into from
Closed

feat(test): POC e2e tests #3998

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3221db
feat(test): POC e2e tests
yannickcr Jul 30, 2019
9ecb579
Fix ESLint errors
yannickcr Jul 30, 2019
ef78b20
Fix tests
yannickcr Jul 30, 2019
6dc0b3a
Add E2E tests on CircleCI with SauceLabs
yannickcr Jul 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ module.exports = {
'valid-jsdoc': 0,
},
},
{
files: ['e2e/**/*.js'],
extends: ['plugin:wdio/recommended'],
plugins: ['wdio'],
env: {
mocha: true,
jest: false,
},
rules: {
'jest/valid-expect': 'off',
},
},
],
settings: {
react: {
Expand Down
51 changes: 51 additions & 0 deletions e2e/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
const expect = require('chai').expect;

describe('InstantSearch e-commerce demo', () => {
before(() => {
browser.maximizeWindow();
});

it('must work', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Screen Shot 2019-08-01 at 18 30 52

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

await browser.url(
'https://deploy-preview-3967--instantsearchjs.netlify.com/examples/e-commerce/'
);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason this isn't querySelector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Contributor Author

@yannickcr yannickcr Jul 30, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's also a possibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly this is not compatible with WebdriverIO 😞

await brand.click();

// Fill search input with "macbook"
const searchInput = await browser.$('[type=search]');
await searchInput.setValue('macbook');

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// 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',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😇

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

'Apple - MacBook Air® (Latest Model) - 13.3" Display - Intel Core i5 - 8GB Memory - 256GB Flash Storage - Silver',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M3 - 8GB Memory - 256GB Flash Storage - Space Gray',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M3 - 8GB Memory - 256GB Flash Storage - Gold',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M3 - 8GB Memory - 256GB Flash Storage - Rose Gold',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M3 - 8GB Memory - 256GB Flash Storage - Silver',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M5 - 8GB Memory - 512GB Flash Storage - Space Gray',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M5 - 8GB Memory - 512GB Flash Storage - Rose Gold',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M5 - 8GB Memory - 512GB Flash Storage - Gold',
'Apple - Macbook® (Latest Model) - 12" Display - Intel Core M5 - 8GB Memory - 512GB Flash Storage - Silver',
'Apple - MacBook Pro with Retina display - 13.3" Display - 8GB Memory - 128GB Flash Storage - Silver',
'Apple - MacBook Pro® - 13" Display - Intel Core i5 - 8 GB Memory - 256GB Flash Storage (latest model) - Space Gray',
'Apple - MacBook® Pro - 15.4" Display - Intel Core i7 - 16GB Memory - 256GB Flash Storage - Silver',
'Apple - MacBook Pro® - 13" Display - Intel Core i5 - 8 GB Memory - 256GB Flash Storage (latest model) - Silver',
'Apple - MacBook® Pro - Intel Core i5 - 13.3" Display - 4GB Memory - 500GB Hard Drive - Silver',
'Apple - MacBook Pro 13.3" Refurbished Laptop - Intel Core i5 - 4GB Memory - 320GB - Silver',
]);
});
});
1 change: 1 addition & 0 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module.exports = {
'<rootDir>/node_modules/',
'<rootDir>/dist*',
'<rootDir>/functional-tests',
'<rootDir>/e2e',
],
watchPathIgnorePatterns: [
'<rootDir>/cjs',
Expand Down
14 changes: 14 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@
"type-check": "tsc",
"type-check:watch": "yarn type-check --watch",
"test": "jest",
"test:e2e": "yarn test:e2e:lambdatest",
"test:e2e:lambdatest": "wdio wdio.lambdatest.config",
"test:e2e:crossbrowsertesting": "wdio wdio.crossbrowsertesting.config.js",
"test:e2e:browserstack": "wdio wdio.browserstack.config.js",
"test:e2e:experitest": "wdio wdio.experitest.config.js",
"test:e2e:saucelabs": "wdio wdio.saucelabs.config.js",
"test:watch": "jest --watch --bail",
"test:size": "bundlesize",
"test:argos": "argos upload functional-tests/screenshots --token $ARGOS_TOKEN || true",
Expand Down Expand Up @@ -80,6 +86,11 @@
"@types/storybook__addon-actions": "^3.4.2",
"@typescript-eslint/eslint-plugin": "1.13.0",
"@typescript-eslint/parser": "1.13.0",
"@wdio/cli": "5.11.6",
"@wdio/crossbrowsertesting-service": "5.11.4",
"@wdio/local-runner": "5.11.7",
"@wdio/mocha-framework": "5.11.0",
"@wdio/spec-reporter": "5.11.7",
"algoliasearch": "3.33.0",
"babel-eslint": "10.0.2",
"babel-jest": "24.8.0",
Expand All @@ -90,6 +101,7 @@
"babel-plugin-transform-react-pure-class-to-function": "1.0.1",
"babel-plugin-transform-react-remove-prop-types": "0.4.24",
"bundlesize": "0.18.0",
"chai": "4.2.0",
"colors": "1.3.2",
"conventional-changelog-cli": "2.0.21",
"doctoc": "1.4.0",
Expand All @@ -105,13 +117,15 @@
"eslint-plugin-jest": "22.14.0",
"eslint-plugin-prettier": "3.1.0",
"eslint-plugin-react": "7.14.3",
"eslint-plugin-wdio": "5.11.0",
"inquirer": "6.5.0",
"jest": "24.8.0",
"jest-diff": "24.8.0",
"jest-environment-jsdom": "24.8.0",
"jest-environment-jsdom-global": "1.2.0",
"jest-watch-typeahead": "0.3.1",
"jsdom-global": "3.0.2",
"mocha": "6.2.0",
"mversion": "1.13.0",
"places.js": "1.16.4",
"prettier": "1.18.2",
Expand Down
64 changes: 64 additions & 0 deletions wdio.browserstack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/* eslint-disable import/no-commonjs */

const config = {
hostname: 'hub.browserstack.com',
port: 443,
baseUrl: '',
user: process.env.BROWSERSTACK_USER,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

key: process.env.BROWSERSTACK_KEY,

specs: ['./e2e/test.js'],
maxInstances: 10,

// Capabilities Generator
// https://www.browserstack.com/automate/capabilities?tag=selenium-4
commonCapabilities: {
'browserstack.debug': true,
'browserstack.video': true,
'browserstack.console': 'verbose',
'browserstack.networkLogs': true,
},

capabilities: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the time impact of adding multiple other combinations?

Copy link
Contributor Author

@yannickcr yannickcr Jul 30, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
os: 'Windows',
osVersion: '10',
resolution: '1680x1050',
browserName: 'Chrome',
browserVersion: '75.0',
},
{
os: 'OS X',
osVersion: 'Mojave',
resolution: '1600x1200',
browserName: 'Firefox',
browserVersion: '68.0',
},
{
osVersion: '12',
deviceName: 'iPhone XS',
realMobile: 'true',
},
],

logLevel: 'trace',
coloredLogs: true,
waitforTimeout: 10000,
connectionRetryTimeout: 90000,
connectionRetryCount: 0,

framework: 'mocha',
reporters: ['spec'],
mochaOpts: {
ui: 'bdd',
timeout: 60000,
},
};

// Map common capabilities to capabilities
config.capabilities = config.capabilities.map(capability => ({
...config.commonCapabilities,
...capability,
}));

exports.config = config;
62 changes: 62 additions & 0 deletions wdio.crossbrowsertesting.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/* eslint-disable import/no-commonjs */

const config = {
hostname: 'hub.crossbrowsertesting.com',
port: 80,
baseUrl: '',
user: process.env.CBT_USER,
key: process.env.CBT_KEY,
services: ['crossbrowsertesting'],

specs: ['./e2e/test.js'],
maxInstances: 10,

// Capabilities Generator
// https://app.crossbrowsertesting.com/selenium/run
commonCapabilities: {
record_video: 'true', // eslint-disable-line @typescript-eslint/camelcase
record_network: 'true', // eslint-disable-line @typescript-eslint/camelcase
},

capabilities: [
{
browserName: 'Chrome',
version: '75',
platform: 'Windows 10',
screenResolution: '1680x1050',
},
{
browserName: 'Firefox',
platform: 'Mac OSX 10.14',
screenResolution: '1440x1050',
},
{
browserName: 'Safari',
deviceName: 'iPhone XR Simulator',
platformVersion: '12.0',
platformName: 'iOS',
deviceOrientation: 'portrait',
},
],

logLevel: 'trace',
coloredLogs: true,
waitforTimeout: 10000,
connectionRetryTimeout: 90000,
connectionRetryCount: 0,

framework: 'mocha',
reporters: ['spec'],
mochaOpts: {
ui: 'bdd',
timeout: 60000,
},
};

// Map common capabilities to capabilities
config.capabilities = config.capabilities.map(capability => ({
...config.commonCapabilities,
...capability,
}));

exports.config = config;
57 changes: 57 additions & 0 deletions wdio.experitest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* eslint-disable import/no-commonjs */

const config = {
hostname: 'cloud.seetest.io',
protocol: 'https',
port: 443,

specs: ['./e2e/test.js'],
maxInstances: 10,

// Capabilities Generator
// https://cloud.seetest.io/index.html#/quickstart
commonCapabilities: {},

capabilities: [
{
browserName: 'chrome',
browserVersion: '75.0.3770.142',
platform: 'Mac OS X High Sierra',
accessKey: process.env.EXPERITEST_ACCESS_KEY,
},
{
browserName: 'firefox',
browserVersion: '68.0.1',
platform: 'Windows 10',

accessKey: process.env.EXPERITEST_ACCESS_KEY,
},
{
platformName: 'ios',
deviceQuery: "@os='ios' and @version='12.0.1' and @category='PHONE'",

accessKey: process.env.EXPERITEST_ACCESS_KEY,
},
],

logLevel: 'trace',
coloredLogs: true,
waitforTimeout: 10000,
connectionRetryTimeout: 90000,
connectionRetryCount: 0,

framework: 'mocha',
reporters: ['spec'],
mochaOpts: {
ui: 'bdd',
timeout: 60000,
},
};

// Map common capabilities to capabilities
config.capabilities = config.capabilities.map(capability => ({
...config.commonCapabilities,
...capability,
}));

exports.config = config;
58 changes: 58 additions & 0 deletions wdio.lambdatest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/* eslint-disable import/no-commonjs */

const config = {
hostname: 'hub.lambdatest.com',
port: 80,
baseUrl: '',
user: process.env.LT_USER,
key: process.env.LT_KEY,

specs: ['./e2e/test.js'],
maxInstances: 10,

// Capabilities Generator
// https://www.lambdatest.com/capabilities-generator/
commonCapabilities: {
visual: true,
video: true,
console: true,
network: true,
},

capabilities: [
{
platform: 'Windows 10',
browserName: 'Chrome',
version: '76.0',
resolution: '1680x1050',
},
{
platform: 'macOS Mojave',
browserName: 'Firefox',
version: '67.0',
resolution: '1600x900',
},
// No mobile test :(
],

logLevel: 'trace',
coloredLogs: true,
waitforTimeout: 10000,
connectionRetryTimeout: 90000,
connectionRetryCount: 0,

framework: 'mocha',
reporters: ['spec'],
mochaOpts: {
ui: 'bdd',
timeout: 60000,
},
};

// Map common capabilities to capabilities
config.capabilities = config.capabilities.map(capability => ({
...config.commonCapabilities,
...capability,
}));

exports.config = config;