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

Create GitHub Action to Run Cypress #112

Merged
merged 5 commits into from
Dec 22, 2021
Merged

Create GitHub Action to Run Cypress #112

merged 5 commits into from
Dec 22, 2021

Conversation

BCerki
Copy link
Collaborator

@BCerki BCerki commented Dec 22, 2021

This PR addresses issue:#101

Includes tests? y
Rebased on main address to conflicts? y
Updated docs? y

Proposed changes:

  • Add a github action that runs the cypress test

Additional notes:

@BCerki BCerki changed the title 101 cypress ga Create GitHub Action to Run Cypress Dec 22, 2021
Comment on lines +8 to +10
COOKIE_SECRET: "foobar"
MAILER_HOST: "foobar"
MAILER_PORT: "321"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The app wouldn't start unless I had these filled out, but these placeholders seem to work

Choose a reason for hiding this comment

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

Tech debt: let's run something like Sendria so these configured values are real within the testing environment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@BCerki BCerki marked this pull request as ready for review December 22, 2021 18:00
Copy link

@wenzowski wenzowski left a comment

Choose a reason for hiding this comment

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

I don't think the daemonization was intended. Let's fix that, document tech debt as new issues, and ship this

Comment on lines +8 to +10
COOKIE_SECRET: "foobar"
MAILER_HOST: "foobar"
MAILER_PORT: "321"

Choose a reason for hiding this comment

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

Tech debt: let's run something like Sendria so these configured values are real within the testing environment.

package.json Outdated Show resolved Hide resolved
Co-authored-by: Alec Wenzowski <alec@button.is>
Copy link

@wenzowski wenzowski left a comment

Choose a reason for hiding this comment

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

Regarding the Additional Notes:
Let's make sure we've got a tech debt card for our CI/CD process that notes our e2e tests should be booting the candidate docker image that we intend to release to production (as a 12 factor app, we should be able to boot the prod image with development or test environment flag set). The cypress errors appear to be due to the fact that we're running headless from inside a runner that does not have access to a gpu. There's probably a way to squelch these, so that should be tracked as tech debt as well. The npm run browserslist --update-db call should probably also be part of our ci config so I'd track that as separate tech debt.

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.

None yet

2 participants