Switch e2e tests over to cypress #183

tw-paraxial · 2018-04-17T08:33:24Z

E2E tests now use a new seeder, and run in a dedicated container running cypress.
E2E test coverage to be improved later - this is initial set up.

…w seeder in e2e tests

Added very basic first test. Updated README to reflect new usage.

…er-compose config

…tainer.

camjackson

Thanks for this PR, I'm excited to get it in master!

There are a few things I'd like to see done differently before merging, mostly to do with Docker things and scripting things and how we kind of orchestrate tasks locally.

See my comments below for details, and let me know if anything I said doesn't make sense :)

camjackson · 2018-04-17T14:16:36Z

backend/bin/e2e-seed-new.js

+
+const createDefaultBranch = () => {
+  console.log('Creating default branch.');
+  const req = { body: { id: branchId, name: 'A branch' } };


Looking at the branchesController, I'm pretty sure that the id you pass in here is going to be ignored - the controller generates an ID for you, so you don't need to pass one in. That means you can delete lines 16 & 17 too.

camjackson · 2018-04-17T14:19:00Z

backend/bin/e2e-seed-new.js

+  .catch(error => {
+    console.error('Seed failed:', error);
+    process.exit(1);
+  });


The diff between this file and seed.js is tiny, so we've got a lot of copy-paste here. I think that's fine for a first cut while we're still working all this stuff out, but at some point it might be good for the two files to either: 1) converge and be parameterised somehow, or 2) have common stuff pulled out into a seed-utils.js or something. I wouldn't do it yet but just something to think about.

Also I think we can go ahead and just call this file e2e-seed.js, rather than -new. The old file contents are obsolete anyway, so no need to keep them around!

In short: Yes to all your points. We should really take this out of the merge request. But in order to first test the e2e tests setup with cyprus to run a hello world in CI we need them to compile. Hence we raised this request early against the branch.

e2e-see-new.js is a pure copy and paste as it is very early stages of work in progress. We are using the seed as a reference and adopt from there. Refactoring needed for sure once we can add the e2e dependencies.

camjackson · 2018-04-17T14:20:19Z

backend/package.json

    "seed": "node bin/seed.js && node bin/test-seed.js",
-    "start": "node index.js"
+    "start": "node index.js",
+    "start-test": "NODE_ENV=test node index.js"


I'm hesitant to add more scripts to the package.json, as it's already a bit out of control. Can we just do NODE_ENV=test yarn start anywhere we need this?

Personal preference - I see what you mean, but, this one feels like a useful shorthand that would be used relatively commonly, and also serves as documentation for future devs.

camjackson · 2018-04-17T14:32:39Z

docker-compose.yml

    build:
      context: docker/
-      dockerfile: Dockerfile.dev
+      dockerfile: Dockerfile.prod


What was the reason for renaming this file? We only use this particular Dockerfile in development, not for building production images.

I may have misunderstood the way that the script in bin/docker-build-and-deploy.sh works, then - I had thought that it recreated the rabblerouser/core image from the local image. I may have gotten a bit lost in the architecture, though.

camjackson · 2018-04-17T14:33:41Z

docker-compose.yml

      TEST_STREAM_NAME: test_rabblerouser_stream
    command: sleep 2147483647 # Sleep forever, we'll shell into the container later
    stdin_open: true
-    user: $UID


Is there a reason you removed this? IIRC, we need this to make it run properly on a Linux host, as discovered by @akjones.

Ah, interesting.
I found that it threw potentially confusing warnings (as $UID is not an env variable on OSX), and was apparently redundant given the user creation in the dockerfile itself.
(If only docker was as portable as it claimed to be). Can re-add since there's a need.

camjackson · 2018-04-17T14:35:10Z

e2e/cypress/fixtures/profile.json

+  "id": 8739,
+  "name": "Jane",
+  "email": "jane@example.com"
+}


Do we need this file or can we delete it?

camjackson · 2018-04-17T14:35:38Z

e2e/cypress/fixtures/users.json

+      "bs": "target end-to-end models"
+    }
+  }
+]


Do we need this file or can we delete it?

camjackson · 2018-04-17T14:37:46Z

.travis.yml

-script: ./go.sh yarn test
+script:
+  - ./go.sh yarn test
+  - ./go.sh e2e


See comment right at the end, but I think it might be better to just directly do ./bin/e2e.sh as the second command here.

camjackson · 2018-04-17T14:39:13Z

go.sh

+then
+  ./bin/e2e.sh
+  exit
+fi


Your comment above is correct that this is quite a different path/use case than the other things that we use go.sh for. With that in mind, I think it might better for it to just stay as a separate thing that can be run on its own, rather than making it a 'special case' of the go script.

Keen to revert this change, then.

camjackson · 2018-04-17T14:41:05Z

go.sh

 if [[ -z $1 ]];
 then
-  docker-compose exec core /bin/sh
+  docker-compose exec core /bin/bash


Did you test this change? I think you'll find that bash is not present in Alpine linux, which is what the core image is built from. So it needs to be sh.

I checked this change against the alpine image and it seemed fine - I suppose I can revert it.

edit My environment was unclean! Good catch.

cypress tests.

camjackson · 2018-05-29T06:15:09Z

Hey @tw-paraxial I'm going to take another look at this one. Just wanted to check where it's at from your point of view?

I.e., the original PR was kind of just a starting point, right? Is this still a work in progress, or is it ready to be merged as far as you're concerned?

This reverts commit 07e0795.

Sebastian Henn and others added 6 commits April 11, 2018 16:40

Duplicate the e2e seeder from seed.js using event-streams; use the ne…

3f8af28

…w seeder in e2e tests

[Amelyn/Para] Added cypress module...

6556833

Added very basic first test. Updated README to reflect new usage.

Updated travis config; Changed spin up script; Removed user from dock…

b7cfbd6

…er-compose config

Reverted changes to the prod dockerfile. e2e tests now run in own con…

a3a9871

…tainer.

Edited go script to also include e2e test runner

d363e03

Correcting minor typos

8ba0c1c

camjackson self-requested a review April 17, 2018 14:13

camjackson requested changes Apr 17, 2018

View reviewed changes

paraxial and others added 14 commits April 18, 2018 13:11

[Alice/Amelyn/Para] Addressed some comments for PR 183

98ec8dc

Fix script command typo.

55650c9

Make log messages appear for core_test_1 and core_core_1

038eab5

add scripts to install dependencies needed to run e2e test.

3d93a3d

Convert Admin with invalid credential e2e test from casperjs to cypress.

a1989da

Fix issue with not being able to login when running e2e tests.

4ecceab

Add more login tests.

e7c4a55

Remove tests temporarily.

07e0795

Add config to differentiate between localhost docker container for

0886ca0

cypress tests.

Add script to run cypress tests on localhost

4aa3c15

Minor changes to main README.md

9639d68

Update readme for end to end tests

2ad225c

Update e2e readme

cdba448

Improve wording of e2e README.md

aab7ff4

Revert "Remove tests temporarily."

5ac6433

This reverts commit 07e0795.

Switch e2e tests over to cypress #183

Are you sure you want to change the base?

Switch e2e tests over to cypress #183

Uh oh!

Conversation

tw-paraxial commented Apr 17, 2018

Uh oh!

camjackson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camjackson Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camjackson Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

torstenleibrich Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tw-paraxial Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camjackson commented May 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

camjackson left a comment •

edited

Loading

camjackson Apr 17, 2018 •

edited

Loading

camjackson Apr 17, 2018 •

edited

Loading

torstenleibrich Apr 18, 2018 •

edited

Loading

tw-paraxial Apr 18, 2018 •

edited

Loading