WIP: migrate to GitHub actions

[ranocha]

Closes #286

TODO:

• We should update .codecov.yml to account for the increased number of parallel builds
• Check whether coveralls works with these parallel builds
• Communicate: [skip ci] does not work anymore in PRs

[codecov]

Codecov Report

Merging #288 into master will increase coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
+ Coverage   91.36%   91.49%   +0.13%     
==========================================
  Files          42       42              
  Lines        8867     8867              
==========================================
+ Hits         8101     8113      +12     
+ Misses        766      754      -12     

Flag	Coverage Δ
unittests	91.49% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/mesh/parallel.jl	96.10% <0.00%> (+1.29%)	⬆️
src/solvers/dg/2d/parallel.jl	93.77% <0.00%> (+3.60%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d8401f...d2f4c4e. Read the comment docs.

[ranocha]

I've migrated our CI infrastructure completely to GitHub actions to replace Travis CI. Using the current setup, we run all tests automatically on Linux, Mac OS, and Windows. Since we're allowed to run more jobs in parallel than on Travis, this doesn't really increase the CI time. The only downside is that [skip ci] doesn't work anymore in PRs, but I can live with that. In the end, this could also be a feature 😉

To sum up, I'm quite satisfied with this setup. If you all agree (especially @sloede), I would like to merge this, get rid of the remaining Travis CI setup in our repo settings, and propagate this change to all open PRs.

[sloede]

This is great work - thanks for taking care of this for so quickly, @ranocha!

I've left a few comments/questions. Also, I'd like to propose to revisit the decision to immediately retire Travis. Would it be possible to just have both enabled, at least while working on Taal? That we we can be confident that there are no changes in the test results due to the new CI setup, and if there are, we have a better chance of detecting/fixing them. What do you think?

[ranocha]

Also, I'd like to propose to revisit the decision to immediately retire Travis.

I've re-added Travis. I've opened #289 to remember removing Appveyor and Travis when Taal is alive.

[ranocha]

Can we merge this, @sloede?

[sloede]

If the final comment is resolved, I think this is good to merge.

[efaulhaber]

@ranocha

The only downside is that [skip ci] doesn't work anymore in PRs, but I can live with that.

I didn't test it, but something like this should work:

if: !(github.event_name == 'pull_request' && contains(github.event.pull_request.title, '[skip ci]'))

[ranocha]

Looking at https://docs.github.com/en/free-pro-team@latest/rest/reference/pulls#create-a-pull-request--parameters, I get the impression that this checks only the title of the PR, i.e. WIP: migrate to GitHub actions for this one. In contrast, we usually want to annotate a single commit to not trigger CI runs. That's possible for push events using the check

if: "!contains(github.event.head_commit.message, 'skip ci')"

in our ci.yml. However, the pull_request event type doesn't seem to support this directly. I guess it should be possible to get the latest commit in a PR, too, using something like https://docs.github.com/en/free-pro-team@latest/rest/reference/pulls#list-commits-on-a-pull-request. Then, it should be possible to get the message of the latest commit and check whether it contains skip ci.

[efaulhaber]

I don't understand exactly what you mean. Your code snipped should check the commit that triggered the action. So the last commit in a PR decides if a CI run should be triggered.

[ranocha]

Yeah, but that code works only on push events, not on pull_request events. For example, look at the latest commit in #320: The commit message contains [skip ci] but our GitHub CI action was still triggered - since it is a pull_request event. In contrast, the latest commit to https://github.com/trixi-framework/Trixi2Vtk.jl contains the same label in the commit message and CI was skipped accordingly - since it is a push event, i.e. a push to master in this case. If I hadn't included skip ci in the commit message, CI would have run there, too.

[efaulhaber]

Ah, I see. We could have a look at this project: https://github.com/marketplace/actions/ci-skip-action

[ranocha]

Looks good at first sight. Do you want to give it a shot and make a PR?

[efaulhaber]

Yeah, sounds good.

[efaulhaber]

Okay, it seems like there is no elegant way to do this. One option would be to write if: ${{ env.CI_SKIP == 'false' }} on every step, the other would be to let the job fail if the commit contains [skip ci], but this would mark the action as failed in GitHub.

[ranocha]

In that case, I'm okay with keeping the current setup. What do the other think, in particular @sloede?

In some sense, we can also argue that it's a feature, not a bug 😉 In a worst case scenario, we can also cancel CI runs manually. Otherwise, it's mostly some discipline regarding commits and pushes to PRs (like "Don't push any tiny commit immediately but wait for some minutes to gather more of them"). Additionally, a lot of possibilities where skip ci is useful should be covered by our settings of paths-ignore.

However, I don't have a strong opinion here. If other feel like we should add the check

if: ${{ env.CI_SKIP == 'false' }}

to every step, that's okay for me.

[sloede]

I think the strongest reason for allowing CI skip is to save time when only documentation has changed. Since this should be handled by paths-ignore, I am also ok with keeping the current setup.