Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

test: add End-2-End test suite #2824

Merged
merged 4 commits into from
Sep 17, 2019
Merged

test: add End-2-End test suite #2824

merged 4 commits into from
Sep 17, 2019

Conversation

yannickcr
Copy link
Contributor

@yannickcr yannickcr commented Sep 11, 2019

Summary

Same as algolia/instantsearch#4035 but for react-instantsearch.

This PR makes the following changes:

  • Add algolia/instantsearch-e2e-tests as devDependency.
  • Add 2 WebdriverIO configurations: one to run tests locally and one to run them on Sauce Labs.
  • Add .env file to .gitignore so you can use it to store Sauce Labs credentials (see https://github.com/algolia/instantsearch-e2e-tests#credentials ).
  • Add npm scripts to run End-2-End tests:
    • test:e2e: Alias for test:e2e:local.
    • test:e2e:local: Run the test suite on Chrome browser on your local machine.
    • test:e2e:saucelabs: Run the test suite on multiple browsers on the Sauce Labs service.
  • Rename existing test:e2e to test:integration.
  • Update CircleCI configuration
    • Rename test_e2e task to test_integration.
    • Add the new test_e2e task.

@yannickcr yannickcr changed the base branch from chore/examples-e2e to master September 11, 2019 15:22
@yannickcr yannickcr marked this pull request as ready for review September 11, 2019 15:45
@yannickcr yannickcr requested review from tkrugg and Haroenv September 11, 2019 15:45
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

Looks promising overall & passes! All there’s to say is it seems some build steps happen multiple time, and that should be looked into

.circleci/config.yml Show resolved Hide resolved
"@wdio/sauce-service": "5.12.1",
"@wdio/selenium-standalone-service": "5.12.1",
"@wdio/spec-reporter": "5.12.1",
"@wdio/static-server-service": "5.12.1",
Copy link
Contributor

@tkrugg tkrugg Sep 11, 2019

Choose a reason for hiding this comment

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

I think I asked this question already but I'm no longer sure what's the answer.
Is it not possible to hide this away in instantsearch-e2e-tests and just expose a command there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my answer: algolia/instantsearch#4035 (comment)
(it can be discussed of course 😉)

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think we should hide this

Copy link
Member

Choose a reason for hiding this comment

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

It would also make sense to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well... I'm not for this.

First of all there is very few advantages (if any?) to "hide" dependencies but the real problem is mainly that it will makes us call a subdependency from the parent project (react-instantsearch calls wdio directly), which is not recommenced (the package version we're targeting may not be at the node_modules top level, depending how npm/yarn deduplicated the dependencies).

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 also hide wdio call then?

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 would be the objective of hiding these dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

It would remove the overall perceived complexity, as well as reduce the noise and cognitive overhead of managing these dependencies.

I find it surprising that we went all the way to packaging the E2E tests (e.g. see also wdio.saucelabs.conf.js which only exports the E2E package import), but are reluctant to hide the dependencies themself.

The E2E test suite would be more predictable if the wdio dependencies are packaged within it. Here, we are responsible for the wdio deps to work with a black-boxed test suite, which is quite contradictory.

package.json Outdated Show resolved Hide resolved
"@wdio/sauce-service": "5.12.1",
"@wdio/selenium-standalone-service": "5.12.1",
"@wdio/spec-reporter": "5.12.1",
"@wdio/static-server-service": "5.12.1",
Copy link
Member

Choose a reason for hiding this comment

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

It would also make sense to me.

yannickcr and others added 2 commits September 17, 2019 11:11
Co-Authored-By: François Chalifour <francoischalifour@users.noreply.github.com>
@yannickcr yannickcr merged commit 7f4943d into master Sep 17, 2019
@yannickcr yannickcr deleted the test/e2e branch September 17, 2019 09:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants