Move cypress test project to own folder #134

jesstelford · 2018-06-23T00:12:31Z

I've left packages/test-project alone, but the Cypress tests now run against cypress/projects/basic.

Ideally there will be other cypress/projects/* in the future for different setups.

dominikwilkowski · 2018-06-23T00:29:04Z

I was just working on this. I thought we can still use the test project but clean data via graphql queries rather than relying on the reset route... would it not cause a lot more work now that we have two test projects? Relaying on one implementation and taking charge of it's store of data would be a better outcome IMO?

jesstelford · 2018-06-23T02:47:21Z

The main problem I'm trying to solve is completely different Keystone setups, not just data.

For example, I'll need a project where access control on everything is set to read: false, which means none of the other tests will pass.

The changes in this PR are the first step to having project(s) separate to the test-project that are run against cypress.

The biggest down side here is that the test-project will get out of sync with the projects we're testing against, but we could add a set of smoke tests to test-project to make sure it's all ok.

In terms of resetting data via GraphQL: That's a great idea, and would work really well with each of the individual projects we setup with this PR 👍

jesstelford · 2018-06-24T07:03:35Z

Huzzah! It works:

$ CYPRESS_CACHE_FOLDER=$CIRCLE_WORKING_DIRECTORY/node_modules/cypress/.cache/ yarn cypress:run:ci
yarn run v1.6.0
$ start-server-and-test cypress:start:server http-get://localhost:3000/admin cypress:run:cmd:ci
starting server using command "npm run cypress:start:server"
and when url "http-get://localhost:3000/admin" is responding
running tests using command "cypress:run:cmd:ci"

> keystone@5.0.0 cypress:start:server /home/circleci/repo
> cd cypress/projects/basic && PORT=3000 yarn start

$ node -r dotenv-safe/config index.js
KeystoneJS 5 ready on port 3000
Connection success
ℹ ｢wdm｣: wait until bundle finished: /admin
ℹ ｢wdm｣:    533 modules
ℹ ｢wdm｣: Compiled successfully.

> keystone@5.0.0 cypress:run:cmd:ci /home/circleci/repo
> cypress run --env PORT=3000 --reporter junit --reporter-options "mochaFile=reports/junit/cypress-results.xml"

# ... Cypress runs

    All specs passed!                           01:11       33       33        -        -        -  

Done in 123.57s.

jesstelford · 2018-06-24T07:40:49Z

cypress/integration/add-edit-delete-data.js

@@ -6,8 +6,6 @@ describe('Adding data', () => {
        'ks-input-name': 'John Doe',
        'ks-input-email': 'john@gmail.com',
        'ks-input-password': 'password1',
-        'ks-input-twitterId': '@johndoe',
-        'ks-input-twitterUsername': 'John Doe',


I simplified the tests a bit by removing the twitter stuff since it wasn't actually under test beyond "is shown in admin UI", which is passing tests elsewhere.

jesstelford · 2018-06-24T07:42:09Z

package.json

+    "cypress:run:cmd:ci": "cypress run --env PORT=3000 --reporter junit --reporter-options \"mochaFile=reports/junit/cypress-results.xml\"",
+    "cypress:open": "start-server-and-test cypress:start:server http-get://localhost:3000/admin cypress:open:cmd",
+    "cypress:run": "start-server-and-test cypress:start:server http-get://localhost:3000/admin cypress:run:cmd",
+    "cypress:run:ci": "start-server-and-test cypress:start:server http-get://localhost:3000/admin cypress:run:cmd:ci",


start-server-and-test waits for the server to be up and running before trying to run the tests, removing any timing issues with webpack compilation 👍

JedWatson · 2018-06-25T00:34:38Z

package.json

@@ -98,7 +103,8 @@
  },
  "bolt": {
    "workspaces": [
-      "packages/*"
+      "packages/*",
+      "cypress/projects/*"


@jesstelford I was thinking we would (when ready) do effectively this, but split into multiple test-project packages at some higher level and have cypress test them.

roughly:

"workspaces": [ "packages/*", "projects/*", ]

This way we can have multiple test projects set up demonstrating different features / configurations / etc

Do you think it's a good idea to also split on projects used explicitly for cypress, vs. those not under test? I'm not sure about not having the other test projects not automatically tested. I worry that'll lead to them diverging, or work being done against a "main" test project that's not caught by tests, etc.

What if we did came at this the other way around? Created multiple test projects at the top level, actually included the test scripts with them (and imported from the cypress directory) so we are colocating the code currently under cypress/integration with the test project?

Not sure which of these approaches is more scalable or likely to keep us on the right track long term.

"workspaces": [ "packages/*", "projects/*", ]

This way we can have multiple test projects set up demonstrating different features / configurations / etc

Great idea. Done.

Do you think it's a good idea to also split on projects used explicitly for cypress, vs. those not under test? I'm not sure about not having the other test projects not automatically tested. I worry that'll lead to them diverging, or work being done against a "main" test project that's not caught by tests, etc.

It's a great point. We should definitely have tests covering each of the projects, so they never get out of sync. I'll make sure there are at least basic tests for each.

Created multiple test projects at the top level, actually included the test scripts with them (and imported from the cypress directory)

I don't think Cypress allows this - they're oddly strict in the folder structure of where tests are located, etc. But if it's possible, it'd be great!

dominikwilkowski · 2018-07-03T23:51:26Z

Yes that's totally right. After a brief discussion with @jesstelford I think it's best to prefix each env var so that it clearly corresponds to each project. In CI as well as in the .env files.
I suggest something like:

project-basic-CLOUDINARY_CLOUD_NAME = abc
project-basic-CLOUDINARY_KEY = abc
project-basic-CLOUDINARY_SECRET = abc
project-basic-PORT = 123

project-auth-CLOUDINARY_CLOUD_NAME = abc
project-auth-CLOUDINARY_KEY = abc
project-auth-CLOUDINARY_SECRET = abc
project-auth-PORT = 123

project-twitter-CLOUDINARY_CLOUD_NAME = abc
project-twitter-CLOUDINARY_KEY = abc
project-twitter-CLOUDINARY_SECRET = abc
project-twitter-PORT = 123

Explicit over implicit and if it ever gets too verbose we can still add some magic.

jesstelford · 2018-07-04T22:20:02Z

So we've gone with number 5 from above: Running cypress tests in parallel and from their project directories.

There are 2 caveats:

When running locally, the output is garbage (multiple tests outputting at once).d
When run on CircleCI, our account only has 1x parallelism, so while it's all setup to run in parallel, in practice it still runs serially (~5:00min vs ~2:40).

jesstelford · 2018-07-04T22:20:35Z

.circleci/config.yml

@@ -4,88 +4,116 @@
 #
 version: 2

+x-job-setup: &job-setup


This is like a "default" config that is imported lower down in the .yml file, so we don't have to repeat ourselves.

jesstelford · 2018-07-04T22:20:58Z

.circleci/config.yml

+x-job-setup: &job-setup
+  docker:
+    # specify the version you desire here
+    - image: circleci/node:carbon-browsers


The -browsers version removes the need for apt-get install later, reducing build time by about 22seconds per test run.

jesstelford · 2018-07-04T23:09:29Z

.circleci/config.yml

+          name: Login E2E tests
+          command: CYPRESS_CACHE_FOLDER=$CIRCLE_WORKING_DIRECTORY/node_modules/cypress/.cache/ bolt ws run cypress:run:ci --only-fs projects/login
+          environment:
+            PORT: 4000


NOTE: Thanks to running these in seperate processes, the env vars no longer clash, so prefixing is not necessary.

jesstelford · 2018-07-04T23:10:32Z

package.json

@@ -5,8 +5,8 @@
  "license": "MIT",
  "scripts": {
    "coverage": "jest --coverage --collectCoverageFrom=packages/**/*.{js}",
