From c00a82d00980eee37a6c816b0409b9ea95bba677 Mon Sep 17 00:00:00 2001 From: Christina Holt <56881914+christinaholtNOAA@users.noreply.github.com> Date: Mon, 21 Oct 2024 14:51:51 -0600 Subject: [PATCH] Fix for Rocoto streq dependencies. (#612) Fixes a bug associated with using the Rocoto streq dependency. To prevent similar failures for other dependencies, validation checks are added to each config object used to test the rocoto mechanics. --- src/uwtools/rocoto.py | 4 +- src/uwtools/tests/test_rocoto.py | 118 +++++++++++++++++++++++-------- 2 files changed, 93 insertions(+), 29 deletions(-) diff --git a/src/uwtools/rocoto.py b/src/uwtools/rocoto.py index 73d8ee39f..84001ccf0 100644 --- a/src/uwtools/rocoto.py +++ b/src/uwtools/rocoto.py @@ -261,7 +261,9 @@ def _add_task_dependency_strequality(self, e: _Element, config: dict, tag: str) :param config: Configuration data for the tag. :param tag: Name of new element to add. """ - self._set_attrs(SubElement(e, tag), config) + e = SubElement(e, tag) + for k, v in config.items(): + self._add_compound_time_string(e, v, k) def _add_task_dependency_taskdep(self, e: _Element, config: dict) -> None: """ diff --git a/src/uwtools/tests/test_rocoto.py b/src/uwtools/tests/test_rocoto.py index 9bd86eb96..94bf39327 100644 --- a/src/uwtools/tests/test_rocoto.py +++ b/src/uwtools/tests/test_rocoto.py @@ -12,7 +12,7 @@ from uwtools import rocoto from uwtools.config.formats.yaml import YAMLConfig from uwtools.exceptions import UWConfigError, UWError -from uwtools.tests.support import fixture_path +from uwtools.tests.support import fixture_path, schema_validator # Fixtures @@ -119,10 +119,12 @@ def test__add_compound_time_string_basic(self, config, instance, root): assert child.text == str(config) def test__add_compound_time_string_cyclestr(self, instance, root): - config = {"cyclestr": {"attrs": {"baz": "42"}, "value": "qux"}} + config = {"cyclestr": {"attrs": {"offset": "00:05:00"}, "value": "qux"}} + errors = schema_validator("rocoto", "$defs", "cycleString") + assert not errors(config) instance._add_compound_time_string(e=root, config=config, tag="foo") cyclestr = root[0][0] - assert cyclestr.get("baz") == "42" + assert cyclestr.get("offset") == "00:05:00" assert cyclestr.text == "qux" def test__add_compound_time_string_list(self, instance, root): @@ -133,6 +135,8 @@ def test__add_compound_time_string_list(self, instance, root): {"cyclestr": {"value": "%s", "attrs": {"offset": "00:06:00"}}}, ".log", ] + errors = schema_validator("rocoto", "$defs", "compoundTimeString") + assert not errors(config) xml = "{}".format( "".join( [ @@ -149,32 +153,51 @@ def test__add_compound_time_string_list(self, instance, root): def test__add_metatask(self, instance, root): config = { - "metatask_foo": "1", "attrs": {"mode": "parallel", "throttle": 42}, - "task_bar": "2", "var": {"baz": "3", "qux": "4"}, + "metatask_nest": { + "var": {"bar": "1 2 3"}, + "task_bar": { + "cores": 2, + "walltime": "00:10:00", + "command": "echo hello", + }, + }, } - taskname = "test-metatask" + errors = schema_validator("rocoto", "$defs") + metataskname = "metatask_test" + assert not errors({metataskname: config}) orig = instance._add_metatask with patch.multiple(instance, _add_metatask=D, _add_task=D) as mocks: - orig(e=root, config=config, name_attr=taskname) + orig(e=root, config=config, name_attr=metataskname) metatask = root[0] assert metatask.tag == "metatask" assert metatask.get("mode") == "parallel" - assert metatask.get("name") == taskname + assert metatask.get("name") == metataskname assert metatask.get("throttle") == "42" assert {e.get("name"): e.text for e in metatask.xpath("var")} == {"baz": "3", "qux": "4"} - mocks["_add_metatask"].assert_called_once_with(metatask, "1", "foo") - mocks["_add_task"].assert_called_once_with(metatask, "2", "bar") + mocks["_add_metatask"].assert_called_once_with( + metatask, + { + "var": {"bar": "1 2 3"}, + "task_bar": {"cores": 2, "walltime": "00:10:00", "command": "echo hello"}, + }, + "nest", + ) def test__add_task(self, instance, root): config = { "attrs": {"foo": "1", "bar": "2"}, "account": "baz", - "dependency": "qux", + "dependency": {"taskdep": {"attrs": {"task": "task_foo"}}}, "envars": {"A": "apple"}, + "walltime": "00:12:00", + "command": "echo hello", + "cores": 2, } - taskname = "test-task" + taskname = "task_test" + errors = schema_validator("rocoto", "$defs") + assert not errors({taskname: config}) with patch.multiple(instance, _add_task_dependency=D, _add_task_envar=D) as mocks: instance._add_task(e=root, config=config, name_attr=taskname) task = root[0] @@ -182,7 +205,9 @@ def test__add_task(self, instance, root): assert task.get("name") == taskname assert task.get("foo") == "1" assert task.get("bar") == "2" - mocks["_add_task_dependency"].assert_called_once_with(task, "qux") + mocks["_add_task_dependency"].assert_called_once_with( + task, {"taskdep": {"attrs": {"task": "task_foo"}}} + ) mocks["_add_task_envar"].assert_called_once_with(task, "A", "apple") @mark.parametrize("cores", [1, "1"]) @@ -193,6 +218,8 @@ def test__add_task_cores_int_or_str(self, cores, instance, root): def test__add_task_dependency_and(self, instance, root): config = {"and": {"or_get_obs": {"taskdep": {"attrs": {"task": "foo"}}}}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -208,6 +235,8 @@ def test__add_task_dependency_datadep(self, instance, root, value): age = "00:00:02:00" minsize = "1K" config = {"datadep": {"attrs": {"age": age, "minsize": minsize}, "value": value}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -229,6 +258,8 @@ def test__add_task_dependency_fail_bad_operand(self, instance, root): def test__add_task_dependency_metataskdep(self, instance, root): config = {"metataskdep": {"attrs": {"metatask": "foo"}}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -238,10 +269,12 @@ def test__add_task_dependency_metataskdep(self, instance, root): @mark.parametrize( "tag_config", - [("and", {"strneq": {"attrs": {"left": "&RUN_GSI;", "right": "YES"}}})], + [("and", {"strneq": {"left": "&RUN_GSI;", "right": "YES"}})], ) def test__add_task_dependency_operator(self, instance, root, tag_config): tag, config = tag_config + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency_child(e=root, config=config, tag=tag) for tag, _ in config.items(): assert tag == next(iter(config)) @@ -249,6 +282,8 @@ def test__add_task_dependency_operator(self, instance, root, tag_config): def test__add_task_dependency_operator_datadep_operand(self, instance, root): value = "/some/file" config = {"value": value} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors({"datadep": config}) instance._add_task_dependency_child(e=root, config=config, tag="datadep") e = root[0] assert e.tag == "datadep" @@ -257,6 +292,8 @@ def test__add_task_dependency_operator_datadep_operand(self, instance, root): def test__add_task_dependency_operator_task_operand(self, instance, root): taskname = "some-task" config = {"attrs": {"task": taskname}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors({"taskdep": config}) instance._add_task_dependency_child(e=root, config=config, tag="taskdep") e = root[0] assert e.tag == "taskdep" @@ -265,6 +302,8 @@ def test__add_task_dependency_operator_task_operand(self, instance, root): def test__add_task_dependency_operator_timedep_operand(self, instance, root): value = 20230103120000 config = value + errors = schema_validator("rocoto", "$defs", "compoundTimeString") + assert not errors(config) instance._add_task_dependency_child(e=root, config=config, tag="timedep") e = root[0] assert e.tag == "timedep" @@ -272,6 +311,8 @@ def test__add_task_dependency_operator_timedep_operand(self, instance, root): def test__add_task_dependency_sh(self, instance, root): config = {"sh_foo": {"attrs": {"runopt": "-c", "shell": "/bin/bash"}, "command": "ls"}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -283,31 +324,38 @@ def test__add_task_dependency_sh(self, instance, root): assert sh.text == "ls" def test__add_task_dependency_streq(self, instance, root): - config = {"streq": {"attrs": {"left": "&RUN_GSI;", "right": "YES"}}} + config = {"streq": {"left": "&RUN_GSI;", "right": "YES"}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" streq = dependency[0] assert streq.tag == "streq" - assert streq.get("left") == "&RUN_GSI;" + assert streq[0].text == "&RUN_GSI;" + assert streq[1].text == "YES" @mark.parametrize( "config", [ - ("streq", {"attrs": {"left": "&RUN_GSI;", "right": "YES"}}), - ("strneq", {"attrs": {"left": "&RUN_GSI;", "right": "YES"}}), + ("streq", {"left": "&RUN_GSI;", "right": "YES"}), + ("strneq", {"left": "&RUN_GSI;", "right": "YES"}), ], ) def test__add_task_dependency_strequality(self, config, instance, root): + errors = schema_validator("rocoto", "$defs", "dependency") tag, config = config + assert not errors({tag: config}) instance._add_task_dependency_strequality(e=root, config=config, tag=tag) element = root[0] assert tag == element.tag - for attr, val in config["attrs"].items(): - assert element.get(attr) == val + for idx, val in enumerate(config.values()): + assert element[idx].text == val def test__add_task_dependency_taskdep(self, instance, root): config = {"taskdep": {"attrs": {"task": "foo"}}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -317,6 +365,8 @@ def test__add_task_dependency_taskdep(self, instance, root): def test__add_task_dependency_taskvalid(self, instance, root): config = {"taskvalid": {"attrs": {"task": "foo"}}} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -334,6 +384,8 @@ def test__add_task_dependency_taskvalid(self, instance, root): ) def test__add_task_dependency_timedep(self, instance, root, value): config = {"timedep": value} + errors = schema_validator("rocoto", "$defs", "dependency") + assert not errors(config) instance._add_task_dependency(e=root, config=config) dependency = root[0] assert dependency.tag == "dependency" @@ -389,29 +441,39 @@ def test__add_task_envar_compound(self, instance, root): def test__add_workflow(self, instance): config = { "workflow": { - "attrs": {"foo": "1", "bar": "2"}, - "cycledef": "3", - "log": "4", - "tasks": "5", + "attrs": {"realtime": True, "scheduler": "slurm"}, + "cycledef": [], + "log": "1", + "tasks": { + "task_foo": { + "command": "echo hello", + "cores": 1, + "walltime": "00:01:00", + }, + }, } } + errors = schema_validator("rocoto") + assert not errors(config) with patch.multiple( instance, _add_workflow_cycledef=D, _add_workflow_log=D, _add_workflow_tasks=D ) as mocks: instance._add_workflow(config=config) workflow = instance._root assert workflow.tag == "workflow" - assert workflow.get("foo") == "1" - assert workflow.get("bar") == "2" - mocks["_add_workflow_cycledef"].assert_called_once_with(workflow, "3") + assert workflow.get("realtime") == "True" + assert workflow.get("scheduler") == "slurm" + mocks["_add_workflow_cycledef"].assert_called_once_with(workflow, []) mocks["_add_workflow_log"].assert_called_once_with(workflow, config["workflow"]) - mocks["_add_workflow_tasks"].assert_called_once_with(workflow, "5") + mocks["_add_workflow_tasks"].assert_called_once_with(workflow, config["workflow"]["tasks"]) def test__add_workflow_cycledef(self, instance, root): config: list[dict] = [ {"attrs": {"group": "g1"}, "spec": "t1"}, {"attrs": {"group": "g2"}, "spec": "t2"}, ] + errors = schema_validator("rocoto", "$defs") + assert not errors({"cycledef": config}) instance._add_workflow_cycledef(e=root, config=config) for i, item in enumerate(config): assert root[i].get("group") == item["attrs"]["group"]