Changes needed for CESM to define an FUNITSHARE system test in the allactive area #4895
Conversation
This is needed by the CESM_share extension to FUNIT
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4895 +/- ##
===========================================
+ Coverage 31.43% 42.05% +10.62%
===========================================
Files 264 134 -130
Lines 38683 19739 -18944
Branches 8386 4366 -4020
===========================================
- Hits 12161 8302 -3859
+ Misses 25281 10632 -14649
+ Partials 1241 805 -436 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm looking into the test failures... I think they're due to some mock return values that I need to adjust. I'm working on figuring out which of the |
The previous commit introduced an extra call to case.get_value to get
SYSTEM_TESTS_DIR for component=allactive. This broke the mock get_value
returns. To make these returns more robust, I have changed the way
they're done to be based on a dictionary lookup rather than an ordered
list.
This kind of replacement should probably be done for other
case.get_value mocks. Note that I identified the argument values that
needed to be handled with this code:
for i, call in enumerate(case.get_value.call_args_list):
print(f" Call {i}: {call}")
|
I have fixed the offending case.get_value mock replacements. I reworked them to be based on dictionary lookups rather than ordered lists because this felt more robust / future-proof. In my view, this should eventually be done for other similar case.get_value replacements, since the previous design made it hard to figure out what was wrong with the test and how to fix it. (And I noticed that the previous version seemed to have some incorrect return values, though it happened to work okay by chance.) I identified the argument values that needed to be handled by adding the following near the end of each test: for i, call in enumerate(case.get_value.call_args_list):
print(f" Call {i}: {call}") |
Requires the CIME changes in ESMCI/cime#4895.
billsacks
changed the title
|
In doing some more testing of this branch, I realized it causes problems with running tests that are defined by a component (e.g., clm-defined tests), I think because now two different paths resolve to the same place so CIME thinks there are multiple definitions of the test. I'll do something to resolve this and then remove the WIP label. |
Or if multiple SYSTEM_TESTS_DIRs resolve to the same path.... This happened when I introduced an entry for allactive: in a standalone checkout of CTSM, COMP_ROOT_DIR_LND is the same as SRCROOT, so the allactive and clm CONFIG_TESTS_FILEs and SYSTEM_TESTS_DIRs pointed to the same path, causing an error.
billsacks
changed the title
I have resolved this issue, so this is ready for review. |
|
See also ESCOMP/CESM#362, the CESM PR that needs these changes. |
|
The e3sm system-testing failure is: I'm not sure what to do about this....? |
1 participant
Description
I'd like to add an FUNITSHARE system test - an extension of the FUNIT test that will run the CESM_share tests on derecho as part of prealpha testing. I needed a couple of small additions in CIME to allow this to work:
(1) Allowing new System Test types to be defined in the "allactive" area in addition to the cime_config associated with each component.
(2) Allowing extra arguments to the
run_tests.pycommand.See ESCOMP/CESM#362 for the CESM changes that leverage this.
Checklist