Skip to content
This repository has been archived by the owner on Mar 1, 2023. It is now read-only.

Switch CI to use Github Actions, run tests in parallel and fast fail when possible #1234

Merged
merged 1 commit into from
Jun 13, 2020

Conversation

jakebolewski
Copy link
Contributor

  • Run subset on every push in parallel
  • Kill jobs that fail quickly
  • Kill stale jobs quickly upon new commits to a branch
  • Run all OS unit tests only on bors try / bors merge
  • Remove Azure Devops Pipelines

Description

A clear and concise description of the code with usage.

I have

  • Written and run all necessary tests with CLIMA by including tests/runtests.jl
  • Followed all necessary style guidelines and run julia .dev/climaformat.jl .
  • Updated the documentation to reflect changes from this PR.

For review by CLIMA developers

  • There are no open pull requests for this already
  • CLIMA developers with relevant expertise have been assigned to review this submission
  • The code conforms to the style guidelines and has consistent naming conventions. julia .dev/format.jl has been run in a separate commit.
  • This code does what it is technically intended to do (all numerics make sense physically and/or computationally)

@charleskawczynski
Copy link
Member

Looks awesome. We still need to configure bors dependence, right?

slurmci-test.toml Outdated Show resolved Hide resolved
@jkozdon
Copy link

jkozdon commented Jun 11, 2020

Looks awesome. We still need to configure bors dependence, right?

Do we need each github action job to have it's own name to work with bors?

See this discussion

Alternatively we can rename the jobs this way

@jakebolewski
Copy link
Contributor Author

I haven't configured bors yet as I didn't know how it worked with the status indicator. It seem like you can reference the job name so we'll just have to make those two unique instead of both being named test?

@jakebolewski
Copy link
Contributor Author

The other behavior would be to decide if we only want to run the tests for PR's and not for every push. I think this might be better default, if you need to test some behavior out you can create a draft / WIP PR.

@charleskawczynski
Copy link
Member

I haven't configured bors yet as I didn't know how it worked with the status indicator.

From what I tested here, the syntax of the status name depends on whether we're using a matrix job or not.

The other behavior would be to decide if we only want to run the tests for PR's and not for every push. I think this might be better default, if you need to test some behavior out you can create a draft / WIP PR.

IMO, every push seems unnecessary. Often I find myself doing what @kpamnany does (canceling docs/azure after pushing to not waste resources). It'd be really nice to have a way to trigger incremental tests. Could mergify somehow help?

@jakebolewski
Copy link
Contributor Author

if you push now, then its configured to cancel any previous jobs on that branch.

@jakebolewski
Copy link
Contributor Author

I'm guessing that is the desired behavior, automatically kill stale jobs only build the latest commit on every branch.

I guess my question is if these jobs should only be triggered for PR's and not every branch that get's pushed.

@jakebolewski
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 11, 2020
@bors
Copy link
Contributor

bors bot commented Jun 11, 2020

try

Build failed:

@jakebolewski
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 11, 2020
@jakebolewski
Copy link
Contributor Author

bors try-

@kpamnany
Copy link
Contributor

I guess my question is if these jobs should only be triggered for PR's and not every branch that get's pushed.

Currently, CI is not triggered for branch pushes and only runs when you open a PR for that branch. I think this behavior is fine.

@kpamnany
Copy link
Contributor

I think what Charlie was getting at was if pushes to PRs should always trigger full CI. If we can just do a Linux CI build for pushes to a PR branch and do the full CI on bors try or bors r+ that would perhaps be a good thing?

@jakebolewski
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 11, 2020
@jakebolewski jakebolewski changed the title WIP: Switch CI to use Github Actions, run tests in parallel and fast fail when possible Switch CI to use Github Actions, run tests in parallel and fast fail when possible Jun 11, 2020
@bors
Copy link
Contributor

bors bot commented Jun 11, 2020

try

Build failed:

@jakebolewski
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 11, 2020
@jakebolewski jakebolewski force-pushed the jcb/github_actions branch 3 times, most recently from b5686a8 to 06a54f9 Compare June 12, 2020 13:18
@jakebolewski
Copy link
Contributor Author

I think this is ready to go, it has / had the behavior that @kpamnany that you were describing :). I put a reasonable time limit on the jobs as well.

@kpamnany kpamnany added the CI Continuous Integration label Jun 12, 2020
@jakebolewski
Copy link
Contributor Author

@charleskawczynski I can add some documentation, where would be the best place to do so? (wiki, docs, etc.?)

Copy link
Contributor

@kpamnany kpamnany left a comment

Choose a reason for hiding this comment

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

So excited for this!

@charleskawczynski
Copy link
Member

@charleskawczynski I can add some documentation, where would be the best place to do so? (wiki, docs, etc.?)

Good question. The badge you can add to the README, this is what I've done in the past for a GHA badge. We could also add a blurb of how this works to the continuous integration section of the contributing docs. But wiki sounds good too. But I don't have strong feelings as to where it belongs (I do like the idea of the badges on the front page though!).

@jakebolewski jakebolewski force-pushed the jcb/github_actions branch 2 times, most recently from 5e34baf to b979b49 Compare June 12, 2020 15:26
@jakebolewski
Copy link
Contributor Author

Ok I added some docs describing the current CI checks to Contributing / continuous_integration section.

@jakebolewski
Copy link
Contributor Author

bors r+

