search

CorrelAid / correlaid_website

Source code for the CorrelAid website
https://correlaid.org
3 stars 0 forks source link

Add (end to end) testing to CD action #281

Closed jstet closed 11 months ago

jstet commented 12 months ago

https://github.com/CorrelAid/correlaid_website/blob/main/.github/workflows/cd.yaml will be triggered either manually or periodically by a directus workflow.

KonradUdoHannes commented 11 months ago

@jstet I would like to set this up, but need a little more information on how the vercel deployment looks for this/what is needed to integrate it in the vercel deployment. I assume that currently two things happen on vercel site (either directly or indicrectly).

  1. npm install
  2. npm run build

In order to implement this issue we need the following three steps instead

  1. npm install
  2. npx playwright install --with-deps
  3. npm run build-and-test

What do we need to get there?

Also as a practical issue: In case the build step fails due to something not related to the source code (for instance cms issues). What does it take to simply retry an existing commit, without having to artificially creating a new commit on github.

jstet commented 11 months ago

Yes npm install is the install command and npm run build is the build command.

Couldn't we also add the playwright testing to the github action? Maybe we could separate some tests into different actions to allow for concurrent execution.

If doing this with vercels, we could just set npx playwright install --with-deps && npm run build-and-test as the build command.

KonradUdoHannes commented 11 months ago

Yes && should work, our we could also combine it into a single project script if that helps.

Generally we can also set it up in CI with github actions, but this is more of a process design question in my view.

Running e2e testing in CI means that CI will fail if there are problems with directus. This is in contrast to the unit tests, which do not depend on directus. If e2e is included it means that if directus is down no PRs can be done as well. But in the normal scenario one gets feedback a little sooner of whether the build works (even though it can also be run locally). If e2e is not included PRs can always be done but we might notice during CD that there is an issue that was not caught by unit tests.

I think both approaches can make sense and it depends a bit on how many PRs one expects, how much issues with directus and how many occasional contributors (where it helps the most to see that their PRs pass all checks). In our case I don't we have any constraint that puts us clearly on one side, so we could also try and see how it goes.

jstet commented 11 months ago

we could also include e2e into an action that is triggered when a push to main happens and deactivate the automatic build so that a deploy hook is only called when e2e succeeds. that way prs are possible without e2e suceeding. the same action could be used for triggering a build from the directus ui.

KonradUdoHannes commented 11 months ago

That sounds like a good idea to me :+1:.

KonradUdoHannes commented 11 months ago

I've added the e2e testing to our CD action in #425 but did not modify the the triggers yet. Manual workflow dispatch worked.

In accordance with the earlier discussion I would suggest to disable vercels automatic builds for main and add pr to main as a trigger to the workflow, such that all deployments run through the CD workflow. @jstet as I don't have access to the vercel configuration, do we want to coordinate this or do you want to do these last steps?

jstet commented 11 months ago

I need to ask @Frie regarding giving you access and they are on vacation right now

jstet commented 11 months ago

Finished implementing this in the pr mentioned above