Adding an easy local environment setup and Cypress e2e tests #3069

youknowriad · 2017-10-19T14:15:01Z

Local environment

Right now, to setup a local dev environment for Gutenberg, you need to install WordPress on your own using vagrant, docker or a local install of php/mysql and then upload the gutenberg plugin there and built it.

This PR adds a script (that uses docker) to ease this setup, so install docker locally and then do:

# install the dependencies
npm install

# build the plugin (you can use npm run dev)
npm run build 

# setup WordPress env
# this runs the docker containers, install a WP site and active the Gutenberg plugin
# the WP username is "admin"
# the password is "password"
./bin/setup-local-env.sh

# your install should be availablee in http://localhost:8888

The first time you use docker, it will download the images (wordpress, mysql and wordpress:cli), it may time some minutes, but it will be quite quick the next ones.

E2E Tests

If you're using the local environment above, you can run the e2e tests locally using this command:

npx cypress run

or interactively

npx cypress open

If you're using another local environment setup, you can still run the e2e tests by overriding the base URL and the default WP username/password used in the tests like so:

cypress_base_url=http://my-custom-basee-url cypress_username=myusername cypress_password=mypassword npx cypress run

CI

This PR do not setup CI for these tests yet, but using the local environment and cypress runner, we can easily setup a job like this:

npm install
npm run build
./bin/run-e2e-tests.sh

I didn't add documentation yet. I wanted feedback first.

jubianchi · 2017-10-19T14:45:09Z

bin/run-e2e-tests.sh

+set -e
+
+# Change to the expected directory
+cd "$(dirname "$0")"


you should use pushd "$(dirname "$0")" here

jubianchi · 2017-10-19T14:45:23Z

bin/run-e2e-tests.sh

+
+# Run the tests
+cd "$(dirname "$0")"
+cd ../


you should use popd here

jubianchi · 2017-10-19T14:45:57Z

bin/run-e2e-tests.sh

+cd "$(dirname "$0")"
+
+# Setup local environement
+./setup-local-env.sh


you could run this in a subshell to avoir having to get back to your working directory (cd at line 11):

( ./setup-local-env.sh )

jubianchi · 2017-10-19T14:47:15Z

bin/setup-local-env.sh

+
+# Change to the expected directory
+cd "$(dirname "$0")"
+cd ../docker


you can merge these two cds into a single one.

jubianchi · 2017-10-19T14:48:04Z

bin/setup-local-env.sh

+docker-compose up -d
+
+# Find a way to sleep until the docker containers are setup properely
+sleep 20


you should look at docker-compose ps and grep:

Something like (in a loop):

docker-compose ps wordpress | grep 'Up'

Or a simple curl in a loop on the WP container until the homepage responds with a 200 Ok

mtias · 2017-10-19T15:34:50Z

It might be a nice way to recreate the demo post (or equivalent) as a test, including all these interactions:

Inserting a new block from the + inserter.
Inserting a block from the / inserter.
Moving a block up/down.
Changing attributes in the toolbar.
Changing attributes in the inspector.
Selecting multiple blocks and moving them.

If we can make sure the demo post can be recreated with every release we do, it should cover a big range of possible regressions.

youknowriad · 2017-10-19T15:37:34Z

It might be a nice way to recreate the demo post (or equivalent) as a test, including all these interactions

I would be more in favor of splitting these interactions into separate smaller posts, just to have the possibility to maintain and debug each one separately.

aduth · 2017-10-19T20:51:24Z

Is the local environment related to the e2e tests? Trying to understand the relation in a single pull request.

youknowriad · 2017-10-19T22:52:56Z

@aduth yes, they are related in a way. we need a local environment to run e2e tests in CI. the ./bin/run-e2e-tests.sh script setup the environment and runs the e2e tests on it.

Granted, I can split this into two PRs if needed. But the setup of both was small enough

ephox-mogran · 2017-10-20T03:58:16Z

The setup seems to work for me. Have successfully run the tests. Nice.

jasmussen · 2017-10-20T09:06:55Z

OMG this is amazing. This can really help get people up and running quick. CC @nosolosw because you've talked about something similar.

aduth

As we've seen a few interactions slip through the cracks lately, I would be very much in support of adding these e2e tests.

A few things:

When running locally, I'm seeing one failing test:

Adding blocks Should insert content using the placeholder, the quick inserter, the regular inserter:
CypressError: Timed out retrying: Expected to find element: ':focus', but never found it.

In this case, when the tests complete, the process never ends. It's stuck after a message:

(Screenshots)

