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

Speed up CI tests #42

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Conversation

patrickersing
Copy link
Contributor

@patrickersing patrickersing commented Apr 29, 2024

With the additional features being added having a single CI job became very slow.
This PR should speed up the CI time by splitting the tests into multiple jobs. Furthermore, macOS and windows tests will run without coverage and become restricted to the upstream testset.

@andrewwinters5000
Copy link
Member

With the additional features being added having a single CI job became very slow. This PR should speed up the CI time by splitting the tests into multiple jobs and also restricts the coverage testing to ubuntu.

This is a good idea to make the CI faster. Should we already drop back the macOS version here as discussed in #40?

Copy link

codecov bot commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.22%. Comparing base (71bba1e) to head (96962a8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #42   +/-   ##
=======================================
  Coverage   99.22%   99.22%           
=======================================
  Files          45       45           
  Lines        1680     1680           
=======================================
  Hits         1667     1667           
  Misses         13       13           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@patrickersing
Copy link
Contributor Author

With the additional features being added having a single CI job became very slow. This PR should speed up the CI time by splitting the tests into multiple jobs and also restricts the coverage testing to ubuntu.

This is a good idea to make the CI faster. Should we already drop back the macOS version here as discussed in #40?

I think this will not be necessary, since in this setup we would no longer do coverage testing on macOS. Since the problem in #40 only appeared in the coverage tests I expect that this would fix the problem.

@andrewwinters5000
Copy link
Member

With the additional features being added having a single CI job became very slow. This PR should speed up the CI time by splitting the tests into multiple jobs and also restricts the coverage testing to ubuntu.

This is a good idea to make the CI faster. Should we already drop back the macOS version here as discussed in #40?

I think this will not be necessary, since in this setup we would no longer do coverage testing on macOS. Since the problem in #40 only appeared in the coverage tests I expect that this would fix the problem.

Ah, gotcha. Then this sounds doubly good if it provides a workaround for the issues encountered on Mac-ARM.

@patrickersing
Copy link
Contributor Author

This provides a good speedup from the single job tests that previously ran for over 30min.
For additional speed we could also separate the coverage tests into it's own job.
However, the checks become a bit crowded now as we test everything on macOS, windows and ubuntu.
@andrewwinters5000 What do you think about restricting the macOS and windows tests to the upstream testset, which should cover the most important features of TrixiSW?

@andrewwinters5000
Copy link
Member

This provides a good speedup from the single job tests that previously ran for over 30min. For additional speed we could also separate the coverage tests into it's own job. However, the checks become a bit crowded now as we test everything on macOS, windows and ubuntu. @andrewwinters5000 What do you think about restricting the macOS and windows tests to the upstream testset, which should cover the most important features of TrixiSW?

That might be for the best, especially because it avoids the need to adjust the tolerances for the wet/dry well-balancedness tests on macOS which I see are failing due to round-off differences.

@patrickersing
Copy link
Contributor Author

Okay, I have restricted macOS & windows testing to upstream and separated the coverage tests.

@patrickersing patrickersing changed the title [WIP] Speed up CI tests Speed up CI tests Apr 29, 2024
@patrickersing patrickersing marked this pull request as ready for review April 29, 2024 11:51
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for speeding up the CI testing. I think we need to change the branch protections to reflect the new structure instead of the tests like Julia 1.10 - macOS-latest - x64 - pull_request that no longer exist.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
@patrickersing
Copy link
Contributor Author

LGTM! Thanks for speeding up the CI testing. I think we need to change the branch protections to reflect the new structure instead of the tests like Julia 1.10 - macOS-latest - x64 - pull_request that no longer exist.

Yes, branch protection still needs to be updated. Can you take care of this? I don't have sufficient rights.

@patrickersing patrickersing merged commit b928469 into trixi-framework:main Apr 30, 2024
19 checks passed
@patrickersing patrickersing deleted the ci_update branch April 30, 2024 07:23
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.

3 participants