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

Automatically call case_setup for case2 in compareTwo #4557

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

jgfouca
Copy link
Contributor

@jgfouca jgfouca commented Jan 3, 2024

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)?:

It's removal in #4546 seems to have broken a number of
less-common test types (ERR, MCC, IRT, PRE).
@jgfouca jgfouca requested a review from jedwards4b January 3, 2024 19:41
@jgfouca jgfouca self-assigned this Jan 3, 2024
@jgfouca jgfouca requested a review from dqwu January 3, 2024 19:43
@jedwards4b
Copy link
Contributor

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

@jedwards4b
Copy link
Contributor

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

@jgfouca
Copy link
Contributor Author

jgfouca commented Jan 3, 2024

@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
Copy link
Contributor

I would be fine with that change.

.. so all the subclasses don't have to remember to do it.
@jgfouca
Copy link
Contributor Author

jgfouca commented Jan 3, 2024

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

@jgfouca jgfouca changed the title Re-add newcase setup to create_clone Automatically call case_setup for case2 in compareTwo Jan 3, 2024
@jgfouca jgfouca merged commit 14fd58b into master Jan 3, 2024
6 checks passed
@jgfouca jgfouca deleted the jgfouca/fix_clone_test_cases branch January 3, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants