search

corona-warn-app / cwa-website

Corona-Warn-App website. The CWA development ended on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
522 stars 225 forks source link

Update build-and-test.yml | Run tests in parallel #3389

Closed larswmh closed 1 year ago

larswmh commented 1 year ago

We want to edit the build-and-test workflow, which is triggered on pull requests, so that it first works on the required build job. Once that required job succeeded, the optional jobs (one for each test using matrix) will start by making use of the build done in the previous job to save time. Not only would this be an improvement in total time needed to complete all tests, as they would be performed in parallel instead of back to back, but it would also give users a better understanding on what specific test went wrong on the first look.

image

In my current draft I linked above, every test builds the page again. This is a problem that is also observable in the build-and-test that is currently in use on master here, as we also couldn't find a proper solution for the problem described above.

Although GitHub provides these resources for free on open source repositories, we still want to use them responsibly. Going ahead with the draft I currently have wouldn't meet these standards for us, but we're still looking out for a solution for this.

Internal Tracking ID: EXPOSUREAPP-14831

larswmh commented 1 year ago

@MikeMcC399 brought https://github.com/actions/upload-artifact & https://github.com/actions/download-artifact to my attention, which I will test in this soon. Depending on the time needed to upload / download, this could be a sufficient solution. The documentation for this can be found here.

In addition, I've remove the long check_links test for now.

MikeMcC399 commented 1 year ago

@larswmh

larswmh commented 1 year ago

Uploading and downloading an artifact seems to be a good solution once good exclusions are figured out, as it would take very long without (see manually cancelled check). One test fails because of a path I excluded where I thought the content wouldn't be used in tests (see check_videos)

Some test still seem to fail without giving much information about the cause, for example in faq_link_attr or check_anchor_links

MikeMcC399 commented 1 year ago

@larswmh

I see you made some good progress! Well done!

You managed to workaround the very large size of the web by excluding the big files.

One test fails because of a path I excluded where I thought the content wouldn't be used in tests (see check_videos)

I suggest to move any science video files to the specifc video directory /src/assets/video. All the blog videos area hosted there, so the science videos should go there as well. That would allow the test to run.

Some test still seem to fail without giving much information about the cause, for example in faq_link_attr or check_anchor_links

Those jobs were cancelled because the video job failed. To stop that happening, you need to add if: always() I believe. see below for fail-fast.

larswmh commented 1 year ago

@MikeMcC399

I suggest to move any science video files to the specifc video directory /src/assets/video. All the blog videos area hosted there, so the science videos should go there as well. That would allow the test to run.

I've opened up a PR for your suggestion in #3400

MikeMcC399 commented 1 year ago

Sorry, the second part was bad advice:

https://docs.github.com/en/actions/examples/using-concurrency-expressions-and-a-test-matrix

"Setting fail-fast to false prevents GitHub from cancelling all in-progress jobs if any matrix job fails."

larswmh commented 1 year ago

Thanks for the advice @MikeMcC399. As expected, all tests are passing. This PR is now ready for review.