@jakebolewski
Copy link
Contributor Author

lets see how this goes, I'm sure with some experience we'll need to make some tweaks to the config so don't hesitate to raise issues

bors bot added a commit that referenced this pull request Jun 12, 2020
1234: Switch CI to use Github Actions, run tests in parallel and fast fail when possible  r=jakebolewski a=jakebolewski

* Run subset on every push in parallel
* Kill jobs that fail quickly
* Kill stale jobs quickly upon new commits to a branch
* Run all OS unit tests only on bors try / bors merge
* Remove Azure Devops Pipelines

# Description

A clear and concise description of the code with usage.



Co-authored-by: jakebolewski <[email protected]>
Comment on lines +13 to +18
test-modules: ["Arrays,Atmos,Common,Diagnostics,Driver,InputOutput,Utilities",
"Numerics/DGMethods",
"Numerics/Mesh",
"Numerics/ODESolvers",
"Numerics/SystemSolvers",
"Ocean"]
Copy link
Member

Choose a reason for hiding this comment

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

this is super neat!

@bors
Copy link
Contributor

bors bot commented Jun 12, 2020

try

Timed out.

@jakebolewski
Copy link
Contributor Author

jakebolewski commented Jun 12, 2020

hmm all checks passed but ittlooks like bors was misconfigured to check the status of the github action matrix? It seems stuck in a running state.

@simonbyrne / @charleskawczynski any ideas?

bors.toml Outdated
@@ -1,5 +1,5 @@
status = [
"azure-ci",
"test-os",
Copy link

Choose a reason for hiding this comment

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

Suggested change
"test-os",
"test-os (ubuntu-latest)",
"test-os (windows-latest)",
"test-os (macos-latest)",

Copy link

Choose a reason for hiding this comment

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

I think it reads the names list on the left side of https://github.com/CliMA/ClimateMachine.jl/runs/766357298

@bors
Copy link
Contributor

bors bot commented Jun 12, 2020

Canceled.

@jakebolewski
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Jun 12, 2020
1199: Added single stack horizontal statistics and test r=charleskawczynski a=ilopezgp

Added horizontal statistics diagnostics for single stack driver and a single stack tutorial of the 3D Burgers equation. Modified plothelpers.jl to admit the xlabel as a keyword argument.

# Description

A clear and concise description of the code with usage.



1232: Add simple hydrostatic spin-down test for Ocean Model r=blallen a=blallen

# Description

This PR adds a simple test of the hydrostatic dynamics of the Ocean model. It is intended to be used as the basic test case when implementing the split-explicit time stepper. The analytic solution (and more) can be found in this overleaf: https://www.overleaf.com/project/5e9dfd2716ba2600018de266

Currently, I'm just testing that the solution is within 0.5% of the analytic solution after a day. If anyone has better ideas please suggest them. I'd also like to turn this into a Literature tutorial, so I don't think I want to go the full convergence testing route with this, altho it is possible. 



1234: Switch CI to use Github Actions, run tests in parallel and fast fail when possible  r=jakebolewski a=jakebolewski

* Run subset on every push in parallel
* Kill jobs that fail quickly
* Kill stale jobs quickly upon new commits to a branch
* Run all OS unit tests only on bors try / bors merge
* Remove Azure Devops Pipelines

# Description

A clear and concise description of the code with usage.



Co-authored-by: Ignacio <[email protected]>
Co-authored-by: Brandon Allen <[email protected]>
Co-authored-by: jakebolewski <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 12, 2020

Build failed (retrying...):

  • test-os (ubuntu-latest)
  • test-os (windows-latest)
  • test-os (macos-latest)

bors bot added a commit that referenced this pull request Jun 13, 2020
1232: Add simple hydrostatic spin-down test for Ocean Model r=blallen a=blallen

# Description

This PR adds a simple test of the hydrostatic dynamics of the Ocean model. It is intended to be used as the basic test case when implementing the split-explicit time stepper. The analytic solution (and more) can be found in this overleaf: https://www.overleaf.com/project/5e9dfd2716ba2600018de266

Currently, I'm just testing that the solution is within 0.5% of the analytic solution after a day. If anyone has better ideas please suggest them. I'd also like to turn this into a Literature tutorial, so I don't think I want to go the full convergence testing route with this, altho it is possible. 



1234: Switch CI to use Github Actions, run tests in parallel and fast fail when possible  r=jakebolewski a=jakebolewski

* Run subset on every push in parallel
* Kill jobs that fail quickly
* Kill stale jobs quickly upon new commits to a branch
* Run all OS unit tests only on bors try / bors merge
* Remove Azure Devops Pipelines

# Description

A clear and concise description of the code with usage.



Co-authored-by: Brandon Allen <[email protected]>
Co-authored-by: jakebolewski <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 13, 2020

Build failed (retrying...):

* Run subset on every push in parallel
* Kill jobs that fail quickly
* Kill stale jobs quickly upon new commits to a branch
* Run all OS unit tests only on bors try / bors merge
* Remove Azure Devops Pipelines
@bors
Copy link
Contributor

bors bot commented Jun 13, 2020

Canceled.

@jakebolewski
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 13, 2020

Build succeeded:

@bors bors bot merged commit cef6389 into master Jun 13, 2020
@bors bors bot deleted the jcb/github_actions branch June 13, 2020 02:38
@charleskawczynski charleskawczynski mentioned this pull request Jun 15, 2020
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants