CTSM Testing and PR high level workflow - ESCOMP/CTSM GitHub Wiki

General workflow

General notes

The full test suite must be run on the final version of the code before it can be merged to master (although an exception can be made if the only changes since the last full run of the test suite were a few small changes to comments and whitespace). There is some flexibility as to when you do this full run of the test suite: It can be done before the PR is reviewed (in which case the test results are sufficient if there end up being no substantive changes based on PR feedback), or after the PR review is done, just before merging to master. For larger changes, there will often be a few runs of the full test suite, at various points. But for smaller changes, a single run may suffice. However, in all cases, the full test suite run must be done on a version of the code that is up-to-date with the latest version of master.

Up until now, CLM software engineers (Erik, Ben, Bill) have typically done most of the process other than initial development. We'll need to determine how much of this process should be done by the the scientist-developer, and how much should be done after hand-off to a software engineer.

Initial development:

  • Make new feature branch off of latest ctsm master tag
  • Do development, running a small test suite as you go

When development is done:

  • OPTIONAL: Update to latest ctsm master tag
    • This should be done at this point if you are far out of date, or if you expect conflicts with changes on master.
    • If not done now, this will have to be done before merging to master.
  • OPTIONAL: Run full test suite
    • This should be done at this point if your changes are substantial.
    • If not done now, this will have to be done before merging to master.
    • You may submit the PR before the full test suite is done running
  • Submit PR
  • Incorporate PR feedback, running a small test suite as you go

When you're done incorporating PR feedback:

  • "Take a number": get assigned a tag for merging to master

When it's your turn to merge to master:

  • Update to latest ctsm master tag
    • You can determine if you are up-to-date with the latest ctsm master tag with: git log your_branch..latest_ctsm_master_tag. If this shows nothing, then you are up-to-date with that tag already.
    • If you are not up-to-date with the latest version of master, do this update by merging master into your branch, NOT rebasing, in order to maintain a record of exactly what testing you have run, and to keep the commits referenced in the PR.
      • (Bill Sacks note) I'm not sure if this is important. It seems like github PRs are pretty smart about handling rebases. It's still a very good idea not to rebase if someone else may have based changes off of yours, or if you have a record of what testing you did on earlier commits. For these reasons, I tend to use a merging rather than rebasing workflow. But I'd be open to allowing people to rebase if others think it's preferable.
  • Run full test suite
    • Exception: If you were already up-to-date with the latest ctsm master tag, and you have already run the full test suite on your latest branch code, then you can skip this step.
    • Exception: If the only changes you have made since the last run of the full test suite are trivial (meaning a few small changes to comments and/or whitespace), then you may verify the latest changes by running one NAG debug test rather than the full test suite.
  • If non-trivial changes were needed to get the final test suite to pass, then the final version should be returned to the PR reviewers for sign-off. If further changes are required based on this re-review, then you will need to incorporate these changes and return to "When you're done incorporating PR feedback".
  • Merge to master
⚠️ **GitHub.com Fallback** ⚠️