From ed885061bd9b98ebfcb0aa1286cfbea9ade71325 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 09:42:07 -0700 Subject: [PATCH 01/11] run_sys_tests: Check Python environment for FatesColdTwoStream tests. --- python/ctsm/run_sys_tests.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index e4a0bcf009..959d9e52ae 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -249,7 +249,7 @@ def run_sys_tests( else: raise RuntimeError("None of suite_name, testfile or testlist were provided") if not running_ctsm_py_tests: - _try_systemtests(testname_list) + _check_py_env(testname_list) _run_create_test( cime_path=cime_path, test_args=test_args, @@ -708,7 +708,7 @@ def _run_test_suite( ) -def _try_systemtests(testname_list): +def _check_py_env(test_attributes): err_msg = " can't be loaded. Do you need to activate the ctsm_pylib conda environment?" # Suppress pylint import-outside-toplevel warning because (a) we only want to import # this when certain tests are requested, and (b) the import needs to be in a try-except @@ -716,11 +716,21 @@ def _try_systemtests(testname_list): # pylint: disable=import-outside-toplevel disable # Suppress pylint unused-import warning because the import itself IS the use. # pylint: disable=unused-import disable - if any("FSURDATMODIFYCTSM" in t for t in testname_list): + if any("FSURDATMODIFYCTSM" in t for t in test_attributes): try: import ctsm.modify_input_files.modify_fsurdat except ModuleNotFoundError as err: raise ModuleNotFoundError("modify_fsurdat" + err_msg) from err + if any("FatesColdTwoStream" in t for t in test_attributes): + # This bit is needed because it's outside the top-level python/ directory. + _FATES_DIR = os.path.join( + os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir,"src", "fates" + ) + sys.path.insert(1, _FATES_DIR) + try: + import tools.modify_fates_paramfile + except ModuleNotFoundError as err: + raise ModuleNotFoundError("modify_fates_paramfile" + err_msg) from err def _get_compilers_for_suite(suite_name, machine_name, running_ctsm_py_tests): @@ -730,7 +740,8 @@ def _get_compilers_for_suite(suite_name, machine_name, running_ctsm_py_tests): "No tests found for suite {} on machine {}".format(suite_name, machine_name) ) if not running_ctsm_py_tests: - _try_systemtests([t["testname"] for t in test_data]) + _check_py_env([t["testname"] for t in test_data]) + _check_py_env([t["testmods"] for t in test_data]) compilers = sorted({one_test["compiler"] for one_test in test_data}) logger.info("Running with compilers: %s", compilers) return compilers From 4f65f39b2baf856150f8ade9268205da385866b2 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 10:36:33 -0700 Subject: [PATCH 02/11] run_sys_tests: Check exact name of testmods. --- python/ctsm/run_sys_tests.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 959d9e52ae..c996f3fdaa 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -716,12 +716,25 @@ def _check_py_env(test_attributes): # pylint: disable=import-outside-toplevel disable # Suppress pylint unused-import warning because the import itself IS the use. # pylint: disable=unused-import disable + + # Check requirements for FSURDATMODIFYCTSM, if needed if any("FSURDATMODIFYCTSM" in t for t in test_attributes): try: import ctsm.modify_input_files.modify_fsurdat except ModuleNotFoundError as err: raise ModuleNotFoundError("modify_fsurdat" + err_msg) from err - if any("FatesColdTwoStream" in t for t in test_attributes): + + # Isolate testmods, producing a list like ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] + test_attributes_split = [] + for t in test_attributes: + for x in t.split("."): + y = x.replace("/", "-") + for z in y.split("--"): + test_attributes_split.append(z) + + # Check that list for any testmods that use modify_fates_paramfile.py + testmods_to_check = ["clm-FatesColdTwoStream", "clm-FatesColdTwoStreamNoCompFixedBioGeo"] + if any(t in testmods_to_check for t in test_attributes_split): # This bit is needed because it's outside the top-level python/ directory. _FATES_DIR = os.path.join( os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir,"src", "fates" From 33be9f67aab9b0db6fa46014e6aa0297024300c4 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 10:56:09 -0700 Subject: [PATCH 03/11] run_sys_tests: Handle tests without testmods. --- python/ctsm/run_sys_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index c996f3fdaa..7dcafd059f 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -754,7 +754,7 @@ def _get_compilers_for_suite(suite_name, machine_name, running_ctsm_py_tests): ) if not running_ctsm_py_tests: _check_py_env([t["testname"] for t in test_data]) - _check_py_env([t["testmods"] for t in test_data]) + _check_py_env([t["testmods"] for t in test_data if "testmods" in t.keys()]) compilers = sorted({one_test["compiler"] for one_test in test_data}) logger.info("Running with compilers: %s", compilers) return compilers From c5966a4a77b908f673a0738ab94dd4b4638af410 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 10:57:21 -0700 Subject: [PATCH 04/11] run_sys_tests: Changes to satisfy pylint. --- python/ctsm/run_sys_tests.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 7dcafd059f..0d6cf9dcd5 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -716,6 +716,9 @@ def _check_py_env(test_attributes): # pylint: disable=import-outside-toplevel disable # Suppress pylint unused-import warning because the import itself IS the use. # pylint: disable=unused-import disable + # Suppress pylint import-error warning because the whole point here is to check + # whether import is possible. + # pylint: disable=import-error disable # Check requirements for FSURDATMODIFYCTSM, if needed if any("FSURDATMODIFYCTSM" in t for t in test_attributes): @@ -724,22 +727,23 @@ def _check_py_env(test_attributes): except ModuleNotFoundError as err: raise ModuleNotFoundError("modify_fsurdat" + err_msg) from err - # Isolate testmods, producing a list like ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] + # Isolate testmods, producing a list like\ + # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] test_attributes_split = [] - for t in test_attributes: - for x in t.split("."): - y = x.replace("/", "-") - for z in y.split("--"): - test_attributes_split.append(z) + for test_attribute in test_attributes: + for dot_split in test_attribute.split("."): + slash_replaced = dot_split.replace("/", "-") + for ddash_split in slash_replaced.split("--"): + test_attributes_split.append(ddash_split) # Check that list for any testmods that use modify_fates_paramfile.py testmods_to_check = ["clm-FatesColdTwoStream", "clm-FatesColdTwoStreamNoCompFixedBioGeo"] if any(t in testmods_to_check for t in test_attributes_split): # This bit is needed because it's outside the top-level python/ directory. - _FATES_DIR = os.path.join( + fates_dir = os.path.join( os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir,"src", "fates" ) - sys.path.insert(1, _FATES_DIR) + sys.path.insert(1, fates_dir) try: import tools.modify_fates_paramfile except ModuleNotFoundError as err: From 0dcd0a3c1abcaffe5529f8d79a6bc34734b195c7 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 10:57:57 -0700 Subject: [PATCH 05/11] run_sys_tests: Reformatting with black. --- python/ctsm/run_sys_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 0d6cf9dcd5..033edf918d 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -741,7 +741,7 @@ def _check_py_env(test_attributes): if any(t in testmods_to_check for t in test_attributes_split): # This bit is needed because it's outside the top-level python/ directory. fates_dir = os.path.join( - os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir,"src", "fates" + os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir, "src", "fates" ) sys.path.insert(1, fates_dir) try: From 41e7db5ba74f0a0a4673c9f81fb0cf94ca75eda1 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 19 Jan 2024 10:58:46 -0700 Subject: [PATCH 06/11] Added previous commit to .git-blame-ignore-revs. --- .git-blame-ignore-revs | 1 + 1 file changed, 1 insertion(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index e63de8e099..8708f8e0c2 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -21,6 +21,7 @@ e4d38681df23ccca0ae29581a45f8362574e0630 025d5e7c2e80263717fb029101d65cbbf261c3c4 a9d96219902cf609636886c7073a84407f450d9a d866510188d26d51bcd6d37239283db690af7e82 +0dcd0a3c1abcaffe5529f8d79a6bc34734b195c7 # Ran SystemTests and python/ctsm through black python formatter 5364ad66eaceb55dde2d3d598fe4ce37ac83a93c 8056ae649c1b37f5e10aaaac79005d6e3a8b2380 From 975e628b10ce49990cc60846079f2f735508caaf Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 22 Jan 2024 10:07:07 -0700 Subject: [PATCH 07/11] Fix typo in comment. --- python/ctsm/run_sys_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 033edf918d..33ff158f27 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -727,7 +727,7 @@ def _check_py_env(test_attributes): except ModuleNotFoundError as err: raise ModuleNotFoundError("modify_fsurdat" + err_msg) from err - # Isolate testmods, producing a list like\ + # Isolate testmods, producing a list like # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] test_attributes_split = [] for test_attribute in test_attributes: From 473a581de75ceead7c690938b5cfb8fafc9a2001 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 22 Jan 2024 11:06:41 -0700 Subject: [PATCH 08/11] run_sys_tests: Functionize _get_testmod_list(). --- python/ctsm/run_sys_tests.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 33ff158f27..a386598766 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -708,6 +708,20 @@ def _run_test_suite( ) +def _get_testmod_list(test_attributes): + # Isolate testmods, producing a list like + # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] + # Handles test attributes passed in from run_sys_tests calls using -t, -f, or -s + + testmods = [] + for test_attribute in test_attributes: + for dot_split in test_attribute.split("."): + slash_replaced = dot_split.replace("/", "-") + for ddash_split in slash_replaced.split("--"): + testmods.append(ddash_split) + return testmods + + def _check_py_env(test_attributes): err_msg = " can't be loaded. Do you need to activate the ctsm_pylib conda environment?" # Suppress pylint import-outside-toplevel warning because (a) we only want to import @@ -727,18 +741,10 @@ def _check_py_env(test_attributes): except ModuleNotFoundError as err: raise ModuleNotFoundError("modify_fsurdat" + err_msg) from err - # Isolate testmods, producing a list like - # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] - test_attributes_split = [] - for test_attribute in test_attributes: - for dot_split in test_attribute.split("."): - slash_replaced = dot_split.replace("/", "-") - for ddash_split in slash_replaced.split("--"): - test_attributes_split.append(ddash_split) - # Check that list for any testmods that use modify_fates_paramfile.py testmods_to_check = ["clm-FatesColdTwoStream", "clm-FatesColdTwoStreamNoCompFixedBioGeo"] - if any(t in testmods_to_check for t in test_attributes_split): + testmods = _get_testmod_list(test_attributes) + if any(t in testmods_to_check for t in testmods): # This bit is needed because it's outside the top-level python/ directory. fates_dir = os.path.join( os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir, "src", "fates" From 039a243f52d6a1f565ebed513fe90084e14fbd8b Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 22 Jan 2024 11:12:34 -0700 Subject: [PATCH 09/11] Add 'unique' option to _get_testmod_list(), default True. --- python/ctsm/run_sys_tests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index a386598766..7e232d8efe 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -708,7 +708,7 @@ def _run_test_suite( ) -def _get_testmod_list(test_attributes): +def _get_testmod_list(test_attributes, unique=True): # Isolate testmods, producing a list like # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] # Handles test attributes passed in from run_sys_tests calls using -t, -f, or -s @@ -718,7 +718,9 @@ def _get_testmod_list(test_attributes): for dot_split in test_attribute.split("."): slash_replaced = dot_split.replace("/", "-") for ddash_split in slash_replaced.split("--"): - testmods.append(ddash_split) + if ddash_split not in testmods or not unique: + testmods.append(ddash_split) + return testmods From a7dbc378cf88f2e0befdd34e83bca5902995fa89 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 22 Jan 2024 11:45:03 -0700 Subject: [PATCH 10/11] _get_testmod_list(): Only include strings with 'clm-'. --- python/ctsm/run_sys_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ctsm/run_sys_tests.py b/python/ctsm/run_sys_tests.py index 7e232d8efe..de93081504 100644 --- a/python/ctsm/run_sys_tests.py +++ b/python/ctsm/run_sys_tests.py @@ -708,7 +708,7 @@ def _run_test_suite( ) -def _get_testmod_list(test_attributes, unique=True): +def _get_testmod_list(test_attributes, unique=False): # Isolate testmods, producing a list like # ["clm-test1mod1", "clm-test2mod1", "clm-test2mod2", ...] # Handles test attributes passed in from run_sys_tests calls using -t, -f, or -s @@ -718,7 +718,7 @@ def _get_testmod_list(test_attributes, unique=True): for dot_split in test_attribute.split("."): slash_replaced = dot_split.replace("/", "-") for ddash_split in slash_replaced.split("--"): - if ddash_split not in testmods or not unique: + if "clm-" in ddash_split and (ddash_split not in testmods or not unique): testmods.append(ddash_split) return testmods From 8d7d88a245b7f0e7fc4d10a060a5fee567ada2de Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 22 Jan 2024 11:45:42 -0700 Subject: [PATCH 11/11] Add unit tests for _get_testmod_list(). --- python/ctsm/test/test_unit_run_sys_tests.py | 53 ++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/python/ctsm/test/test_unit_run_sys_tests.py b/python/ctsm/test/test_unit_run_sys_tests.py index ee5197d76f..65ec1df5a5 100755 --- a/python/ctsm/test/test_unit_run_sys_tests.py +++ b/python/ctsm/test/test_unit_run_sys_tests.py @@ -16,7 +16,7 @@ from ctsm import add_cime_to_path # pylint: disable=unused-import from ctsm import unit_testing -from ctsm.run_sys_tests import run_sys_tests +from ctsm.run_sys_tests import run_sys_tests, _get_testmod_list from ctsm.machine_defaults import MACHINE_DEFAULTS from ctsm.machine import create_machine from ctsm.joblauncher.job_launcher_factory import JOB_LAUNCHER_FAKE @@ -269,6 +269,57 @@ def test_withDryRun_nothingDone(self): self.assertEqual(os.listdir(self._scratch), []) self.assertEqual(machine.job_launcher.get_commands(), []) + def test_getTestmodList_suite(self): + """Ensure that _get_testmod_list() works correctly with suite-style input""" + input = [ + "clm/default", + "clm/default", + "clm/crop", + "clm/cropMonthlyOutput", + ] + target = [ + "clm-default", + "clm-default", + "clm-crop", + "clm-cropMonthlyOutput", + ] + output = _get_testmod_list(input, unique=False) + self.assertEqual(output, target) + + def test_getTestmodList_suite_unique(self): + """Ensure that _get_testmod_list() works correctly with unique=True""" + input = [ + "clm/default", + "clm/default", + "clm/crop", + "clm/cropMonthlyOutput", + ] + target = [ + "clm-default", + "clm-crop", + "clm-cropMonthlyOutput", + ] + + output = _get_testmod_list(input, unique=True) + self.assertEqual(output, target) + + def test_getTestmodList_testname(self): + """Ensure that _get_testmod_list() works correctly with full test name(s) specified""" + input = [ + "ERS_D_Ld15.f45_f45_mg37.I2000Clm50FatesRs.izumi_nag.clm-crop", + "ERS_D_Ld15.f45_f45_mg37.I2000Clm50FatesRs.izumi_nag.clm-default", + ] + target = ["clm-crop", "clm-default"] + output = _get_testmod_list(input) + self.assertEqual(output, target) + + def test_getTestmodList_twomods(self): + """Ensure that _get_testmod_list() works correctly with full test name(s) specified and two mods in one test""" + input = ["ERS_D_Ld15.f45_f45_mg37.I2000Clm50FatesRs.izumi_nag.clm-default--clm-crop"] + target = ["clm-default", "clm-crop"] + output = _get_testmod_list(input) + self.assertEqual(output, target) + if __name__ == "__main__": unit_testing.setup_for_tests()