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

Add CI to repo #29

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add CI to repo #29

wants to merge 16 commits into from

Conversation

laurenchilutti
Copy link
Contributor

@laurenchilutti laurenchilutti commented Oct 4, 2023

Description

Adding CI tests to this repo. This brings over a similar workflow as is done in the GFDL_atmos_cubed_sphere repository. Will build solo model (nh, sw, and hydro) and then run the 35 tests in RTS/CI.

GitHub recognizes yaml files stored in .github/workflows as GitHub Actions workflows:
Intel_Parallelworks_CI.yaml specifies instructions for automated CI with every pull request. There are 4 steps to the CI:
-checkout: Executes a script checkout.sh stored on Parallelworks cloud. This script will checkout the SHiELD_build directory (checks out the branch/commit of the PR that triggers the CI) and check out component codes. We only compile FMS when the CHECKOUT_code script specifies a new version of FMS. We store the latest version of FMS for use in compiling SHiELD on Parallelworks at /contrib/fv3/2023.2.0/SHiELD_build/externallibs.
-build: Executes the script compile.sh with various build configurations (a total of 36 combinations, of which any with sw + shield will be skipped). The script executes a compile command inside of the CI container on Parallelworks
-test: Runs the script run_test.sh 35 times. Each time giving it an argument specifying which test to run. These tests run in parallel.

The scripts stored on parallelworks will be version controlled in this repository and are at .github/.parallelworks.

Fixes # (issue)

How Has This Been Tested?

Tested by the successful checks on this PR

Checklist:

Please check all whether they apply or not

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@laurenchilutti
Copy link
Contributor Author

I have one more update to make. I will send an email when I am ready for this to be reviewed for a final time.

Copy link
Contributor

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

I notice some scripts are invoking bash and others are invoking sh, is there a reason you can't use one shell throughout?

If the argument list may grow, you may want to consider arg-parsing for the future https://stackoverflow.com/questions/192249/how-do-i-parse-command-line-arguments-in-bash.

@laurenchilutti
Copy link
Contributor Author

@bensonr I can't see any reason why I had some as sh and some as bash. They should all be consistent now. I also modified the argument parsing to be cleaner.

This is now ready for review. Thanks!

Copy link
Contributor

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for considering the suggested actions.

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