Skip to content

Adding Cypress container to run e2e tests#334

Merged
beekley merged 2 commits intomasterfrom
add-cypress-container
Mar 10, 2020
Merged

Adding Cypress container to run e2e tests#334
beekley merged 2 commits intomasterfrom
add-cypress-container

Conversation

@lllleonnnn
Copy link
Contributor

resolves #325

addresses #

Motivation and context

Adding a dockerized test harness for cypress

Screenshots

before after

What I did

  • dockerized app to run effectively alongside tests
  • added cypress container w/ example tests

@lllleonnnn lllleonnnn changed the title HTTPS redirect errors w/ e2e container loading Adding Cypress container to run e2e tests Feb 25, 2020
@beekley beekley mentioned this pull request Feb 25, 2020
Copy link
Contributor

@beekley beekley left a comment

Choose a reason for hiding this comment

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

Good work putting this together. I left a few small comments/questions. Something that might be good for a future PR is to set docker up for general dev use, not just for testing (e.g. by adding nodemon and updating the readme).

I had worked on dockerizing this too in #335 😆.

# note: inside e2e container, the network allows accessing
# "web" host under name "web"
# so "curl http://web" would return whatever the webserver
# in the "web" container is cooking
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should this be "webapp"?

ports:
- "8080:8080"
env_file:
- webapp.env
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the mongo container's setup points to .env. Do both containers use the same file?

"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "Gleb Bahmutov <gleb.bahmutov@gmail.com> (https://glebbahmutov.com/)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this?

COPY . .

EXPOSE 8080
CMD [ "npm", "start" ] No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use nodemon here? npm start might require us to restart the container every time the code changes.

version: '2'

services:
webapp:
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to add a depends_on mongo so we can turn all containers up together.

Copy link
Contributor

@beekley beekley left a comment

Choose a reason for hiding this comment

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

I’m going to merge this since it shouldn’t affect anything dev/prod use of the app. Then I'll work on the feedback from the comments in a new PR.

@beekley beekley merged commit 0b7d183 into master Mar 10, 2020
@beekley beekley deleted the add-cypress-container branch March 10, 2020 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install cypress for e2e tests

2 participants