diff --git a/.github/workflows/formatting_python.yml b/.github/workflows/python-tests.yml similarity index 68% rename from .github/workflows/formatting_python.yml rename to .github/workflows/python-tests.yml index 131e44a7af..a13e594acd 100644 --- a/.github/workflows/formatting_python.yml +++ b/.github/workflows/python-tests.yml @@ -1,4 +1,4 @@ -name: Check Python formatting +name: Run Python tests on: push: @@ -18,7 +18,27 @@ on: - 'cime_config/buildnml/**' jobs: - lint-and-format-check: + 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 + + python-lint-and-black: runs-on: ubuntu-latest steps: # Checkout the code 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/subset_data.py b/python/ctsm/subset_data.py index de4e51db9b..1fe7f8c580 100644 --- a/python/ctsm/subset_data.py +++ b/python/ctsm/subset_data.py @@ -605,7 +605,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 """ @@ -623,9 +623,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_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..627fb1d32b --- /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_machine_node_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 bee1aac715..3980b3bd49 100755 --- a/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py +++ b/python/ctsm/test/test_unit_gen_mksurfdata_jobscript_single.py @@ -6,40 +6,15 @@ import unittest import os -import sys import shutil -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) - - -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 @@ -47,65 +22,9 @@ def create_empty_file(filename): # 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") - create_empty_file(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) - 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) - 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 @@ -123,7 +42,7 @@ def test_simple_derecho_args(self): machine = "derecho" nodes = 1 tasks = 64 - 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: @@ -139,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_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: @@ -212,7 +86,7 @@ def test_zero_tasks(self): machine = "derecho" nodes = 5 tasks = 0 - add_args(machine, nodes, tasks) + unit_testing.add_machine_node_args(machine, nodes, tasks) args = get_parser().parse_args() with self.assertRaisesRegex( SystemExit, @@ -225,7 +99,7 @@ def test_bld_build_path(self): machine = "derecho" nodes = 10 tasks = 64 - 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() @@ -237,7 +111,7 @@ def test_mksurfdata_exist(self): machine = "derecho" nodes = 10 tasks = 64 - 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 "): @@ -248,7 +122,7 @@ def test_env_mach_specific_exist(self): machine = "derecho" nodes = 10 tasks = 64 - 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"): @@ -259,7 +133,7 @@ def test_bad_machine(self): machine = "zztop" nodes = 1 tasks = 64 - 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/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) diff --git a/python/ctsm/test/test_unit_subset_data.py b/python/ctsm/test/test_unit_subset_data.py index c4ce21e959..a127a282e0 100755 --- a/python/ctsm/test/test_unit_subset_data.py +++ b/python/ctsm/test/test_unit_subset_data.py @@ -104,7 +104,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", @@ -184,7 +184,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", 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..8370830b4d 100644 --- a/python/ctsm/unit_testing.py +++ b/python/ctsm/unit_testing.py @@ -1,8 +1,22 @@ """Functions to aid unit tests""" +import sys from ctsm.ctsm_logging import setup_logging_for_tests +def add_machine_node_args(machine, nodes, tasks): + """add arguments to sys.argv""" + args_to_add = [ + "--machine", + machine, + "--number-of-nodes", + str(nodes), + "--tasks-per-node", + str(tasks), + ] + sys.argv += args_to_add + + def setup_for_tests(enable_critical_logs=False): """Call this at the beginning of unit testing