/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/cypress/screenshots/Adding blocks -- Should insert content using the placeholder the quick inserter the regular inserter.png (2560x1440)

Would we plan to integrate this with CI? Otherwise how do we enforce that these tests are run? Are the tests fast enough ("Hello Gutenberg" took two full seconds for the single test).

Is browser state reset between tests, and this why we need to log in before each test?

aduth · 2017-10-23T13:02:21Z

bin/run-e2e-tests.sh

+( ./bin/setup-local-env.sh )
+
+# Run the tests
+npx cypress run


npx is only bundled with npm 5.x+ which is not explicitly required by our target Node version, though the new Node 8.x / npm 5.x LTS release is supposedly slated to be released by the end of the month. The alternative is:

./node_modules/.bin/cypress run

aduth · 2017-10-23T13:07:57Z

package.json

@@ -61,6 +61,7 @@
    "codecov": "2.3.0",
    "concurrently": "3.5.0",
    "cross-env": "3.2.4",
+    "cypress": "1.0.2",


Should we add an npm script to run these e2e tests?

Yeah, npx cypress run was simple enough, but since it's not yet available for everyone, a script could be helpful.

Also just consistency / consolidation. We have npx jest and npx eslint as well, with npm scripts which are effectively just that.

youknowriad · 2017-10-23T13:24:08Z

When running locally, I'm seeing one failing test:

I'm seeing it too, but surprisingly, when running the tests using npx cypress open it's working as expected.

I understand the error as the newly created paragraph didn't receive the focus when we clicked the "Write Story" placeholder. I wonder if it's something happening from time to time or it's just related to the tests context (this is a question we'd face quite often with e2e tests). I can update the test slightly to make it pass consistently.

Would we plan to integrate this with CI?

Everything is quite ready for it. We just need a travis job, I was wondering if we should limit to this to a manually run test when releasing (at first).

youknowriad · 2017-10-23T14:17:09Z

Is browser state reset between tests, and this why we need to log in before each test?

Between each "file", the browser state is reset but we can split a unique test into multiple "it" like done in bd2b032

aduth · 2017-10-24T16:23:52Z

In this case, when the tests complete, the process never ends. It's stuck after a message:

I'm still seeing the process become stuck after the tests complete.

aduth · 2017-10-24T16:24:32Z

.gitignore

@@ -1,6 +1,9 @@
 # Directories/files that may be generated by this project
 build
 coverage
+cypress/videos


I suppose these are useful to have when debugging a failing test? Do they come with some overhead to the test run time? Can they be disabled, and if so, should that be the default? Or at least considered for a CI context.

Yes, this could be turned off with a config option (or an env variable), but I do think it makes sense to keep it even in a CI context. The e2e tests can fail for mysterious reasons in CI and work in local and having videos somewhere could be great to debug those issues, don't you think?

Anyway, I don't mind disabling those by default and see if we need them later

aduth · 2017-10-24T16:25:02Z

bin/setup-local-env.sh

