From 35cd9e6976529bbcfb2f1bad235a729e70ec668a Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 13:16:26 -0600 Subject: [PATCH 1/8] Python: Avoid non-portable os.mknod(). Resolves ESCOMP/CTSM#2986. --- cime_config/SystemTests/sspmatrixcn.py | 5 +++-- python/ctsm/test/test_unit_sspmatrix.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cime_config/SystemTests/sspmatrixcn.py b/cime_config/SystemTests/sspmatrixcn.py index 17ac8abd74..87c4ab2e80 100644 --- a/cime_config/SystemTests/sspmatrixcn.py +++ b/cime_config/SystemTests/sspmatrixcn.py @@ -14,6 +14,7 @@ """ import shutil, glob, os, sys +from pathlib import Path from datetime import datetime if __name__ == "__main__": @@ -205,9 +206,9 @@ def run_indv(self, nstep, st_archive=True): restdir = os.path.join(rest_r, rundate) os.mkdir(restdir) rpoint = os.path.join(restdir, "rpointer.clm." + rundate) - os.mknod(rpoint) + Path.touch(rpoint) rpoint = os.path.join(restdir, "rpointer.cpl." + rundate) - os.mknod(rpoint) + Path.touch(rpoint) def run_phase(self): "Run phase" diff --git a/python/ctsm/test/test_unit_sspmatrix.py b/python/ctsm/test/test_unit_sspmatrix.py index 1b1bc60185..dd81a7df4f 100755 --- a/python/ctsm/test/test_unit_sspmatrix.py +++ b/python/ctsm/test/test_unit_sspmatrix.py @@ -53,7 +53,7 @@ def create_clone( Extend to handle creation of user_nl_clm file """ clone = super().create_clone(newcase, keepexe=keepexe) - os.mknod(os.path.join(newcase, "user_nl_clm")) + Path.touch(os.path.join(newcase, "user_nl_clm")) # Also make the needed case directories clone.make_case_dirs(self._tempdir) return clone @@ -165,7 +165,7 @@ def test_append_user_nl_step2(self): if os.path.exists(ufile): os.remove(ufile) - os.mknod(ufile) + Path.touch(ufile) expect = "\nhist_nhtfrq = -8760, hist_mfilt = 2\n" self.ssp.append_user_nl(caseroot=".", n=2) From 4ccfca9c208c16140fed8784a2ea02a044173fd0 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 13:30:34 -0600 Subject: [PATCH 2/8] Avoid Python test errors on systems without default clmforcingindir. Resolves ESCOMP/CTSM#2985. --- python/ctsm/subset_data.py | 6 +++--- python/ctsm/test/test_unit_subset_data.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/python/ctsm/subset_data.py b/python/ctsm/subset_data.py index 81f1f703f3..1e430952cb 100644 --- a/python/ctsm/subset_data.py +++ b/python/ctsm/subset_data.py @@ -604,7 +604,7 @@ def determine_num_pft(crop): return num_pft -def setup_files(args, defaults, cesmroot): +def setup_files(args, defaults, cesmroot, testing=False): """ Sets up the files and folders needed for this program """ @@ -622,9 +622,9 @@ def setup_files(args, defaults, cesmroot): else: clmforcingindir = args.inputdatadir - if not os.path.isdir(clmforcingindir): + if not testing and not os.path.isdir(clmforcingindir): logger.info("clmforcingindir does not exist: %s", clmforcingindir) - abort("inputdata directory does not exist") + abort(f"inputdata directory does not exist: {clmforcingindir}") file_dict = {"main_dir": clmforcingindir} diff --git a/python/ctsm/test/test_unit_subset_data.py b/python/ctsm/test/test_unit_subset_data.py index eeb0a9a38a..9dda38af94 100755 --- a/python/ctsm/test/test_unit_subset_data.py +++ b/python/ctsm/test/test_unit_subset_data.py @@ -90,7 +90,7 @@ def test_inputdata_setup_files_basic(self): Test """ self.args = check_args(self.args) - files = setup_files(self.args, self.defaults, self.cesmroot) + files = setup_files(self.args, self.defaults, self.cesmroot, testing=True) self.assertEqual( files["fsurf_in"], "surfdata_0.9x1.25_hist_2000_16pfts_c240908.nc", @@ -153,7 +153,7 @@ def test_check_args_outsurfdat_provided(self): sys.argv = ["subset_data", "point", "--create-surface", "--out-surface", "outputsurface.nc"] self.args = self.parser.parse_args() self.args = check_args(self.args) - files = setup_files(self.args, self.defaults, self.cesmroot) + files = setup_files(self.args, self.defaults, self.cesmroot, testing=True) self.assertEqual( files["fsurf_out"], "outputsurface.nc", From 4ede239a86b7b1b1cd482cf76748bf0f00ec5452 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 14:35:07 -0600 Subject: [PATCH 3/8] Use pathlib.Path.touch() instead of os.system("touch"). --- .../test_unit_gen_mksurfdata_jobscript_single.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py index bee1aac715..5072939e3e 100755 --- a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py +++ b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py @@ -8,6 +8,7 @@ import os import sys import shutil +from pathlib import Path import tempfile @@ -35,11 +36,6 @@ def add_args(machine, nodes, tasks): sys.argv.append(item) -def create_empty_file(filename): - """create an empty file""" - os.system("touch " + filename) - - # Allow test names that pylint doesn't like; otherwise hard to make them # readable # pylint: disable=invalid-name @@ -79,13 +75,13 @@ def setUp(self): os.makedirs(self._bld_path) self.assertTrue(os.path.isdir(self._bld_path)) self._nlfile = os.path.join(self._tempdir, "namelist_file") - create_empty_file(self._nlfile) + Path.touch(self._nlfile) self.assertTrue(os.path.exists(self._nlfile)) self._mksurf_exe = os.path.join(self._bld_path, "mksurfdata") - create_empty_file(self._mksurf_exe) + Path.touch(self._mksurf_exe) self.assertTrue(os.path.exists(self._mksurf_exe)) self._env_mach = os.path.join(self._bld_path, ".env_mach_specific.sh") - create_empty_file(self._env_mach) + Path.touch(self._env_mach) self.assertTrue(os.path.exists(self._env_mach)) sys.argv = [ "gen_mksurfdata_jobscript_single", From 51948b5059b047d486d8323ba3372588e2995754 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 15:05:21 -0600 Subject: [PATCH 4/8] Move test_derecho_mpirun to its own system testing file. This is the last step needed to make it so that UNIT tests are portable to machines other than those defined in CESM, and thus: Resolves ESCOMP/CTSM#2984. --- ...gen_mksurfdata_jobscript_single_derecho.py | 78 ++++++++++ ...st_unit_gen_mksurfdata_jobscript_single.py | 140 ++---------------- ..._gen_mksurfdata_jobscript_single_parent.py | 76 ++++++++++ python/ctsm/unit_testing.py | 15 ++ 4 files changed, 178 insertions(+), 131 deletions(-) create mode 100755 python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py create mode 100755 python/ctsm/test_gen_mksurfdata_jobscript_single_parent.py diff --git a/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py b/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py new file mode 100755 index 0000000000..1813529d0b --- /dev/null +++ b/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py @@ -0,0 +1,78 @@ +#!/usr/bin/env python3 + +""" +System tests for gen_mksurfdata_jobscript_single.py subroutines on Derecho +""" + +import unittest +import os + +from ctsm import unit_testing +from ctsm.test_gen_mksurfdata_jobscript_single_parent import TestFGenMkSurfJobscriptSingleParent +from ctsm.path_utils import path_to_cime +from ctsm.os_utils import run_cmd_output_on_error +from ctsm.toolchain.gen_mksurfdata_jobscript_single import get_parser +from ctsm.toolchain.gen_mksurfdata_jobscript_single import get_mpirun +from ctsm.toolchain.gen_mksurfdata_jobscript_single import check_parser_args +from ctsm.toolchain.gen_mksurfdata_jobscript_single import write_runscript_part1 + + +# Allow test names that pylint doesn't like; otherwise hard to make them +# readable +# pylint: disable=invalid-name + + +# pylint: disable=protected-access +# pylint: disable=too-many-instance-attributes +class TestFGenMkSurfJobscriptSingleDerecho(TestFGenMkSurfJobscriptSingleParent): + """Tests the gen_mksurfdata_jobscript_single subroutines on Derecho""" + + def test_derecho_mpirun(self): + """ + test derecho mpirun. This would've helped caught a problem we ran into + It will also be helpful when sumodules are updated to guide to solutions + to problems + """ + machine = "derecho" + nodes = 4 + tasks = 128 + unit_testing.add_args(machine, nodes, tasks) + args = get_parser().parse_args() + check_parser_args(args) + self.assertEqual(machine, args.machine) + self.assertEqual(tasks, args.tasks_per_node) + self.assertEqual(nodes, args.number_of_nodes) + self.assertEqual(self._account, args.account) + # Create the env_mach_specific.xml file needed for get_mpirun + # This will catch problems with our usage of CIME objects + # Doing this here will also catch potential issues in the gen_mksurfdata_build script + configure_path = os.path.join(path_to_cime(), "CIME", "scripts", "configure") + self.assertTrue(os.path.exists(configure_path)) + options = " --macros-format CMake --silent --compiler intel --machine " + machine + cmd = configure_path + options + cmd_list = cmd.split() + run_cmd_output_on_error( + cmd=cmd_list, errmsg="Trouble running configure", cwd=self._bld_path + ) + self.assertTrue(os.path.exists(self._env_mach)) + expected_attribs = {"mpilib": "default"} + with open(self._jobscript_file, "w", encoding="utf-8") as runfile: + attribs = write_runscript_part1( + number_of_nodes=nodes, + tasks_per_node=tasks, + machine=machine, + account=self._account, + walltime=args.walltime, + runfile=runfile, + ) + self.assertEqual(attribs, expected_attribs) + (executable, mksurfdata_path, env_mach_path) = get_mpirun(args, attribs) + expected_exe = "time mpibind " + self.assertEqual(executable, expected_exe) + self.assertEqual(mksurfdata_path, self._mksurf_exe) + self.assertEqual(env_mach_path, self._env_mach) + + +if __name__ == "__main__": + unit_testing.setup_for_tests() + unittest.main() diff --git a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py index 5072939e3e..33dda3be05 100755 --- a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py +++ b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py @@ -6,36 +6,15 @@ import unittest import os -import sys import shutil -from pathlib import Path - -import tempfile from ctsm import unit_testing -from ctsm.path_utils import path_to_ctsm_root -from ctsm.path_utils import path_to_cime -from ctsm.os_utils import run_cmd_output_on_error +from ctsm.test_gen_mksurfdata_jobscript_single_parent import TestFGenMkSurfJobscriptSingleParent from ctsm.toolchain.gen_mksurfdata_jobscript_single import get_parser -from ctsm.toolchain.gen_mksurfdata_jobscript_single import get_mpirun from ctsm.toolchain.gen_mksurfdata_jobscript_single import check_parser_args from ctsm.toolchain.gen_mksurfdata_jobscript_single import write_runscript_part1 -def add_args(machine, nodes, tasks): - """add arguments to sys.argv""" - args_to_add = [ - "--machine", - machine, - "--number-of-nodes", - str(nodes), - "--tasks-per-node", - str(tasks), - ] - for item in args_to_add: - sys.argv.append(item) - - # Allow test names that pylint doesn't like; otherwise hard to make them # readable # pylint: disable=invalid-name @@ -43,65 +22,9 @@ def add_args(machine, nodes, tasks): # pylint: disable=protected-access # pylint: disable=too-many-instance-attributes -class TestFGenMkSurfJobscriptSingle(unittest.TestCase): +class TestFGenMkSurfJobscriptSingle(TestFGenMkSurfJobscriptSingleParent): """Tests the gen_mksurfdata_jobscript_single subroutines""" - def setUp(self): - """Setup for trying out the methods""" - testinputs_path = os.path.join(path_to_ctsm_root(), "python/ctsm/test/testinputs") - self._testinputs_path = testinputs_path - self._previous_dir = os.getcwd() - self._tempdir = tempfile.mkdtemp() - os.chdir(self._tempdir) - self._account = "ACCOUNT_NUMBER" - self._jobscript_file = "output_jobscript" - self._output_compare = """#!/bin/bash -# Edit the batch directives for your batch system -# Below are default batch directives for derecho -#PBS -N mksurfdata -#PBS -j oe -#PBS -k eod -#PBS -S /bin/bash -#PBS -l walltime=12:00:00 -#PBS -A ACCOUNT_NUMBER -#PBS -q main -#PBS -l select=1:ncpus=128:mpiprocs=64:mem=218GB - -# This is a batch script to run a set of resolutions for mksurfdata_esmf input namelist -# NOTE: THIS SCRIPT IS AUTOMATICALLY GENERATED SO IN GENERAL YOU SHOULD NOT EDIT it!! - -""" - self._bld_path = os.path.join(self._tempdir, "tools_bld") - os.makedirs(self._bld_path) - self.assertTrue(os.path.isdir(self._bld_path)) - self._nlfile = os.path.join(self._tempdir, "namelist_file") - Path.touch(self._nlfile) - self.assertTrue(os.path.exists(self._nlfile)) - self._mksurf_exe = os.path.join(self._bld_path, "mksurfdata") - Path.touch(self._mksurf_exe) - self.assertTrue(os.path.exists(self._mksurf_exe)) - self._env_mach = os.path.join(self._bld_path, ".env_mach_specific.sh") - Path.touch(self._env_mach) - self.assertTrue(os.path.exists(self._env_mach)) - sys.argv = [ - "gen_mksurfdata_jobscript_single", - "--bld-path", - self._bld_path, - "--namelist-file", - self._nlfile, - "--jobscript-file", - self._jobscript_file, - "--account", - self._account, - ] - - def tearDown(self): - """ - Remove temporary directory - """ - os.chdir(self._previous_dir) - shutil.rmtree(self._tempdir, ignore_errors=True) - def assertFileContentsEqual(self, expected, filepath, msg=None): """Asserts that the contents of the file given by 'filepath' are equal to the string given by 'expected'. 'msg' gives an optional message to be @@ -119,7 +42,7 @@ def test_simple_derecho_args(self): machine = "derecho" nodes = 1 tasks = 64 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) args = get_parser().parse_args() check_parser_args(args) with open(self._jobscript_file, "w", encoding="utf-8") as runfile: @@ -135,57 +58,12 @@ def test_simple_derecho_args(self): self.assertFileContentsEqual(self._output_compare, self._jobscript_file) - def test_derecho_mpirun(self): - """ - test derecho mpirun. This would've helped caught a problem we ran into - It will also be helpful when sumodules are updated to guide to solutions - to problems - """ - machine = "derecho" - nodes = 4 - tasks = 128 - add_args(machine, nodes, tasks) - args = get_parser().parse_args() - check_parser_args(args) - self.assertEqual(machine, args.machine) - self.assertEqual(tasks, args.tasks_per_node) - self.assertEqual(nodes, args.number_of_nodes) - self.assertEqual(self._account, args.account) - # Create the env_mach_specific.xml file needed for get_mpirun - # This will catch problems with our usage of CIME objects - # Doing this here will also catch potential issues in the gen_mksurfdata_build script - configure_path = os.path.join(path_to_cime(), "CIME", "scripts", "configure") - self.assertTrue(os.path.exists(configure_path)) - options = " --macros-format CMake --silent --compiler intel --machine " + machine - cmd = configure_path + options - cmd_list = cmd.split() - run_cmd_output_on_error( - cmd=cmd_list, errmsg="Trouble running configure", cwd=self._bld_path - ) - self.assertTrue(os.path.exists(self._env_mach)) - expected_attribs = {"mpilib": "default"} - with open(self._jobscript_file, "w", encoding="utf-8") as runfile: - attribs = write_runscript_part1( - number_of_nodes=nodes, - tasks_per_node=tasks, - machine=machine, - account=self._account, - walltime=args.walltime, - runfile=runfile, - ) - self.assertEqual(attribs, expected_attribs) - (executable, mksurfdata_path, env_mach_path) = get_mpirun(args, attribs) - expected_exe = "time mpibind " - self.assertEqual(executable, expected_exe) - self.assertEqual(mksurfdata_path, self._mksurf_exe) - self.assertEqual(env_mach_path, self._env_mach) - def test_too_many_tasks(self): """test trying to use too many tasks""" machine = "derecho" nodes = 1 tasks = 129 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) args = get_parser().parse_args() check_parser_args(args) with open(self._jobscript_file, "w", encoding="utf-8") as runfile: @@ -208,7 +86,7 @@ def test_zero_tasks(self): machine = "derecho" nodes = 5 tasks = 0 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) args = get_parser().parse_args() with self.assertRaisesRegex( SystemExit, @@ -221,7 +99,7 @@ def test_bld_build_path(self): machine = "derecho" nodes = 10 tasks = 64 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) # Remove the build path directory shutil.rmtree(self._bld_path, ignore_errors=True) args = get_parser().parse_args() @@ -233,7 +111,7 @@ def test_mksurfdata_exist(self): machine = "derecho" nodes = 10 tasks = 64 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) args = get_parser().parse_args() os.remove(self._mksurf_exe) with self.assertRaisesRegex(SystemExit, "mksurfdata_esmf executable "): @@ -244,7 +122,7 @@ def test_env_mach_specific_exist(self): machine = "derecho" nodes = 10 tasks = 64 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) args = get_parser().parse_args() os.remove(self._env_mach) with self.assertRaisesRegex(SystemExit, "Environment machine specific file"): @@ -255,7 +133,7 @@ def test_bad_machine(self): machine = "zztop" nodes = 1 tasks = 64 - add_args(machine, nodes, tasks) + unit_testing.add_args(machine, nodes, tasks) with self.assertRaises(SystemExit): get_parser().parse_args() diff --git a/python/ctsm/test_gen_mksurfdata_jobscript_single_parent.py b/python/ctsm/test_gen_mksurfdata_jobscript_single_parent.py new file mode 100755 index 0000000000..b6a3741444 --- /dev/null +++ b/python/ctsm/test_gen_mksurfdata_jobscript_single_parent.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 + +""" +Parent class for some unittest modules relating to gen_mksurfdata_jobscript_single.py +""" + +import unittest +import os +import sys +import shutil +from pathlib import Path + +import tempfile + +from ctsm.path_utils import path_to_ctsm_root + + +# pylint: disable=too-many-instance-attributes +class TestFGenMkSurfJobscriptSingleParent(unittest.TestCase): + """Parent class for some unittest modules relating to gen_mksurfdata_jobscript_single.py""" + + def setUp(self): + """Setup for trying out the methods""" + testinputs_path = os.path.join(path_to_ctsm_root(), "python/ctsm/test/testinputs") + self._testinputs_path = testinputs_path + self._previous_dir = os.getcwd() + self._tempdir = tempfile.mkdtemp() + os.chdir(self._tempdir) + self._account = "ACCOUNT_NUMBER" + self._jobscript_file = "output_jobscript" + self._output_compare = """#!/bin/bash +# Edit the batch directives for your batch system +# Below are default batch directives for derecho +#PBS -N mksurfdata +#PBS -j oe +#PBS -k eod +#PBS -S /bin/bash +#PBS -l walltime=12:00:00 +#PBS -A ACCOUNT_NUMBER +#PBS -q main +#PBS -l select=1:ncpus=128:mpiprocs=64:mem=218GB + +# This is a batch script to run a set of resolutions for mksurfdata_esmf input namelist +# NOTE: THIS SCRIPT IS AUTOMATICALLY GENERATED SO IN GENERAL YOU SHOULD NOT EDIT it!! + +""" + self._bld_path = os.path.join(self._tempdir, "tools_bld") + os.makedirs(self._bld_path) + self.assertTrue(os.path.isdir(self._bld_path)) + self._nlfile = os.path.join(self._tempdir, "namelist_file") + Path.touch(self._nlfile) + self.assertTrue(os.path.exists(self._nlfile)) + self._mksurf_exe = os.path.join(self._bld_path, "mksurfdata") + Path.touch(self._mksurf_exe) + self.assertTrue(os.path.exists(self._mksurf_exe)) + self._env_mach = os.path.join(self._bld_path, ".env_mach_specific.sh") + Path.touch(self._env_mach) + self.assertTrue(os.path.exists(self._env_mach)) + sys.argv = [ + "gen_mksurfdata_jobscript_single", + "--bld-path", + self._bld_path, + "--namelist-file", + self._nlfile, + "--jobscript-file", + self._jobscript_file, + "--account", + self._account, + ] + + def tearDown(self): + """ + Remove temporary directory + """ + os.chdir(self._previous_dir) + shutil.rmtree(self._tempdir, ignore_errors=True) diff --git a/python/ctsm/unit_testing.py b/python/ctsm/unit_testing.py index d3a308c796..04c4482e31 100644 --- a/python/ctsm/unit_testing.py +++ b/python/ctsm/unit_testing.py @@ -1,8 +1,23 @@ """Functions to aid unit tests""" +import sys from ctsm.ctsm_logging import setup_logging_for_tests +def add_args(machine, nodes, tasks): + """add arguments to sys.argv""" + args_to_add = [ + "--machine", + machine, + "--number-of-nodes", + str(nodes), + "--tasks-per-node", + str(tasks), + ] + for item in args_to_add: + sys.argv.append(item) + + def setup_for_tests(enable_critical_logs=False): """Call this at the beginning of unit testing From 9c4aa34be1bbb0876e2a65d5550581620b57cc10 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 15:18:42 -0600 Subject: [PATCH 5/8] Add GitHub workflow to run Python unit tests. --- .github/workflows/python-tests.yml | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 .github/workflows/python-tests.yml diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml new file mode 100644 index 0000000000..d58c26def6 --- /dev/null +++ b/.github/workflows/python-tests.yml @@ -0,0 +1,39 @@ +name: Run Python tests + +on: + push: + # Run when a change to these files is pushed to any branch. Without the "branches:" line, for some reason this will be run whenever a tag is pushed, even if the listed files aren't changed. + branches: ['*'] + paths: + - 'python/**' + - 'cime_config/SystemTests/**' + - 'cime_config/buildlib/**' + - 'cime_config/buildnml/**' + pull_request: + # Run on pull requests that change the listed files + paths: + - 'python/**' + - 'cime_config/SystemTests/**' + - 'cime_config/buildlib/**' + - 'cime_config/buildnml/**' + +jobs: + python-unit-tests: + runs-on: ubuntu-latest + steps: + # Checkout the code + - uses: actions/checkout@v4 + + # Set up the conda environment + - uses: conda-incubator/setup-miniconda@v3 + with: + activate-environment: ctsm_pylib + environment-file: python/conda_env_ctsm_py.yml + channels: conda-forge + auto-activate-base: false + + # Run Python unit tests check + - name: Run Python unit tests + run: | + cd python + conda run -n ctsm_pylib ./run_ctsm_py_tests -u From ac3b0a5b7fceed3ab81fb203c1d0a120aecbf4e4 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 6 Jun 2025 15:20:20 -0600 Subject: [PATCH 6/8] Move Python lint/black check into python-tests.yml. Since they were working on the same triggers. --- .github/workflows/formatting_python.yml | 47 ------------------------- .github/workflows/python-tests.yml | 28 +++++++++++++++ 2 files changed, 28 insertions(+), 47 deletions(-) delete mode 100644 .github/workflows/formatting_python.yml diff --git a/.github/workflows/formatting_python.yml b/.github/workflows/formatting_python.yml deleted file mode 100644 index 131e44a7af..0000000000 --- a/.github/workflows/formatting_python.yml +++ /dev/null @@ -1,47 +0,0 @@ -name: Check Python formatting - -on: - push: - # Run when a change to these files is pushed to any branch. Without the "branches:" line, for some reason this will be run whenever a tag is pushed, even if the listed files aren't changed. - branches: ['*'] - paths: - - 'python/**' - - 'cime_config/SystemTests/**' - - 'cime_config/buildlib/**' - - 'cime_config/buildnml/**' - pull_request: - # Run on pull requests that change the listed files - paths: - - 'python/**' - - 'cime_config/SystemTests/**' - - 'cime_config/buildlib/**' - - 'cime_config/buildnml/**' - -jobs: - lint-and-format-check: - runs-on: ubuntu-latest - steps: - # Checkout the code - - uses: actions/checkout@v4 - - # Set up the conda environment - - uses: conda-incubator/setup-miniconda@v3 - with: - activate-environment: ctsm_pylib - environment-file: python/conda_env_ctsm_py.yml - channels: conda-forge - auto-activate-base: false - - # Run pylint check - - name: Run pylint - run: | - cd python - conda run -n ctsm_pylib make lint - - # Run black check - - name: Run black - # Run this step even if previous step(s) failed - if: success() || failure() - run: | - cd python - conda run -n ctsm_pylib make black diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index d58c26def6..a13e594acd 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -37,3 +37,31 @@ jobs: run: | cd python conda run -n ctsm_pylib ./run_ctsm_py_tests -u + + python-lint-and-black: + runs-on: ubuntu-latest + steps: + # Checkout the code + - uses: actions/checkout@v4 + + # Set up the conda environment + - uses: conda-incubator/setup-miniconda@v3 + with: + activate-environment: ctsm_pylib + environment-file: python/conda_env_ctsm_py.yml + channels: conda-forge + auto-activate-base: false + + # Run pylint check + - name: Run pylint + run: | + cd python + conda run -n ctsm_pylib make lint + + # Run black check + - name: Run black + # Run this step even if previous step(s) failed + if: success() || failure() + run: | + cd python + conda run -n ctsm_pylib make black From a976348a3604778656da8f1cf4b7510f08604369 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 26 Jun 2025 16:11:11 -0600 Subject: [PATCH 7/8] unit_testing.py: Simplify add_args(). --- python/ctsm/unit_testing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/ctsm/unit_testing.py b/python/ctsm/unit_testing.py index 04c4482e31..d72a37842a 100644 --- a/python/ctsm/unit_testing.py +++ b/python/ctsm/unit_testing.py @@ -14,8 +14,7 @@ def add_args(machine, nodes, tasks): "--tasks-per-node", str(tasks), ] - for item in args_to_add: - sys.argv.append(item) + sys.argv += args_to_add def setup_for_tests(enable_critical_logs=False): From 71cb2d103347b1746a1c4a341b3f50bae1a8e418 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 26 Jun 2025 16:12:21 -0600 Subject: [PATCH 8/8] unit_testing.py: Rename add_args() to add_machine_node_args(). --- ..._sys_gen_mksurfdata_jobscript_single_derecho.py | 2 +- .../test_unit_gen_mksurfdata_jobscript_single.py | 14 +++++++------- python/ctsm/unit_testing.py | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py b/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py index 1813529d0b..627fb1d32b 100755 --- a/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py +++ b/python/ctsm/test/test_sys_gen_mksurfdata_jobscript_single_derecho.py @@ -36,7 +36,7 @@ def test_derecho_mpirun(self): machine = "derecho" nodes = 4 tasks = 128 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() check_parser_args(args) self.assertEqual(machine, args.machine) diff --git a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py index 33dda3be05..3980b3bd49 100755 --- a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py +++ b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py @@ -42,7 +42,7 @@ def test_simple_derecho_args(self): machine = "derecho" nodes = 1 tasks = 64 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() check_parser_args(args) with open(self._jobscript_file, "w", encoding="utf-8") as runfile: @@ -63,7 +63,7 @@ def test_too_many_tasks(self): machine = "derecho" nodes = 1 tasks = 129 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() check_parser_args(args) with open(self._jobscript_file, "w", encoding="utf-8") as runfile: @@ -86,7 +86,7 @@ def test_zero_tasks(self): machine = "derecho" nodes = 5 tasks = 0 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() with self.assertRaisesRegex( SystemExit, @@ -99,7 +99,7 @@ def test_bld_build_path(self): machine = "derecho" nodes = 10 tasks = 64 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) # Remove the build path directory shutil.rmtree(self._bld_path, ignore_errors=True) args = get_parser().parse_args() @@ -111,7 +111,7 @@ def test_mksurfdata_exist(self): machine = "derecho" nodes = 10 tasks = 64 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() os.remove(self._mksurf_exe) with self.assertRaisesRegex(SystemExit, "mksurfdata_esmf executable "): @@ -122,7 +122,7 @@ def test_env_mach_specific_exist(self): machine = "derecho" nodes = 10 tasks = 64 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() os.remove(self._env_mach) with self.assertRaisesRegex(SystemExit, "Environment machine specific file"): @@ -133,7 +133,7 @@ def test_bad_machine(self): machine = "zztop" nodes = 1 tasks = 64 - unit_testing.add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) with self.assertRaises(SystemExit): get_parser().parse_args() diff --git a/python/ctsm/unit_testing.py b/python/ctsm/unit_testing.py index d72a37842a..8370830b4d 100644 --- a/python/ctsm/unit_testing.py +++ b/python/ctsm/unit_testing.py @@ -4,7 +4,7 @@ from ctsm.ctsm_logging import setup_logging_for_tests -def add_args(machine, nodes, tasks): +def add_machine_node_args(machine, nodes, tasks): """add arguments to sys.argv""" args_to_add = [ "--machine",