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

Split tests into four groups for CI job matrices #872

Merged
merged 20 commits into from
Aug 27, 2020
Merged

Conversation

ali-ramadhan
Copy link
Member

@ali-ramadhan ali-ramadhan commented Aug 27, 2020

This PR splits the tests into four test groups, selectable with the TEST_GROUP environment variable: unit, integration, regression, and scripts. Using TEST_GROUP=all (the default if TEST_GROUP is not defined) will run all tests.

The purpose of this PR is to address long test build times (see #860) that time out at 50 minutes on Travis CI and at 60 minutes on GitLab CI by splitting tests into multiple jobs (i.e. building a job matrix) that each should individually run much faster.

If the different jobs could be run in parallel this would speed up our test builds significantly, but alas we are stuck with free-tier CI pipelines so we can't run too many jobs in parallel and will probably end up waiting longer. But at least our jobs won't time out.

In working on this PR I was also able to revive the Appveyor and Docker CI pipelines (I think Appveyor will only show up for future PRs).

Resolves #139

@codecov
Copy link

codecov bot commented Aug 27, 2020

Codecov Report

Merging #872 into master will decrease coverage by 1.40%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #872      +/-   ##
==========================================
- Coverage   72.50%   71.09%   -1.41%     
==========================================
  Files         187      187              
  Lines        5331     5217     -114     
==========================================
- Hits         3865     3709     -156     
- Misses       1466     1508      +42     
Impacted Files Coverage Δ
src/AbstractOperations/AbstractOperations.jl 33.33% <0.00%> (-33.34%) ⬇️
src/AbstractOperations/interpolation_utils.jl 63.33% <0.00%> (-30.01%) ⬇️
src/Oceananigans.jl 75.00% <0.00%> (-25.00%) ⬇️
src/Solvers/batched_tridiagonal_solver.jl 60.71% <0.00%> (-24.29%) ⬇️
src/Forcing/simple_forcing.jl 81.81% <0.00%> (-18.19%) ⬇️
test/runtests_utils.jl 50.00% <0.00%> (-17.57%) ⬇️
src/Utils/versioninfo.jl 68.75% <0.00%> (-16.97%) ⬇️
...closure_implementations/anisotropic_diffusivity.jl 84.00% <0.00%> (-16.00%) ⬇️
src/Buoyancy/Buoyancy.jl 63.15% <0.00%> (-15.79%) ⬇️
src/TurbulenceClosures/closure_operators.jl 86.20% <0.00%> (-13.80%) ⬇️
... and 60 more

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 424051c...547455c. Read the comment docs.

@ali-ramadhan
Copy link
Member Author

Alright I think our CI is back online: Travis, GitLab, Appveyor, and Docker CI all work again. And I think CodeCov should be more lenient about failing PRs now so we won't get red crosses on most PRs anymore.

Tests will take longer to run now though.

@ali-ramadhan
Copy link
Member Author

Found a trick to configure Travis CI to build pull requests but only pushes to master (thus reducing redundancy as default Travis CI behavior is to build both pushes and merge commits on PRs).

See: https://stackoverflow.com/a/31882307

@glwagner
Copy link
Member

Successful in 65m...

@ali-ramadhan ali-ramadhan merged commit 46608d7 into master Aug 27, 2020
@ali-ramadhan ali-ramadhan deleted the ar/split-tests branch August 27, 2020 20:34
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.

We need to upgrade our testing infrastructure soon-ish.
2 participants