-
Notifications
You must be signed in to change notification settings - Fork 1
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
ACCESS-ESM1.5 QA test: CICE icefields_nml configuration #54
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 73.48% 78.07% +4.59%
==========================================
Files 15 15
Lines 724 748 +24
==========================================
+ Hits 532 584 +52
+ Misses 192 164 -28 ☔ View full report in Codecov by Sentry. |
I quickly added a generic test for the ACCESS-ESM1.5 specific config tests - to just checking the tests pass without error (and also to pass the codecov CI checks..) I ran some manual tests with an configuration to test the error messages, and that they failed when expected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Thanks for fixing up the codecov tests...the red crosses were a bit scary when I was doing the initial esm1.5 QA checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like there are three things bundled into one PR here ?
- Adding a qa test for cice namelist
- rearranging the directories
- add a config for preindustrial+concentrations
In future we should do this as seperate PRs so we can track the changes easier and review it easier !
Yes, sorry about that! I added the tests so they covered the lines I added in the first commit. But yeah next time, I'll add the wider tests for tests in an earlier PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jo! - I ran some adhoc tests using dev-historical+concentrations
from https://github.com/ACCESS-NRI/access-esm1.5-configs
- The default passes,
- adding &icefields_nml to cice_in gives a positive failure, but the error message could be better,
- adding fields to &setup_nml detects gives a positive failure for duplicated fields (correct)
- renaming ice_history.nml & cice_in.nml both fail correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good Jo
This should also close #44 |
Thanks all for the reviews! |
Added QA test for separate
icefields_nml
inice_history.nml
(in as described in #50)Closes #50
Closes #44