ESMCI / cime

Common Infrastructure for Modeling the Earth
http://esmci.github.io/cime
Other
161 stars 206 forks source link

Automatically call case_setup for case2 in compareTwo #4557

Closed jgfouca closed 9 months ago

jgfouca commented 9 months ago

It's removal in #4546 seems to have broken a number of less-common test types (ERR, MCC, IRT, PRE).

MCC_P1.f19_g16_rx1.A
ERROR: ERROR: must invoke case.setup script before calling build script
ERR_Ln9.f45_g37_rx1.A
Exception from case_test: [Errno 2] No such file or directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/archive/ERR_Ln9.f45_g37_rx1.A.anlgce_gnu.20240102_114925_1cza5r/rest'
Exception from case_test: [Errno 20] Not a directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/ERR_Ln9.f45_g37_rx1.A.anlgce_gnu.20240102_114925_1cza5r/run/case2run/timing'
IRT_N2_Vmct_Ln9.f19_g16_rx1.A
Exception from case_test: [Errno 20] Not a directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/IRT_N2_Vmct_Ln9.f19_g16_rx1.A.anlgce_gnu.20240102_114925_1cza5r/run/case2run/timing'
PRE.f19_f19.ADESP
Exception from case_test: ERROR: Do not know about batch job case.test

Test suite: the test cases above, by hand Test baseline: Test namelist changes: Test status: bit for bit

Fixes [CIME Github issue #]

User interface changes?:

Update gh-pages html (Y/N)?:

jedwards4b commented 9 months ago

I feel like it really should not be there - shouldn't we add a call to case.setup to the tests in question?

jedwards4b commented 9 months ago

case.setup should be called in the test, for example, here

jgfouca commented 9 months ago

@jedwards4b , there are a few approaches here. We should seek a solution that minimizes the chances of a mistake for developers working on SystemTests.

I'm wondering why system_tests_compare_two does not call case_setup for case2 in _setup_cases:

        # This assures that case one namelists are populated                                                                                                                                                                   
        # and creates the case.test script                                                                                                                                                                                     
        self._case.case_setup(test_mode=False, reset=True)
        fix_single_exe_case(self._case)

        # Set up case 2                                                                                                                                                                                                        
        with self._case2:
            self._activate_case2()
            self._common_setup()
            self._case_two_setup()

It does not seem like good design for developers to have to remember to call case_setup in case_two_setup.

jedwards4b commented 9 months ago

I would be fine with that change.

jgfouca commented 9 months ago

@jedwards4b , I pushed another commit and I'm much happier now. I think this is the best solution.