-    "cypress:open": "cypress open",
-    "cypress:run": "cypress run",
+    "cypress:run": "bolt ws run cypress:run",


Used only for local testing.

Should it then rather use bolt ws run cypress:open or will this break?

jesstelford · 2018-07-04T23:24:17Z

projects/basic/cypress/integration/add-edit-delete-data.js

@@ -1,24 +1,22 @@
 describe('Adding data', () => {
  [
    {
-      url: 'http://localhost:3000/admin/users',
+      url: '/admin/users',


By setting the baseUrl in cypress/plugins/index.js, we don't need to specify the entire URL for cy.X commands, avoiding having to read from env vars too.

jesstelford · 2018-07-04T23:26:33Z

projects/basic/cypress/support/commands.js

-Cypress.Commands.add('graphql_query', query =>
-  apolloFetch({ query })
+Cypress.Commands.add('graphql_query', (uri, query) =>
+  getApollo(uri)({ query })


Changed to explicitly pass in the graphql endpoint, enabling future use of querying different endpoints as required.

jesstelford · 2018-07-04T23:26:52Z

projects/basic/cypress/support/commands.js


-const GRAPHQL_URI = 'http://localhost:3000/admin/api';
-const apolloFetch = createApolloFetch({ uri: GRAPHQL_URI });
+const getApollo = memoize(function createApollo(uri) {


Only want to create a single instance per URI, so we memoize based on that param.

jesstelford · 2018-07-04T23:27:55Z

projects/basic/package.json

+    "prepare-test-server": "node -r dotenv-safe/config -e 'require(`execa`)(`start-server-and-test`, [`start`, `http-get://localhost:${process.env.PORT}/admin`, process.argv[1]], { stdio: `inherit` }).catch(error => { console.error(error.toString()); process.exit(error.code) })'",
+    "cypress:run:ci": "yarn prepare-test-server cypress:run:cmd:ci",
+    "cypress:run": "if [ -f .env ]; then yarn prepare-test-server cypress:run:cmd; else echo \"\nError: Must create a projects/basic/.env file.\nSee projects/basic/.env.example for values\n\"; exit 1; fi",
+    "cypress:open": "if [ -f .env ]; then yarn prepare-test-server cypress:open:cmd; else echo \"\nError: Must create a projects/basic/.env file.\nSee projects/basic/.env.example for values\n\"; exit 1; fi"


These commands are repeated in each project. They're of medium complexity, and favour repetition over more scripts given the feedback on the original PR.

Note also there is a friendly error output for first time test run (when .env hasn't been setup).

jesstelford · 2018-07-04T23:28:55Z

projects/twitter-login/package.json

@@ -8,7 +8,8 @@
    "node": ">=8.4.0"
  },
  "scripts": {
-    "start": "node -r dotenv-safe/config index.js"
+    "start": "node -r dotenv-safe/config index.js",
+    "cypress:run:ci": "exit 0"


No tests setup yet for this project, so exit with success.

jesstelford · 2018-07-04T23:45:37Z

☝️ There's some inline comments explaining things that are going on.

I recommend we do a squash and merge for this, the commits got a bit crazy :P

timleslie · 2018-07-05T00:08:12Z

This all looks good to me. I particularly like that the integration tests are explicitly colocated with the project they test. Hopefully this pattern will be valuable to developers building their own projects.

There are two follow up tasks which we should look into after this task goes through:

Provide a mechanism for running the cypress tests locally in some meaningful way (and update the top level README.md to reflect this)
Invest in some more circle CI containers to allow for faster turnaround of tests. This will also give us access to the "Insights" tab in circle CI which will... give us more insights...

jesstelford · 2018-07-05T00:29:04Z

Invest in some more circle CI containers to allow for faster turnaround of tests. This will also give us access to the "Insights" tab in circle CI which will... give us more insights...

🤔 ... insightful!

Provide a mechanism for running the cypress tests locally in some meaningful way (and update the top level README.md to reflect this)

This mechanism already exists, just the output on the CLI is confusing (you'll still see failures though).

I'll update the README.md now.

edit @timleslie see commit 4f7aad0 for updated docs.

dominikwilkowski · 2018-07-05T02:25:57Z

Looks great, LGTM

jesstelford force-pushed the cypress-projects branch from d3f2193 to e5a30af Compare June 23, 2018 03:02

jesstelford force-pushed the cypress-projects branch from 3cc76b7 to c2bb8a4 Compare June 24, 2018 07:33

jesstelford commented Jun 24, 2018

View reviewed changes

JedWatson reviewed Jun 25, 2018

View reviewed changes

jesstelford force-pushed the cypress-projects branch 7 times, most recently from 7bec564 to 1904631 Compare June 29, 2018 05:10

jesstelford added 15 commits July 3, 2018 09:01

Move cypress test project to own folder

a6f556d

Lint fixes

cbf04a3

Update cache step in circle config

7a80e22

Use circle config from master

3485565

Use circle config from master

fb1fa53

Tweak circle config

ecfe157

Working tests after master merge

eff8b21

Remove auth config from basic cypress project

b0da15e

Multiple cypress projects / tests

c338d0a

Specify tests to be run against 'basic' project

65d1fbb

Add a login test

4258773

Use published version of start-server-and-test

1a2bc45

Move projects to root directory for consistency

48f1884

Move test project to a twitter specific project

067e671

Correctly load env vars on CI for multi-project

4e28d9b

jesstelford force-pushed the cypress-projects branch 3 times, most recently from 61a1de6 to e659c66 Compare July 4, 2018 22:01

jesstelford added 2 commits July 5, 2018 08:07

Trade complexity for repitition

d03fd74

Use prebuilt image to speed up CI builds

50a62ef

jesstelford force-pushed the cypress-projects branch from e659c66 to 50a62ef Compare July 4, 2018 22:09

jesstelford commented Jul 4, 2018

View reviewed changes

jesstelford added 3 commits July 5, 2018 08:53

Better error output from test failure

bc68593

Remove unnecessary env var prefixes

6f5ed17

Better DX for first time test runs

a1f058e

jesstelford commented Jul 4, 2018

View reviewed changes

jesstelford added 2 commits July 5, 2018 09:29

Cleanup after refactor

2885fa1

Use correct screenshot/video directory for cypress artifacts

cce2e20

timleslie mentioned this pull request Jul 5, 2018

Add Checkbox (boolean) field type #147

Merged

Documentation / global scripts cleanup

4f7aad0

jesstelford merged commit 84287f2 into master Jul 5, 2018

jesstelford deleted the cypress-projects branch July 5, 2018 05:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move cypress test project to own folder #134

Move cypress test project to own folder #134

jesstelford commented Jun 23, 2018 •

edited

Loading

dominikwilkowski commented Jun 23, 2018

jesstelford commented Jun 23, 2018

jesstelford commented Jun 24, 2018

jesstelford Jun 24, 2018

jesstelford Jun 24, 2018

JedWatson Jun 25, 2018

jesstelford Jun 28, 2018

dominikwilkowski commented Jul 3, 2018

jesstelford commented Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018 •

edited

Loading

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

dominikwilkowski Jul 5, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford Jul 4, 2018

jesstelford commented Jul 4, 2018

timleslie commented Jul 5, 2018

jesstelford commented Jul 5, 2018 •

edited

Loading

dominikwilkowski commented Jul 5, 2018

Move cypress test project to own folder #134

Move cypress test project to own folder #134

Conversation

jesstelford commented Jun 23, 2018 • edited Loading

dominikwilkowski commented Jun 23, 2018

jesstelford commented Jun 23, 2018

jesstelford commented Jun 24, 2018

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

dominikwilkowski commented Jul 3, 2018

jesstelford commented Jul 4, 2018

Choose a reason for hiding this comment

jesstelford Jul 4, 2018 • 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

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

jesstelford commented Jul 4, 2018

timleslie commented Jul 5, 2018

jesstelford commented Jul 5, 2018 • edited Loading

dominikwilkowski commented Jul 5, 2018

jesstelford commented Jun 23, 2018 •

edited

Loading

jesstelford Jul 4, 2018 •

edited

Loading

jesstelford commented Jul 5, 2018 •

edited

Loading