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

Implement GitHub Actions CI testing (Fixes #138) #189

Merged

Conversation

manishvenu
Copy link
Collaborator

This PR fixes Issue #138, which was about implementing CI in MOM_Interface with four initial tasks. Please reference back to the issue for those requirements.

Changes Introduced by this PR:

  • A GitHub Action Workflow (please see below for details) under .github/workflows/general-ci-tests.yml
  • Incidentally, Black formatting on two scripts in cime_config

CI/Github Action Workflow Details:

Workflow is structured into three independent jobs that are triggered on push and pull-request events onto the main branch:

  1. checking the black format of scripts in cime_config
  2. running the check_default_params test under the tests folder
  3. building the standalone MOM6 and running two lightweight MOM6 examples (double_gyre, single_column/KPP)

Testing:

  • Test Failures: The bottom two jobs (2 & 3) were tested by creating an error (See Commits starting with Test CI), and the first job with Black formatting was already failing (commit) and was corrected early on in the process.
  • Test Events/Triggers: Every job in the commit history was triggered on a push. Pull-Requests were tested in Test CI: Pull Requests #188.

Notes:

  • The commits should (need?) be squashed. (My fault, hah)
  • Checking out the correct MOM branch for the standalone build is a little trickier because we are starting with the CESM repo. To be safe, the triggering event is manually checked out instead of using the common Github action actions/checkout@v4 (See the steps Checkout initial event (Pull Request) and Checkout initial event (Push)).
  • I think the workflow is pretty easy to read through, but would welcome any advice/help on the clarity.

Thanks,
Manish V.

Fixes #138

manishvenu and others added 30 commits August 27, 2024 10:22
Create the workflow!
1. Start with the simplest workflow item in this issue: Running the check_default_parameters test.
Silly Mistake. This is for the running of check_default_parameters.py
Remove pyyaml install because already installed, adjust file name
Move checkout from default_params_job to separate job (simple_checkout) and list as a "need" on the check_default_params and black_check.. job
Added:

1. Build Job: Checkout CESM, Load Externals (Including MOM6)
2. Test_lightweight_standalone_mom Job: Run MOM6 on some sample test cases
manishvenu and others added 8 commits August 29, 2024 16:31
This reverts commit 6461882.
This was a test to cause CI to fail, and is reverted because we don't
want to include that failure.
This reverts commit b8e84ab.
This commit caused an error in the build, we don't want that to stay in
this branch.
Copy link
Member

@alperaltuntas alperaltuntas left a comment

Choose a reason for hiding this comment

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

Thanks, @manishvenu! This is awesome, and a much-needed enhancement.

I have a suggestion: Since the workflow is set up to run on Ubuntu, it might be more consistent to use an Ubuntu-specific script rather than the Darwin build script. Here's what I suggest:

(1) Consider creating an ubuntu-gnu.mk template under the standalone/templates directory, perhaps using the homebrew-gnu.mk as a reference.
(2) Instead of having separate build scripts for each machine/compiler configuration, how about creating a single build_examples.sh script with --machine and --compiler arguments? You can take a look at the approach I used in my pull request #186, where I added one of those arguments.

@manishvenu
Copy link
Collaborator Author

manishvenu commented Aug 30, 2024

Notes/Qs in changes resulting from @alperaltuntas review:

  1. submit_casper_build: In standalone/build, the submit_casper_build did not work before this change - the arguments aren't a part of the ./build_examples-ncar.sh. I reformatted it to work, but is this script necessary? Given that it wasn't in a working state beforehand.
  2. build_examples.sh: Deleted both build_examples (darwin and ncar) and merged pretty much the exact changes into one script called build_examples as suggested. This was after merging Improvements in build_examples-ncar.sh script #186.
  3. ubuntu-gnu.mk: Created a ubuntu-gnu.mk copied exactly from homebrew-gnu.mk, given that it works. Please let me know if I should make any changes based on the machine
  4. Added README badge as per @alperaltuntas discussion (offline). If needed, should we name the CI something different? Right now, it is "General MOM_Interface CI". One thing we could try is to split the workflow into a couple different workflows (the jobs are independent) so that we could have a MOM_standalone badge and General MOM_Interface CI Badge for the other two tests.
  5. Tested: Each .mk file & submit_casper_build

Copy link
Member

@alperaltuntas alperaltuntas left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks.

@alperaltuntas alperaltuntas merged commit 3e193bd into ESCOMP:main Sep 4, 2024
3 checks passed
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.

Set up GitHub Actions CI testing
3 participants