From b2380bf71ffbeecdeac2977a4c36b376739a2b4d Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Wed, 3 Apr 2024 17:53:37 -0700 Subject: [PATCH 1/9] Refactors how/when rebuilds are required --- CIME/SystemTests/system_tests_common.py | 4 +- CIME/Tools/check_case | 5 +- CIME/Tools/check_lockedfiles | 3 +- CIME/Tools/xmlchange | 27 +--- CIME/XML/env_batch.py | 11 +- CIME/build.py | 8 +- CIME/case/case.py | 5 - CIME/case/case_run.py | 7 +- CIME/case/case_setup.py | 21 ++- CIME/case/case_submit.py | 29 ++-- CIME/case/check_lockedfiles.py | 151 --------------------- CIME/locked_files.py | 168 ++++++++++++++++++++++-- CIME/tests/test_unit_case.py | 2 +- CIME/tests/test_unit_locked_files.py | 89 +++++++++++++ 14 files changed, 312 insertions(+), 218 deletions(-) delete mode 100644 CIME/case/check_lockedfiles.py create mode 100644 CIME/tests/test_unit_locked_files.py diff --git a/CIME/SystemTests/system_tests_common.py b/CIME/SystemTests/system_tests_common.py index 236b34d0301..13057dd52e9 100644 --- a/CIME/SystemTests/system_tests_common.py +++ b/CIME/SystemTests/system_tests_common.py @@ -131,10 +131,10 @@ def _init_locked_files(self, caseroot, expected): env_run.xml file. If it does exist, restore values changed in a previous run of the test. """ - if is_locked("env_run.orig.xml"): + if is_locked("env_run.orig.xml", caseroot): self.compare_env_run(expected=expected) elif os.path.isfile(os.path.join(caseroot, "env_run.xml")): - lock_file("env_run.xml", caseroot=caseroot, newname="env_run.orig.xml") + lock_file("env_run.xml", caseroot, newname="env_run.orig.xml") def _resetup_case(self, phase, reset=False): """ diff --git a/CIME/Tools/check_case b/CIME/Tools/check_case index 9954b33dc0c..9b061046643 100755 --- a/CIME/Tools/check_case +++ b/CIME/Tools/check_case @@ -21,6 +21,7 @@ from standard_script_setup import * from CIME.utils import expect from CIME.case import Case +from CIME.locked_files import check_lockedfiles import argparse @@ -45,8 +46,10 @@ def _main_func(description): parse_command_line(sys.argv, description) with Case(read_only=False, record=True) as case: - case.check_lockedfiles() + check_lockedfiles(case) + case.create_namelists() + build_complete = case.get_value("BUILD_COMPLETE") if not build_complete: diff --git a/CIME/Tools/check_lockedfiles b/CIME/Tools/check_lockedfiles index ec279754388..a959b953570 100755 --- a/CIME/Tools/check_lockedfiles +++ b/CIME/Tools/check_lockedfiles @@ -5,6 +5,7 @@ This script compares xml files from standard_script_setup import * from CIME.case import Case +from CIME.locked_files import check_lockedfiles def parse_command_line(args, description): @@ -38,7 +39,7 @@ def _main_func(description): caseroot = parse_command_line(sys.argv, description) with Case(case_root=caseroot, read_only=True) as case: - case.check_lockedfiles() + check_lockedfiles(case) if __name__ == "__main__": diff --git a/CIME/Tools/xmlchange b/CIME/Tools/xmlchange index da9619a8fce..cc6b216a825 100755 --- a/CIME/Tools/xmlchange +++ b/CIME/Tools/xmlchange @@ -55,6 +55,7 @@ from CIME.utils import ( get_batch_script_for_job, ) from CIME.case import Case +from CIME.locked_files import check_lockedfiles import re @@ -223,32 +224,10 @@ def xmlchange_single_value( result = case.set_value( xmlid, xmlval, subgroup, ignore_type=force, return_file=True ) - expect(result is not None, 'No variable "%s" found' % xmlid) - filename = result[1] - - setup_already_run = os.path.exists( - get_batch_script_for_job(case.get_primary_job()) - ) - build_already_run = case.get_value("BUILD_COMPLETE") - - if filename.endswith("env_build.xml") and build_already_run: - logger.info( - """For your changes to take effect, run: -./case.build --clean-all -./case.build""" - ) - - elif filename.endswith("env_mach_pes.xml"): - if setup_already_run: - logger.info( - "For your changes to take effect, run:\n./case.setup --reset" - ) - if build_already_run: - if not setup_already_run: - logger.info("For your changes to take effect, run:") - logger.info("./case.build --clean-all\n./case.build") + expect(result is not None, 'No variable "%s" found' % xmlid) + check_lockedfiles(case, skip=["env_case"]) else: logger.warning("'%s' = '%s'", xmlid, xmlval) diff --git a/CIME/XML/env_batch.py b/CIME/XML/env_batch.py index 19d578a389a..2c705555fea 100644 --- a/CIME/XML/env_batch.py +++ b/CIME/XML/env_batch.py @@ -17,7 +17,6 @@ format_time, add_flag_to_cmd, ) -from CIME.locked_files import lock_file, unlock_file from collections import OrderedDict import stat, re, math import pathlib @@ -190,13 +189,19 @@ def set_batch_system(self, batchobj, batch_system_type=None): if batchobj.batch_system_node is not None: self.add_child(self.copy(batchobj.batch_system_node)) + if batchobj.machine_node is not None: self.add_child(self.copy(batchobj.machine_node)) + + from CIME.locked_files import lock_file, unlock_file + if os.path.exists(os.path.join(self._caseroot, "LockedFiles", "env_batch.xml")): - unlock_file(os.path.basename(batchobj.filename), caseroot=self._caseroot) + unlock_file(os.path.basename(batchobj.filename), self._caseroot) + self.set_value("BATCH_SYSTEM", batch_system_type) + if os.path.exists(os.path.join(self._caseroot, "LockedFiles")): - lock_file(os.path.basename(batchobj.filename), caseroot=self._caseroot) + lock_file(os.path.basename(batchobj.filename), self._caseroot) def get_job_overrides(self, job, case): env_workflow = case.get_env("workflow") diff --git a/CIME/build.py b/CIME/build.py index 3f5c57ca998..3845de47f2d 100644 --- a/CIME/build.py +++ b/CIME/build.py @@ -20,7 +20,7 @@ import_from_file, ) from CIME.config import Config -from CIME.locked_files import lock_file, unlock_file +from CIME.locked_files import lock_file, unlock_file, check_lockedfiles from CIME.XML.files import Files logger = logging.getLogger(__name__) @@ -1026,7 +1026,7 @@ def _clean_impl(case, cleanlist, clean_all, clean_depends): run_cmd_no_fail(clean_cmd) # unlink Locked files directory - unlock_file("env_build.xml") + unlock_file("env_build.xml", case.get_value("CASEROOT")) # reset following values in xml files case.set_value("SMP_BUILD", str(0)) @@ -1076,7 +1076,7 @@ def _case_build_impl( comp_classes = case.get_values("COMP_CLASSES") - case.check_lockedfiles(skip="env_batch") + check_lockedfiles(case, skip="env_batch") # Retrieve relevant case data # This environment variable gets set for cesm Make and @@ -1292,7 +1292,7 @@ def post_build(case, logs, build_complete=False, save_build_provenance=True): case.flush() - lock_file("env_build.xml", caseroot=case.get_value("CASEROOT")) + lock_file("env_build.xml", case.get_value("CASEROOT")) ############################################################################### diff --git a/CIME/case/case.py b/CIME/case/case.py index e08b5ffe2c4..7aab433b9a1 100644 --- a/CIME/case/case.py +++ b/CIME/case/case.py @@ -90,11 +90,6 @@ class Case(object): ) from CIME.case.case_run import case_run from CIME.case.case_cmpgen_namelists import case_cmpgen_namelists - from CIME.case.check_lockedfiles import ( - check_lockedfile, - check_lockedfiles, - check_pelayouts_require_rebuild, - ) from CIME.case.preview_namelists import create_dirs, create_namelists from CIME.case.check_input_data import ( check_all_input_data, diff --git a/CIME/case/case_run.py b/CIME/case/case_run.py index 9dd92ec4876..96c4796d112 100644 --- a/CIME/case/case_run.py +++ b/CIME/case/case_run.py @@ -7,6 +7,7 @@ from CIME.utils import run_sub_or_cmd, append_status, safe_copy, model_log, CIMEError from CIME.utils import batch_jobid, is_comp_standalone from CIME.get_timing import get_timing +from CIME.locked_files import check_lockedfiles import shutil, time, sys, os, glob @@ -34,10 +35,14 @@ def _pre_run_check(case, lid, skip_pnl=False, da_cycle=0): # check for locked files, may impact BUILD_COMPLETE skip = None + if case.get_value("EXTERNAL_WORKFLOW"): skip = "env_batch" - case.check_lockedfiles(skip=skip) + + check_lockedfiles(case, skip=skip) + logger.debug("check_lockedfiles OK") + build_complete = case.get_value("BUILD_COMPLETE") # check that build is done diff --git a/CIME/case/case_setup.py b/CIME/case/case_setup.py index a170d1bfddd..2ebfa37b0af 100644 --- a/CIME/case/case_setup.py +++ b/CIME/case/case_setup.py @@ -22,7 +22,7 @@ ) from CIME.utils import batch_jobid from CIME.test_status import * -from CIME.locked_files import unlock_file, lock_file +from CIME.locked_files import unlock_file, lock_file, check_lockedfiles import errno, shutil @@ -348,13 +348,15 @@ def _case_setup_impl( case.set_value("BUILD_THREADED", case.get_build_threaded()) else: - case.check_pelayouts_require_rebuild(models) + caseroot = case.get_value("CASEROOT") - unlock_file("env_build.xml") - unlock_file("env_batch.xml") + unlock_file("env_build.xml", caseroot) + + unlock_file("env_batch.xml", caseroot) case.flush() - case.check_lockedfiles() + + check_lockedfiles(case, skip=["env_build", "env_mach_pes"]) case.initialize_derived_attributes() @@ -404,9 +406,14 @@ def _case_setup_impl( # Make a copy of env_mach_pes.xml in order to be able # to check that it does not change once case.setup is invoked case.flush() + logger.debug("at copy TOTALPES = {}".format(case.get_value("TOTALPES"))) - lock_file("env_mach_pes.xml") - lock_file("env_batch.xml") + + caseroot = case.get_value("CASEROOT") + + lock_file("env_mach_pes.xml", caseroot) + + lock_file("env_batch.xml", caseroot) # Create user_nl files for the required number of instances if not os.path.exists("user_nl_cpl"): diff --git a/CIME/case/case_submit.py b/CIME/case/case_submit.py index e3f30654814..037a08f0615 100644 --- a/CIME/case/case_submit.py +++ b/CIME/case/case_submit.py @@ -9,7 +9,12 @@ import configparser from CIME.XML.standard_module_setup import * from CIME.utils import expect, run_and_log_case_status, CIMEError, get_time_in_seconds -from CIME.locked_files import unlock_file, lock_file +from CIME.locked_files import ( + unlock_file, + lock_file, + check_lockedfile, + check_lockedfiles, +) from CIME.test_status import * logger = logging.getLogger(__name__) @@ -95,15 +100,14 @@ def _submit( batch_system = env_batch.get_batch_system_type() if batch_system != case.get_value("BATCH_SYSTEM"): - unlock_file(os.path.basename(env_batch.filename), caseroot=caseroot) + unlock_file(os.path.basename(env_batch.filename), caseroot) + case.set_value("BATCH_SYSTEM", batch_system) env_batch_has_changed = False if not external_workflow: try: - case.check_lockedfile( - os.path.basename(env_batch.filename), caseroot=caseroot - ) + check_lockedfile(case, os.path.basename(env_batch.filename)) except: env_batch_has_changed = True @@ -116,8 +120,10 @@ def _submit( """ ) env_batch.make_all_batch_files(case) + case.flush() - lock_file(os.path.basename(env_batch.filename), caseroot=caseroot) + + lock_file(os.path.basename(env_batch.filename), caseroot) if resubmit: # This is a resubmission, do not reinitialize test values @@ -143,7 +149,7 @@ def _submit( env_batch_has_changed = False try: - case.check_lockedfile(os.path.basename(env_batch.filename)) + check_lockedfile(case, os.path.basename(env_batch.filename)) except CIMEError: env_batch_has_changed = True @@ -157,10 +163,12 @@ def _submit( ) env_batch.make_all_batch_files(case) - unlock_file(os.path.basename(env_batch.filename), caseroot=caseroot) - lock_file(os.path.basename(env_batch.filename), caseroot=caseroot) + unlock_file(os.path.basename(env_batch.filename), caseroot) + + lock_file(os.path.basename(env_batch.filename), caseroot) case.check_case(skip_pnl=skip_pnl, chksum=chksum) + if job == case.get_primary_job(): case.check_DA_settings() @@ -287,7 +295,8 @@ def submit( def check_case(self, skip_pnl=False, chksum=False): - self.check_lockedfiles() + check_lockedfiles(self) + if not skip_pnl: self.create_namelists() # Must be called before check_all_input_data diff --git a/CIME/case/check_lockedfiles.py b/CIME/case/check_lockedfiles.py deleted file mode 100644 index d9a8995ac35..00000000000 --- a/CIME/case/check_lockedfiles.py +++ /dev/null @@ -1,151 +0,0 @@ -""" -API for checking locked files -check_lockedfile, check_lockedfiles, check_pelayouts_require_rebuild are members -of Class case.py from file case.py -""" - -from CIME.XML.standard_module_setup import * -from CIME.XML.env_build import EnvBuild -from CIME.XML.env_case import EnvCase -from CIME.XML.env_mach_pes import EnvMachPes -from CIME.XML.env_batch import EnvBatch -from CIME.locked_files import unlock_file, LOCKED_DIR -from CIME.build import clean - -logger = logging.getLogger(__name__) - -import glob - - -def check_pelayouts_require_rebuild(self, models): - """ - Create if we require a rebuild, expects cwd is caseroot - """ - locked_pes = os.path.join(LOCKED_DIR, "env_mach_pes.xml") - if os.path.exists(locked_pes): - # Look to see if $comp_PE_CHANGE_REQUIRES_REBUILD is defined - # for any component - env_mach_pes_locked = EnvMachPes( - infile=locked_pes, components=self.get_values("COMP_CLASSES") - ) - for comp in models: - if self.get_value("{}_PE_CHANGE_REQUIRES_REBUILD".format(comp)): - # Changing these values in env_mach_pes.xml will force - # you to clean the corresponding component - old_tasks = env_mach_pes_locked.get_value("NTASKS_{}".format(comp)) - old_threads = env_mach_pes_locked.get_value("NTHRDS_{}".format(comp)) - old_inst = env_mach_pes_locked.get_value("NINST_{}".format(comp)) - - new_tasks = self.get_value("NTASKS_{}".format(comp)) - new_threads = self.get_value("NTHRDS_{}".format(comp)) - new_inst = self.get_value("NINST_{}".format(comp)) - - if ( - old_tasks != new_tasks - or old_threads != new_threads - or old_inst != new_inst - ): - logging.warning( - "{} pe change requires clean build {} {}".format( - comp, old_tasks, new_tasks - ) - ) - cleanflag = comp.lower() - clean(self, cleanlist=[cleanflag]) - - unlock_file("env_mach_pes.xml", self.get_value("CASEROOT")) - - -def check_lockedfile(self, filebase): - caseroot = self.get_value("CASEROOT") - - cfile = os.path.join(caseroot, filebase) - lfile = os.path.join(caseroot, "LockedFiles", filebase) - components = self.get_values("COMP_CLASSES") - if os.path.isfile(cfile): - objname = filebase.split(".")[0] - if objname == "env_build": - f1obj = self.get_env("build") - f2obj = EnvBuild(caseroot, lfile, read_only=True) - elif objname == "env_mach_pes": - f1obj = self.get_env("mach_pes") - f2obj = EnvMachPes(caseroot, lfile, components=components, read_only=True) - elif objname == "env_case": - f1obj = self.get_env("case") - f2obj = EnvCase(caseroot, lfile, read_only=True) - elif objname == "env_batch": - f1obj = self.get_env("batch") - f2obj = EnvBatch(caseroot, lfile, read_only=True) - else: - logging.warning( - "Locked XML file '{}' is not current being handled".format(filebase) - ) - return - - diffs = f1obj.compare_xml(f2obj) - if diffs: - - logging.warning("File {} has been modified".format(lfile)) - toggle_build_status = False - for key in diffs.keys(): - if key != "BUILD_COMPLETE": - logging.warning( - " found difference in {} : case {} locked {}".format( - key, repr(diffs[key][0]), repr(diffs[key][1]) - ) - ) - toggle_build_status = True - if objname == "env_mach_pes": - expect(False, "Invoke case.setup --reset ") - elif objname == "env_case": - expect( - False, - "Cannot change file env_case.xml, please" - " recover the original copy from LockedFiles", - ) - elif objname == "env_build": - if toggle_build_status: - logging.warning("Setting build complete to False") - self.set_value("BUILD_COMPLETE", False) - if "PIO_VERSION" in diffs: - self.set_value("BUILD_STATUS", 2) - logging.critical( - "Changing PIO_VERSION requires running " - "case.build --clean-all and rebuilding" - ) - else: - self.set_value("BUILD_STATUS", 1) - - elif objname == "env_batch": - expect( - False, - "Batch configuration has changed, please run case.setup --reset", - ) - else: - expect(False, "'{}' diff was not handled".format(objname)) - - -def check_lockedfiles(self, skip=None): - """ - Check that all lockedfiles match what's in case - - If caseroot is not specified, it is set to the current working directory - """ - caseroot = self.get_value("CASEROOT") - lockedfiles = glob.glob(os.path.join(caseroot, "LockedFiles", "*.xml")) - skip = [] if skip is None else skip - skip = [skip] if isinstance(skip, str) else skip - for lfile in lockedfiles: - fpart = os.path.basename(lfile) - # ignore files used for tests such as env_mach_pes.ERP1.xml by looking for extra dots in the name - if fpart.count(".") > 1: - continue - - do_skip = False - for item in skip: - if fpart.startswith(item): - do_skip = True - break - - if not do_skip: - self.check_lockedfile(fpart) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index 784b5674941..7579f9ca1d9 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -1,17 +1,28 @@ -from CIME.XML.standard_module_setup import * +import glob +from pathlib import Path + from CIME.utils import safe_copy +from CIME.XML.standard_module_setup import * +from CIME.XML.env_build import EnvBuild +from CIME.XML.env_mach_pes import EnvMachPes +from CIME.XML.env_case import EnvCase +from CIME.XML.env_batch import EnvBatch from CIME.XML.generic_xml import GenericXML logger = logging.getLogger(__name__) + LOCKED_DIR = "LockedFiles" -def lock_file(filename, caseroot=None, newname=None): +def lock_file(filename, caseroot, newname=None): expect("/" not in filename, "Please just provide basename of locked file") - caseroot = os.getcwd() if caseroot is None else caseroot - newname = filename if newname is None else newname + + if newname is None: + newname = filename + fulllockdir = os.path.join(caseroot, LOCKED_DIR) + if not os.path.exists(fulllockdir): os.mkdir(fulllockdir) @@ -23,20 +34,161 @@ def lock_file(filename, caseroot=None, newname=None): # have involved this file. We should probably seek a safer way of locking # files. safe_copy(os.path.join(caseroot, filename), os.path.join(fulllockdir, newname)) + GenericXML.invalidate(os.path.join(fulllockdir, newname)) -def unlock_file(filename, caseroot=None): +def unlock_file(filename, caseroot): expect("/" not in filename, "Please just provide basename of locked file") - caseroot = os.getcwd() if caseroot is None else caseroot + locked_path = os.path.join(caseroot, LOCKED_DIR, filename) + if os.path.exists(locked_path): os.remove(locked_path) logging.debug("Unlocking file {}".format(filename)) -def is_locked(filename, caseroot=None): +def is_locked(filename, caseroot): expect("/" not in filename, "Please just provide basename of locked file") - caseroot = os.getcwd() if caseroot is None else caseroot + return os.path.exists(os.path.join(caseroot, LOCKED_DIR, filename)) + + +def check_lockedfiles(case, skip=None): + """ + Check that all lockedfiles match what's in case + + If caseroot is not specified, it is set to the current working directory + """ + if skip is None: + skip = [] + elif isinstance(skip, str): + skip = [skip] + + caseroot = case.get_value("CASEROOT") + + lockedfiles = glob.glob(os.path.join(caseroot, LOCKED_DIR, "*.xml")) + + for file_path in lockedfiles: + filename = os.path.basename(file_path) + + # Skip files used for tests e.g. env_mach_pes.ERP1.xml or included in skip list + if filename.count(".") > 1 or any([filename.startswith(x) for x in skip]): + continue + + env_name, diff = diff_lockedfile(case, caseroot, filename) + + if diff: + check_diff(case, filename, env_name, diff) + + +def check_lockedfile(case, filebase): + caseroot = case.get_value("CASEROOT") + + env_name, diff = diff_lockedfile(case, caseroot, filebase) + + if diff: + check_diff(case, filebase, env_name, diff) + + +def diff_lockedfile(case, caseroot, filename): + env_name = filename.split(".")[0] + + case_file = Path(caseroot, filename) + + locked_file = case_file.parent / LOCKED_DIR / filename + + if not locked_file.is_file(): + return env_name, {} + + if env_name == "env_build": + l_env = case.get_env("build") + r_env = EnvBuild(caseroot, str(locked_file), read_only=True) + elif env_name == "env_mach_pes": + l_env = case.get_env("mach_pes") + r_env = EnvMachPes( + caseroot, + str(locked_file), + components=case.get_values("COMP_CLASSES"), + read_only=True, + ) + elif env_name == "env_case": + l_env = case.get_env("case") + r_env = EnvCase(caseroot, str(locked_file), read_only=True) + elif env_name == "env_batch": + l_env = case.get_env("batch") + r_env = EnvBatch(caseroot, str(locked_file), read_only=True) + else: + logger.warning( + "Locked XML file {!r} is not currently being handled".format(filename) + ) + + return env_name, {} + + return env_name, l_env.compare_xml(r_env) + + +def check_diff(case, filename, env_name, diff): + logger.warning("Detected diff in locked file {!r}".format(filename)) + + # Remove BUILD_COMPLETE, invalid entry in diff + diff.pop("BUILD_COMPLETE", None) + + # List differences + for key, value in diff.items(): + logger.warning( + "\t{!r} has changed from {!r} to {!r}".format(key, value[1], value[0]) + ) + + if env_name == "env_case": + expect( + False, + f"Cannot change `env_case.xml`, please restore origin {filename!r}", + ) + elif env_name == "env_build": + toggle_build_status = True if diff else False + + if toggle_build_status: + logger.warning("Setting 'BUILD_COMPLETE' to False") + + case.set_value("BUILD_COMPLETE", False) + + build_status = 1 + + if "PIO_VERSION" in diff: + build_status = 2 + + logging.critical( + "Changing 'PIO_VERSION' requires running `./case.build --clean-all` to rebuild" + ) + + case.set_value("BUILD_STATUS", build_status) + elif env_name == "env_batch": + expect( + False, + "Batch configuration has changed, please run `./case.setup --reset`", + ) + else: + toggle_build_status = False + + rebuild_components = [] + + for component in case.get_values("COMP_CLASSES"): + triggers = case.get_values(f"REBUILD_TRIGGER_{component}") + + if any([y.startswith(x) for x in triggers for y in diff.keys()]): + toggle_build_status = True + + rebuild_components.append(component) + + if toggle_build_status: + case.set_value("BUILD_COMPLETE", False) + + clean_targets = " ".join([x.lower() for x in rebuild_components]) + clean_cmd = f"./case.build --clean {clean_targets}" + + expect( + False, + f"A rebuild is required, please run: \n./case.setup --reset\n{clean_cmd}\n./case.build", + ) diff --git a/CIME/tests/test_unit_case.py b/CIME/tests/test_unit_case.py index b14458a8dea..abc2acff8ee 100755 --- a/CIME/tests/test_unit_case.py +++ b/CIME/tests/test_unit_case.py @@ -23,7 +23,7 @@ class TestCaseSubmit(unittest.TestCase): def test_check_case(self): case = mock.MagicMock() # get_value arguments TEST, COMP_WAV, COMP_INTERFACE, BUILD_COMPLETE - case.get_value.side_effect = ["", "", True] + case.get_value.side_effect = ["/tmp/caseroot", "", "", True] case_submit.check_case(case, chksum=True) case.check_all_input_data.assert_called_with(chksum=True) diff --git a/CIME/tests/test_unit_locked_files.py b/CIME/tests/test_unit_locked_files.py new file mode 100644 index 00000000000..2a16ed0fc7d --- /dev/null +++ b/CIME/tests/test_unit_locked_files.py @@ -0,0 +1,89 @@ +import tempfile +import unittest +from unittest import mock +from pathlib import Path + +from CIME import locked_files +from CIME.utils import CIMEError +from CIME.XML.env_case import EnvCase + + +class TestLockedFiles(unittest.TestCase): + def test_check_lockedfile(self): + with tempfile.TemporaryDirectory() as tempdir: + src_case = EnvCase(tempdir) + + src_case.set_value("USER", "root") + + breakpoint() + src_case.write(force_write=True) + + def test_is_locked(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, locked_files.LOCKED_DIR, "env_case.xml") + + src_path.parent.mkdir(parents=True) + + src_path.touch() + + assert locked_files.is_locked("env_case.xml", tempdir) + + src_path.unlink() + + assert not locked_files.is_locked("env_case.xml", tempdir) + + def test_unlock_file_error_path(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, locked_files.LOCKED_DIR, "env_case.xml") + + src_path.parent.mkdir(parents=True) + + src_path.touch() + + with self.assertRaises(CIMEError): + locked_files.unlock_file("/env_case.xml", tempdir) + + def test_unlock_file(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, locked_files.LOCKED_DIR, "env_case.xml") + + src_path.parent.mkdir(parents=True) + + src_path.touch() + + locked_files.unlock_file("env_case.xml", tempdir) + + assert not src_path.exists() + + def test_lock_file_newname(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, "env_case.xml") + + src_path.touch() + + locked_files.lock_file("env_case.xml", tempdir, newname="env_case-old.xml") + + dst_path = Path(tempdir, locked_files.LOCKED_DIR, "env_case-old.xml") + + assert dst_path.exists() + + def test_lock_file_error_path(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, "env_case.xml") + + src_path.touch() + + with self.assertRaises(CIMEError): + locked_files.lock_file("/env_case.xml", tempdir) + + def test_lock_file(self): + with tempfile.TemporaryDirectory() as tempdir: + src_path = Path(tempdir, "env_case.xml") + + src_path.touch() + + locked_files.lock_file("env_case.xml", tempdir) + + dst_path = Path(tempdir, locked_files.LOCKED_DIR, "env_case.xml") + + assert dst_path.exists() From afc8d51be66f45a5d4863e97fb98af1cc0dff0d0 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Thu, 11 Apr 2024 18:28:59 -0700 Subject: [PATCH 2/9] Removes set_trace call and adds debug-statements check --- CIME/tests/test_unit_locked_files.py | 1 - 1 file changed, 1 deletion(-) diff --git a/CIME/tests/test_unit_locked_files.py b/CIME/tests/test_unit_locked_files.py index 2a16ed0fc7d..2cf9689ec28 100644 --- a/CIME/tests/test_unit_locked_files.py +++ b/CIME/tests/test_unit_locked_files.py @@ -15,7 +15,6 @@ def test_check_lockedfile(self): src_case.set_value("USER", "root") - breakpoint() src_case.write(force_write=True) def test_is_locked(self): From 40f93f3dc6f7f24da8aac8a3cc8bd2e94168814f Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Thu, 11 Apr 2024 18:29:23 -0700 Subject: [PATCH 3/9] Removes set_trace call and adds debug-statements check --- .pre-commit-config.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 415d0b468d2..73485746632 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,6 +10,8 @@ repos: exclude: doc/ - id: trailing-whitespace exclude: doc/ + - id: debug-statements + exclude: doc/|CIME/utils.py - repo: https://github.com/psf/black rev: 22.3.0 hooks: From 5b15805a40242db7bfbf3d42108047e41369a9ba Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 12 Apr 2024 10:57:43 -0700 Subject: [PATCH 4/9] Adds testing for locked_files module --- CIME/locked_files.py | 31 ++-- CIME/tests/test_unit_locked_files.py | 202 ++++++++++++++++++++++++++- 2 files changed, 218 insertions(+), 15 deletions(-) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index 7579f9ca1d9..5e6e79c4089 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -77,14 +77,12 @@ def check_lockedfiles(case, skip=None): if filename.count(".") > 1 or any([filename.startswith(x) for x in skip]): continue - env_name, diff = diff_lockedfile(case, caseroot, filename) + check_lockedfile(case, filename, caseroot=caseroot) - if diff: - check_diff(case, filename, env_name, diff) - -def check_lockedfile(case, filebase): - caseroot = case.get_value("CASEROOT") +def check_lockedfile(case, filebase, caseroot=None): + if caseroot is None: + caseroot = case.get_value("CASEROOT") env_name, diff = diff_lockedfile(case, caseroot, filebase) @@ -102,6 +100,17 @@ def diff_lockedfile(case, caseroot, filename): if not locked_file.is_file(): return env_name, {} + try: + l_env, r_env = _get_case_env(case, caseroot, locked_file, env_name) + except NameError as e: + logger.warning(e) + + return env_name, {} + + return env_name, l_env.compare_xml(r_env) + + +def _get_case_env(case, caseroot, locked_file, env_name): if env_name == "env_build": l_env = case.get_env("build") r_env = EnvBuild(caseroot, str(locked_file), read_only=True) @@ -120,13 +129,13 @@ def diff_lockedfile(case, caseroot, filename): l_env = case.get_env("batch") r_env = EnvBatch(caseroot, str(locked_file), read_only=True) else: - logger.warning( - "Locked XML file {!r} is not currently being handled".format(filename) + raise NameError( + "Locked XML file {!r} is not currently being handled".format( + locked_file.name + ) ) - return env_name, {} - - return env_name, l_env.compare_xml(r_env) + return l_env, r_env def check_diff(case, filename, env_name, diff): diff --git a/CIME/tests/test_unit_locked_files.py b/CIME/tests/test_unit_locked_files.py index 2cf9689ec28..446ca515d65 100644 --- a/CIME/tests/test_unit_locked_files.py +++ b/CIME/tests/test_unit_locked_files.py @@ -5,17 +5,211 @@ from CIME import locked_files from CIME.utils import CIMEError -from CIME.XML.env_case import EnvCase +from CIME.XML.entry_id import EntryID +from CIME.XML.env_batch import EnvBatch +from CIME.XML.files import Files + + +def create_batch_system(env_batch, batch_submit_value=None): + batch_system = env_batch.make_child( + name="batch_system", attributes={"type": "slurm"} + ) + env_batch.make_child(name="batch_query", attributes={"args": ""}, root=batch_system) + batch_submit = env_batch.make_child( + name="batch_submit", root=batch_system, text=batch_submit_value + ) + env_batch.make_child(name="batch_cancel", root=batch_system) + env_batch.make_child(name="batch_redirect", root=batch_system) + env_batch.make_child(name="batch_directive", root=batch_system) + directives = env_batch.make_child(name="directives", root=batch_system) + env_batch.make_child(name="directive", root=directives) + + return batch_system + + +def create_fake_env(tempdir): + locked_files_dir = Path(tempdir, locked_files.LOCKED_DIR) + + locked_files_dir.mkdir(parents=True) + + locked_file_path = locked_files_dir / "env_batch.xml" + + env_batch = EnvBatch(tempdir) + + env_batch.write(force_write=True) + + batch_system = create_batch_system(env_batch, "sbatch") + + env_batch.write(str(locked_file_path), force_write=True) + + env_batch.remove_child(batch_system) + + batch_system = create_batch_system(env_batch) + + env_batch.write(force_write=True) + + return env_batch class TestLockedFiles(unittest.TestCase): + def test_check_diff_rebuild_trigger(self): + case = mock.MagicMock() + + case.get_values.side_effect = (("CPL", "ATM"), (), ("NTASKS",)) + + diff = { + "NTASKS": ("32", "16"), + } + + with self.assertRaisesRegex( + CIMEError, "ERROR: A rebuild is required, please run:.*" + ): + locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) + + def test_check_diff_other_env(self): + case = mock.MagicMock() + + diff = { + "some_key": ("value1", "value2"), + } + + with self.assertRaises(CIMEError): + locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) + + def test_check_diff_env_build_no_diff(self): + case = mock.MagicMock() + + diff = {} + + locked_files.check_diff(case, "env_build.xml", "env_build", diff) + + case.set_value.assert_not_called() + + def test_check_diff_env_build_pio_version(self): + case = mock.MagicMock() + + diff = { + "some_key": ("value1", "value2"), + "PIO_VERSION": ("1", "2"), + } + + locked_files.check_diff(case, "env_build.xml", "env_build", diff) + + case.set_value.assert_any_call("BUILD_COMPLETE", False) + case.set_value.assert_any_call("BUILD_STATUS", 2) + + def test_check_diff_env_build(self): + case = mock.MagicMock() + + diff = { + "some_key": ("value1", "value2"), + } + + locked_files.check_diff(case, "env_build.xml", "env_build", diff) + + case.set_value.assert_any_call("BUILD_COMPLETE", False) + case.set_value.assert_any_call("BUILD_STATUS", 1) + + def test_check_diff_env_batch(self): + case = mock.MagicMock() + + diff = { + "some_key": ("value1", "value2"), + } + + with self.assertRaises(CIMEError): + locked_files.check_diff(case, "env_batch.xml", "env_batch", diff) + + def test_check_diff_env_case(self): + case = mock.MagicMock() + + diff = { + "some_key": ("value1", "value2"), + } + + with self.assertRaises(CIMEError): + locked_files.check_diff(case, "env_case.xml", "env_case", diff) + + def test_diff_lockedfile_detect_difference(self): + case = mock.MagicMock() + + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + env_batch = create_fake_env(tempdir) + + case.get_env.return_value = env_batch + + _, diff = locked_files.diff_lockedfile(case, tempdir, "env_batch.xml") + + assert diff + assert diff["batch_submit"] == [None, "sbatch"] + + def test_diff_lockedfile_not_supported(self): + case = mock.MagicMock() + + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + locked_file_path = Path(tempdir, locked_files.LOCKED_DIR, "env_new.xml") + + locked_file_path.parent.mkdir(parents=True) + + locked_file_path.touch() + + _, diff = locked_files.diff_lockedfile(case, tempdir, "env_new.xml") + + assert not diff + + def test_diff_lockedfile_does_not_exist(self): + case = mock.MagicMock() + + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + locked_files.diff_lockedfile(case, tempdir, "env_batch.xml") + + def test_diff_lockedfile(self): + case = mock.MagicMock() + + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + create_fake_env(tempdir) + + locked_files.diff_lockedfile(case, tempdir, "env_batch.xml") + def test_check_lockedfile(self): + case = mock.MagicMock() + with tempfile.TemporaryDirectory() as tempdir: - src_case = EnvCase(tempdir) + case.get_value.side_effect = (tempdir,) + + create_fake_env(tempdir) + + with self.assertRaises(CIMEError): + locked_files.check_lockedfile(case, "env_batch.xml") - src_case.set_value("USER", "root") + def test_check_lockedfiles_skip(self): + case = mock.MagicMock() - src_case.write(force_write=True) + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + create_fake_env(tempdir) + + locked_files.check_lockedfiles(case, skip="env_batch.xml") + + def test_check_lockedfiles(self): + case = mock.MagicMock() + + with tempfile.TemporaryDirectory() as tempdir: + case.get_value.side_effect = (tempdir,) + + create_fake_env(tempdir) + + with self.assertRaises(CIMEError): + locked_files.check_lockedfiles(case) def test_is_locked(self): with tempfile.TemporaryDirectory() as tempdir: From 9a80a737c85cb66bd2496b25e29fcb469b105211 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 19 Apr 2024 18:39:18 -0700 Subject: [PATCH 5/9] Adds documentation --- doc/source/users_guide/components.rst | 62 +++++++++++++++++++++++++++ doc/source/users_guide/index.rst | 1 + 2 files changed, 63 insertions(+) create mode 100644 doc/source/users_guide/components.rst diff --git a/doc/source/users_guide/components.rst b/doc/source/users_guide/components.rst new file mode 100644 index 00000000000..3b48da0c4cc --- /dev/null +++ b/doc/source/users_guide/components.rst @@ -0,0 +1,62 @@ +.. _components: + +========== +Components +========== + +A single component in the smallest unit within a model. Multiple components make up a component set. + +Configuration +-------------- + +The configuration for a component can be found under `cime_config` in the component directory. + +Example contents of a components `config_component.xml`. + +:: + + + + + + + + Stub atm component + + + + char + satm + satm + case_comp + env_case.xml + Name of atmosphere component + + + + + ========================================= + SATM naming conventions in compset name + ========================================= + + + + +Triggering a rebuild +-------------------- + +It's the responsibility of a component to define which settings will require a component to be rebuilt. + +These triggers can be defined as follows. + +:: + + + char + NTASKS,NTHREADS,NINST + rebuild_triggers + env_build.xml + Settings that will trigger a rebuild + + +If a user was to change `NTASKS`, `NTHREADS`, or `NINST` in a case using the component, then a rebuild would be required before the case could be submitted again. diff --git a/doc/source/users_guide/index.rst b/doc/source/users_guide/index.rst index 9df9b084490..1e1e5372af0 100644 --- a/doc/source/users_guide/index.rst +++ b/doc/source/users_guide/index.rst @@ -35,6 +35,7 @@ Case Control System Part 2: Configuration, Porting, Testing and Use Cases :numbered: cime-internals.rst + components.rst compsets.rst grids.rst machine.rst From 6de5bd22cbcdc635452126637621a6f2e072e9aa Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 7 May 2024 08:46:35 -0700 Subject: [PATCH 6/9] Adds missing exceptions for changes to env_build and env_mach_pes --- CIME/locked_files.py | 7 +++++++ CIME/tests/test_unit_locked_files.py | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index 5e6e79c4089..f61990a832a 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -173,11 +173,18 @@ def check_diff(case, filename, env_name, diff): ) case.set_value("BUILD_STATUS", build_status) + + expect( + False, + "For your changes to take effect, run:\n./case.build --clean-all\n./case.build", + ) elif env_name == "env_batch": expect( False, "Batch configuration has changed, please run `./case.setup --reset`", ) + elif env_name == "env_mach_pes": + expect(False, "For your changes to take effect, run:\n./case.setup --reset") else: toggle_build_status = False diff --git a/CIME/tests/test_unit_locked_files.py b/CIME/tests/test_unit_locked_files.py index 446ca515d65..3ada1964bd3 100644 --- a/CIME/tests/test_unit_locked_files.py +++ b/CIME/tests/test_unit_locked_files.py @@ -62,7 +62,7 @@ def test_check_diff_rebuild_trigger(self): } with self.assertRaisesRegex( - CIMEError, "ERROR: A rebuild is required, please run:.*" + CIMEError, "ERROR: For your changes to take effect, run:.*" ): locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) @@ -93,7 +93,13 @@ def test_check_diff_env_build_pio_version(self): "PIO_VERSION": ("1", "2"), } - locked_files.check_diff(case, "env_build.xml", "env_build", diff) + expected_msg = """ERROR: For your changes to take effect, run: +./case.build --clean-all +./case.build + """ + + with self.assertRaises(CIMEError, msg=expected_msg): + locked_files.check_diff(case, "env_build.xml", "env_build", diff) case.set_value.assert_any_call("BUILD_COMPLETE", False) case.set_value.assert_any_call("BUILD_STATUS", 2) @@ -105,7 +111,13 @@ def test_check_diff_env_build(self): "some_key": ("value1", "value2"), } - locked_files.check_diff(case, "env_build.xml", "env_build", diff) + expected_msg = """ERROR: For your changes to take effect, run: +./case.build --clean-all +./case.build + """ + + with self.assertRaises(CIMEError, msg=expected_msg): + locked_files.check_diff(case, "env_build.xml", "env_build", diff) case.set_value.assert_any_call("BUILD_COMPLETE", False) case.set_value.assert_any_call("BUILD_STATUS", 1) From 091b1dd988d9f3747c8e1867d3ff598e332ca583 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 7 May 2024 08:52:04 -0700 Subject: [PATCH 7/9] Fixes checking empty diff --- CIME/locked_files.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index f61990a832a..44336f014bd 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -144,6 +144,10 @@ def check_diff(case, filename, env_name, diff): # Remove BUILD_COMPLETE, invalid entry in diff diff.pop("BUILD_COMPLETE", None) + # Nothing to process + if not diff: + return + # List differences for key, value in diff.items(): logger.warning( From 3b771a9c17651e13d0579cee0f7c1928dab9290b Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 7 May 2024 08:56:12 -0700 Subject: [PATCH 8/9] Removes nested if statement --- CIME/locked_files.py | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index 44336f014bd..c83b469540c 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -159,29 +159,24 @@ def check_diff(case, filename, env_name, diff): False, f"Cannot change `env_case.xml`, please restore origin {filename!r}", ) - elif env_name == "env_build": - toggle_build_status = True if diff else False + elif env_name == "env_build" and diff: + case.set_value("BUILD_COMPLETE", False) - if toggle_build_status: - logger.warning("Setting 'BUILD_COMPLETE' to False") - - case.set_value("BUILD_COMPLETE", False) + build_status = 1 - build_status = 1 + if "PIO_VERSION" in diff: + build_status = 2 - if "PIO_VERSION" in diff: - build_status = 2 - - logging.critical( - "Changing 'PIO_VERSION' requires running `./case.build --clean-all` to rebuild" - ) + logging.critical( + "Changing 'PIO_VERSION' requires running `./case.build --clean-all` to rebuild" + ) - case.set_value("BUILD_STATUS", build_status) + case.set_value("BUILD_STATUS", build_status) - expect( - False, - "For your changes to take effect, run:\n./case.build --clean-all\n./case.build", - ) + expect( + False, + "For your changes to take effect, run:\n./case.build --clean-all\n./case.build", + ) elif env_name == "env_batch": expect( False, From d6491ea8e0cacec2e73e38f5b1afa92ea1417958 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 7 May 2024 09:57:38 -0700 Subject: [PATCH 9/9] Consolidates error messaging --- CIME/locked_files.py | 59 +++++++++++------------ CIME/tests/test_unit_locked_files.py | 71 +++++++++++++++++++++------- 2 files changed, 85 insertions(+), 45 deletions(-) diff --git a/CIME/locked_files.py b/CIME/locked_files.py index c83b469540c..178a8af6942 100644 --- a/CIME/locked_files.py +++ b/CIME/locked_files.py @@ -154,14 +154,18 @@ def check_diff(case, filename, env_name, diff): "\t{!r} has changed from {!r} to {!r}".format(key, value[1], value[0]) ) + reset = False + rebuild = False + message = "" + clean_targets = "" + rebuild_components = [] + if env_name == "env_case": expect( False, f"Cannot change `env_case.xml`, please restore origin {filename!r}", ) elif env_name == "env_build" and diff: - case.set_value("BUILD_COMPLETE", False) - build_status = 1 if "PIO_VERSION" in diff: @@ -173,37 +177,34 @@ def check_diff(case, filename, env_name, diff): case.set_value("BUILD_STATUS", build_status) - expect( - False, - "For your changes to take effect, run:\n./case.build --clean-all\n./case.build", - ) - elif env_name == "env_batch": - expect( - False, - "Batch configuration has changed, please run `./case.setup --reset`", - ) - elif env_name == "env_mach_pes": - expect(False, "For your changes to take effect, run:\n./case.setup --reset") - else: - toggle_build_status = False + rebuild = True - rebuild_components = [] + clean_targets = "--clean-all" + elif env_name in ("env_batch", "env_mach_pes"): + reset = True - for component in case.get_values("COMP_CLASSES"): - triggers = case.get_values(f"REBUILD_TRIGGER_{component}") + for component in case.get_values("COMP_CLASSES"): + triggers = case.get_values(f"REBUILD_TRIGGER_{component}") - if any([y.startswith(x) for x in triggers for y in diff.keys()]): - toggle_build_status = True + if any([y.startswith(x) for x in triggers for y in diff.keys()]): + rebuild = True - rebuild_components.append(component) + rebuild_components.append(component) - if toggle_build_status: - case.set_value("BUILD_COMPLETE", False) + if reset: + message = "For your changes to take effect, run:\n./case.setup --reset\n" - clean_targets = " ".join([x.lower() for x in rebuild_components]) - clean_cmd = f"./case.build --clean {clean_targets}" + if rebuild: + case.set_value("BUILD_COMPLETE", False) - expect( - False, - f"A rebuild is required, please run: \n./case.setup --reset\n{clean_cmd}\n./case.build", - ) + if rebuild_components and clean_targets != "--clean-all": + clean_targets = " ".join([x.lower() for x in rebuild_components]) + + clean_targets = f"--clean {clean_targets}" + + if not reset: + message = "For your changes to take effect, run:\n" + + message = f"{message}./case.build {clean_targets}\n./case.build" + + expect(False, message) diff --git a/CIME/tests/test_unit_locked_files.py b/CIME/tests/test_unit_locked_files.py index 3ada1964bd3..05b2e9952dc 100644 --- a/CIME/tests/test_unit_locked_files.py +++ b/CIME/tests/test_unit_locked_files.py @@ -52,28 +52,62 @@ def create_fake_env(tempdir): class TestLockedFiles(unittest.TestCase): - def test_check_diff_rebuild_trigger(self): + def test_check_diff_reset_and_rebuild(self): case = mock.MagicMock() - case.get_values.side_effect = (("CPL", "ATM"), (), ("NTASKS",)) + # reset triggered by env_mach_pes + # rebuild triggered by REBUILD_TRIGGER_ATM and REBUILD_TRIGGER_LND + # COMP_CLASSES, REBUILD_TRIGGER_CPL, REBUILD_TRIGGER_ATM, REBUILD_TRIGGER_LND + case.get_values.side_effect = ( + ("CPL", "ATM", "LND"), + (), + ("NTASKS",), + ("NTASKS",), + ) diff = { "NTASKS": ("32", "16"), } - with self.assertRaisesRegex( - CIMEError, "ERROR: For your changes to take effect, run:.*" - ): + expected_msg = """ERROR: For your changes to take effect, run: +./case.setup --reset +./case.build --clean atm lnd +./case.build""" + + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) - def test_check_diff_other_env(self): + def test_check_diff_reset_and_rebuild_single(self): case = mock.MagicMock() + # reset triggered by env_mach_pes + # rebuild triggered only by REBUILD_TRIGGER_ATM + # COMP_CLASSES, REBUILD_TRIGGER_CPL, REBUILD_TRIGGER_ATM, REBUILD_TRIGGER_LND + case.get_values.side_effect = (("CPL", "ATM", "LND"), (), ("NTASKS",), ()) + diff = { - "some_key": ("value1", "value2"), + "NTASKS": ("32", "16"), + } + + expected_msg = """ERROR: For your changes to take effect, run: +./case.setup --reset +./case.build --clean atm +./case.build""" + + with self.assertRaisesRegex(CIMEError, expected_msg): + locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) + + def test_check_diff_env_mach_pes(self): + case = mock.MagicMock() + + diff = { + "NTASKS": ("32", "16"), } - with self.assertRaises(CIMEError): + expected_msg = """ERROR: For your changes to take effect, run: +./case.setup --reset""" + + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_mach_pes.xml", "env_mach_pes", diff) def test_check_diff_env_build_no_diff(self): @@ -95,10 +129,9 @@ def test_check_diff_env_build_pio_version(self): expected_msg = """ERROR: For your changes to take effect, run: ./case.build --clean-all -./case.build - """ +./case.build""" - with self.assertRaises(CIMEError, msg=expected_msg): + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_build.xml", "env_build", diff) case.set_value.assert_any_call("BUILD_COMPLETE", False) @@ -113,10 +146,9 @@ def test_check_diff_env_build(self): expected_msg = """ERROR: For your changes to take effect, run: ./case.build --clean-all -./case.build - """ +./case.build""" - with self.assertRaises(CIMEError, msg=expected_msg): + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_build.xml", "env_build", diff) case.set_value.assert_any_call("BUILD_COMPLETE", False) @@ -129,7 +161,10 @@ def test_check_diff_env_batch(self): "some_key": ("value1", "value2"), } - with self.assertRaises(CIMEError): + expected_msg = """ERROR: For your changes to take effect, run: +./case.setup --reset""" + + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_batch.xml", "env_batch", diff) def test_check_diff_env_case(self): @@ -139,7 +174,11 @@ def test_check_diff_env_case(self): "some_key": ("value1", "value2"), } - with self.assertRaises(CIMEError): + expected_msg = ( + "ERROR: Cannot change `env_case.xml`, please restore origin 'env_case.xml'" + ) + + with self.assertRaisesRegex(CIMEError, expected_msg): locked_files.check_diff(case, "env_case.xml", "env_case", diff) def test_diff_lockedfile_detect_difference(self):