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.

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

test(labware-library): add cypress e2e tests to the labware creator #4800

Merged
merged 4 commits into from
Jan 23, 2020

Conversation

ewagoner
Copy link
Contributor

overview

This PR adds cypress.io infrastructure to the repo. Cypress.io is an e2e testing framework, similar to selenium but better suited for modern web applications. The first application to get cypress tests is the labware creator.

Since my time here is limited (that is, I will not be maintaining these tests), I opted for simplicity and minimal code abstraction. There is opportunity to create more general functions used in all the test runs I've made here. Cypress generates movies of all of the test runs, and I've included those in my commit.

Besides testing every permutation of custom labware, it also tests importing existing labware definitions for editing and re-export. All of these tests together run in just over two minutes. The next step for these tests could be to integrate them with the build/CI process so they get run automatically.

changelog

  • Added cypress and cypress-file-upload npm modules to the project.
  • Configured jest to ignore cypress tests.
  • Added 'make test-e2e' task to the labware-library application.
  • Created cypress directory structure and config to the labware-library.
  • Created a number of cypress integration tests for the labware-creator sub-application.

review requests

The only change to existing functionality is adding a rule to jest to skip the cypress tests when it run the unit tests. Everything else sits on top of what's already here.

@ewagoner ewagoner changed the title Lc add cypress e2e tests Tests - add cypress e2e tests to the labware creator Jan 17, 2020
@ewagoner ewagoner changed the title Tests - add cypress e2e tests to the labware creator test(labware creator): add cypress e2e tests to the labware creator Jan 17, 2020
@codecov
Copy link

codecov bot commented Jan 17, 2020

Codecov Report

Merging #4800 into edge will decrease coverage by 0.02%.
The diff coverage is 9.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #4800      +/-   ##
==========================================
- Coverage   57.95%   57.92%   -0.03%     
==========================================
  Files         956      956              
  Lines       26212    26237      +25     
==========================================
+ Hits        15191    15199       +8     
- Misses      11021    11038      +17
Impacted Files Coverage Δ
app/src/components/Resources/index.js 0% <ø> (ø) ⬆️
protocol-designer/src/form-types.js 0% <ø> (ø) ⬆️
app/src/pages/More/Resources.js 0% <ø> (ø) ⬆️
app/src/components/Resources/ResourceCard.js 0% <ø> (ø) ⬆️
app/src/components/App.js 0% <ø> (ø) ⬆️
app/src/index.js 0% <ø> (ø) ⬆️
protocol-designer/src/steplist/fieldLevel/index.js 38.88% <ø> (ø) ⬆️
...r/src/steplist/formLevel/getDefaultsForStepType.js 37.5% <ø> (ø) ⬆️
app/src/pages/More/index.js 0% <ø> (ø) ⬆️
protocol-designer/src/ui/steps/actions/actions.js 25% <0%> (-0.72%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 594bc05...a09ab0a. Read the comment docs.

…re creator

Added cypress and cypress-file-upload npm modules to the project. Configured jest to ignore cypress
tests. Added 'make test-e2e' task to the labware-library application. Created cypress directory
structure and configi to the labware-library. Created a number of integration tests for the
labware-creator sub-application.
test(labware creator): disabled no-undef lint rule for new cypress.io tests
@mcous mcous changed the title test(labware creator): add cypress e2e tests to the labware creator test(labware-library): add cypress e2e tests to the labware creator Jan 21, 2020
Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

This looks great! A few minor things to get this ready to merge:

  1. We should add e2e testing instructions to CONTRIBUTING.md
  2. We should update the eslint configuration to support cypress
  3. I can't find anything recommending committing video artifacts; can you either point me to such a recommendation or remove them?
  4. If possible, I think we'd prefer arrow functions in our describe etc blocks so we don't have to switch back and forth when writing unit and e2e tests

labware-library/Makefile Outdated Show resolved Hide resolved
labware-library/cypress/integration/home.spec.js Outdated Show resolved Hide resolved
labware-library/cypress/integration/home.spec.js Outdated Show resolved Hide resolved
ewagoner and others added 2 commits January 22, 2020 15:52
…back style changes

eslint configuration, integrate server with test make command, remove test run movies, add testing
instructions to contributing document
Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

Sorry, should've posted a more explicit example of what I meant by updating the eslint overrides. Otherwise this looks great!

.eslintrc.js Outdated Show resolved Hide resolved
@@ -194,6 +195,10 @@ make test
# run tests per language
make test-py
make test-js

# run cypress tests for the labware-library
cd labware-library
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally we prefer make -C directory task to explicit cds

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 was basing my addition on this from the protocol-designer readme:

# from the repo root

# install all dependencies
make install

cd protocol-designer/

# run the development server
make dev

The next round of additions will include protocol-designer tests, and I'll want to refactor how the tests get run, I think, since they'll be in multiple top-level applications. I'll revisit this then.

.eslintrc.js Outdated
@@ -13,7 +13,14 @@ module.exports = {
'prettier/standard',
],

plugins: ['flowtype', 'react', 'react-hooks', 'json', 'prettier'],
plugins: [
// 'eslint-plugin-cypress',
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover unnecessary comment (this plugin is enabled by the recommended plugin config in the override block in the appropriate files)

.eslintrc.js Outdated
@@ -35,6 +42,7 @@ module.exports = {
env: {
node: true,
browser: true,
// 'cypress/globals': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also leftover unnecessary comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

D'oh! I rebased my commit and got rid of those.

@ewagoner ewagoner merged commit 271f957 into edge Jan 23, 2020
@ewagoner