+echo "Attempting to connect to wordpress"
+until $(curl -L http://localhost:8888 -so - | grep -q "WordPress"); do
+    printf '.'
+    sleep 5


How long does the setup usually take? Seems like we could check more frequently than once every five seconds, to help reduce time to complete the full test run.

It took like 20 seconds for me

aduth · 2017-10-24T16:25:33Z

bin/setup-local-env.sh

+
+# Wait until the docker containers are setup properely
+echo "Attempting to connect to wordpress"
+until $(curl -L http://localhost:8888 -so - | grep -q "WordPress"); do


Minor: 8888 is a fairly common port, so we could encounter a conflict; then again, the risk always exists.

Is 1440 an option? It would align nicely with Printing since 1440. 💯

aduth · 2017-10-24T16:26:50Z

package.json

@@ -137,6 +138,8 @@
    "test-unit": "jest",
    "test-unit:coverage": "jest --coverage",
    "test-unit:coverage-ci": "jest --coverage --maxWorkers 1 && codecov",
-    "test-unit:watch": "jest --watch"
+    "test-unit:watch": "jest --watch",
+    "test-e2e": "cypress run",


Should these be running bin/run-e2e-tests.sh ? When switching to this branch again and running npm run test-e2e it showed me a cryptic error because the Docker instance wasn't running:

Opening Cypress... Cypress could not verify that the server set as your 'baseUrl' is running: > http://localhost:8888

Without context that bin/run-e2e-tests.sh even exists, I think this could be cause for confusion for new contributors.

Good question! If we switch this to bin/run-e2e-tests.sh this means for development purpose, this is useless since you do not want to setup the environment on each run.

What if we add a test-e2e:ci that runs bin/run-e2e-tests.sh?

Would the following setup work for you?:

"docker:setup-if-not-exists": "./bin/setup-local-env.sh", "pretest-e2e": "npm run docker:setup-if-not-exists", "test-e2e": "cypress run", "pretest-e2e:watch": "npm run docker:setup-if-not-exists", "test-e2e:watch": "cypress open"

This assumes that docker:setup-if-not-exists checks if docker instance is already there. If not it setups everything. This way it would work with both CI and local development.

docker:setup-if-not-exists - could also wrap over docker:setup so you could use it for local development and forget about VVV :)

I'm sure I could pick better names :D

Yeah it seems like we could use some pre hooks on these tasks that run the setup if it's not already available (idempotent setup).

aduth · 2017-10-24T16:27:20Z

bin/run-e2e-tests.sh

@@ -0,0 +1,11 @@
+# Exit if any command fails


Guessing our Windows friends are left out here? 😬

Not sure we want to optimize for Windows users here but if dropping this line is enough to make the tests pass there, maybe we should do it?

ephox-mogran · 2017-10-24T20:28:39Z

Given that cypress doesn't support Firefox at the moment, are we planning on having an alternative for Firefox, or will we just wait until cypress supports it?

According to their project page (cypress-io/cypress#310 (comment)), their current planned support date is the end of the year. They also mention that they have no plans to support IE at all. Is that going to be an issue? Will an alternative be required for IE?

youknowriad · 2017-10-25T08:52:26Z

Given that cypress doesn't support Firefox at the moment, are we planning on having an alternative for Firefox, or will we just wait until cypress supports it?

Running these tests against multiple browsers would be great but I don't see this as critical. For me, the idea here is to ensure we do not have basic feature regressions. These tests can't cover all use-cases because they are not so quick and they are not the easiest tests to debug/fix. The purpose is to not to test everything. We need to find a good balance between testing and maintenance hurdle.

youknowriad · 2017-10-25T09:07:07Z

I reverted the last commit because it's anti-pattern to have tests relying on previous tests https://docs.cypress.io/guides/references/best-practices.html#Having-tests-rely-on-the-state-of-previous-tests

youknowriad · 2017-10-25T09:26:36Z

I'm still seeing the process become stuck after the tests complete.

Yeah, I'm seeing it too, I thought it had to do with watching file changes but apparently not. Not sure what's happening :(

gziolo · 2017-10-27T13:59:04Z

FYI: I opened #3203 where I move setup files for unit tests in their own subfolder (/test/unit) to make it symmetric with changes introduced in this PR.

youknowriad · 2017-10-30T15:55:17Z

Reverting #2705 did fix the tests on travis. It looks like we have write permissions issues on travis because if we can't fetch a vendor script URL, we use the URL in a script tag as a fallback. And the reverted URL is a valid script while the former is not.

Is it a good idea to use a URL that is a valid fallback anyway? Doesn't mean we do not have to fix the underlying issue in Travis.

aduth · 2017-10-30T16:39:29Z

Is it a good idea to use a URL that is a valid fallback anyway? Doesn't mean we do not have to fix the underlying issue in Travis.

Yes, and yes. 😄

and use this environment in e2e tests script

…ndomly

youknowriad · 2017-10-31T10:23:44Z

Ok, this is ready to go, I'm running everything in parallel right now. If we notice slowness in these tests moving forward, we can limit their execution to master only.

codecov · 2017-10-31T11:00:43Z

Codecov Report

Merging #3069 into master will increase coverage by 1.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3069      +/-   ##
==========================================
+ Coverage    31.2%   32.34%   +1.13%     
==========================================
  Files         229      229              
  Lines        6428     6558     +130     
  Branches     1145     1183      +38     
==========================================
+ Hits         2006     2121     +115     
- Misses       3711     3723      +12     
- Partials      711      714       +3

Impacted Files	Coverage Δ
editor/sidebar/last-revision/index.js	`0% <0%> (ø)`	⬆️
editor/selectors.js	`95.79% <0%> (+0.52%)`	⬆️

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 fcca1f2...3e3b67d. Read the comment docs.

gziolo · 2017-10-31T11:41:11Z

This looks great. Awesome work. I'm super excited having this configured. I hope it's only beginning and we will iterate and add more tests :)

youknowriad added the Framework Issues related to broader framework topics, especially as it relates to javascript label Oct 19, 2017

youknowriad self-assigned this Oct 19, 2017

youknowriad requested review from mtias, gziolo, jasmussen and aduth October 19, 2017 14:15

jubianchi reviewed Oct 19, 2017

View reviewed changes

youknowriad requested a review from ephox-mogran October 19, 2017 15:11

youknowriad force-pushed the test/setup-e2e-tests branch from 0a5d5dd to 9f4c90f Compare October 19, 2017 17:02

youknowriad force-pushed the test/setup-e2e-tests branch from 9f4c90f to fb6272f Compare October 20, 2017 09:30

aduth reviewed Oct 23, 2017

View reviewed changes

youknowriad force-pushed the test/setup-e2e-tests branch from 8fbea81 to 8bec9df Compare October 23, 2017 13:34

aduth reviewed Oct 24, 2017

View reviewed changes

youknowriad force-pushed the test/setup-e2e-tests branch from bd2b032 to b165c2e Compare October 25, 2017 09:05

youknowriad mentioned this pull request Oct 26, 2017

Tests: Clarify the travis tests using build stages #3150

Merged

youknowriad force-pushed the test/setup-e2e-tests branch 2 times, most recently from 64631ec to 393ab22 Compare October 26, 2017 13:02

youknowriad force-pushed the test/setup-e2e-tests branch from 8afae05 to 03ecb07 Compare October 27, 2017 14:25

youknowriad mentioned this pull request Oct 27, 2017

Block: Assign block list ref from block child #3205

Merged

youknowriad force-pushed the test/setup-e2e-tests branch 2 times, most recently from a3ec0fa to 80e8acb Compare October 30, 2017 11:04

youknowriad and others added 13 commits October 31, 2017 10:20

Tests: Setup End 2 End Cypress Tests

fbdfe7b

Setup Local Dev environment using dockere

a276539

and use this environment in e2e tests script

Tests: Adding block inserting e2e test

3fb6e13

Local Env: Check for the container to be ready instead of sleeping ra…

b376b83

…ndomly

Tests: Adding npm scripts to run e2e tests

2aff5fc

Tests: make the adding blocks test pass consistently

3d32b0c

Tests: Disable cypress video recording

7947cbb

CI: Adding a job for e2e tests

c754a5c

Testing: Move cypress tests to test/e2e subfolder

5678b25

Testing: Set chrome as a target browser for CLI e2e run

1aba5f1

Documentation: Using the local environment and running e2e tests

af5c287

Tests: Disable secyrity in Chrome when running e2e to fix CI tests

793ec04

Tests: Run everything in parallel

8cc2642

youknowriad force-pushed the test/setup-e2e-tests branch from e37d11e to 8cc2642 Compare October 31, 2017 09:20

Tests: Run all php versions only on master

3e3b67d

youknowriad force-pushed the test/setup-e2e-tests branch from 84939b8 to 3e3b67d Compare October 31, 2017 11:00

gziolo approved these changes Oct 31, 2017

View reviewed changes

youknowriad merged commit 66ec0ad into master Oct 31, 2017

youknowriad deleted the test/setup-e2e-tests branch October 31, 2017 11:41

aduth mentioned this pull request Nov 3, 2017

Testing: Optimize e2e testing path navigation #3333

Merged

Adding an easy local environment setup and Cypress e2e tests #3069

Adding an easy local environment setup and Cypress e2e tests #3069

Conversation

youknowriad commented Oct 19, 2017 • edited Loading

Local environment

E2E Tests

CI

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

mtias commented Oct 19, 2017

youknowriad commented Oct 19, 2017

aduth commented Oct 19, 2017

youknowriad commented Oct 19, 2017 • edited Loading

ephox-mogran commented Oct 20, 2017

jasmussen commented Oct 20, 2017

aduth 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

youknowriad commented Oct 23, 2017

youknowriad commented Oct 23, 2017

aduth commented Oct 24, 2017

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

gziolo Oct 26, 2017 • 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

ephox-mogran commented Oct 24, 2017

youknowriad commented Oct 25, 2017

youknowriad commented Oct 25, 2017

youknowriad commented Oct 25, 2017

gziolo commented Oct 27, 2017

youknowriad commented Oct 30, 2017

aduth commented Oct 30, 2017

youknowriad commented Oct 31, 2017

codecov bot commented Oct 31, 2017 • edited Loading

Codecov Report

gziolo commented Oct 31, 2017

youknowriad commented Oct 19, 2017 •

edited

Loading

youknowriad commented Oct 19, 2017 •

edited

Loading

gziolo Oct 26, 2017 •

edited

Loading

codecov bot commented Oct 31, 2017 •

edited

Loading