Skip to content

Commit

Permalink
outputs: better validation errors for alt qualifiers in completion exprs
Browse files Browse the repository at this point in the history
  • Loading branch information
oliver-sanders committed Apr 23, 2024
1 parent f7d65ff commit a88733a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
21 changes: 17 additions & 4 deletions cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
is_relative_to,
)
from cylc.flow.print_tree import print_tree
from cylc.flow.task_qualifiers import ALT_QUALIFIERS
from cylc.flow.simulation import configure_sim_modes
from cylc.flow.subprocctx import SubFuncContext
from cylc.flow.task_events_mgr import (
Expand All @@ -96,6 +97,7 @@
get_completion_expression,
get_optional_outputs,
get_trigger_completion_variable_maps,
trigger_to_completion_variable,
)
from cylc.flow.task_trigger import TaskTrigger, Dependency
from cylc.flow.taskdef import TaskDef
Expand Down Expand Up @@ -1096,14 +1098,14 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:
return

Check warning on line 1098 in cylc/flow/config.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/config.py#L1098

Added line #L1098 was not covered by tests

(
trigger_to_completion_variable,
completion_variable_to_trigger,
_trigger_to_completion_variable,
_completion_variable_to_trigger,
) = get_trigger_completion_variable_maps(outputs.keys())

# get the optional/required outputs defined in the graph
graph_optionals = {
# completion_variable: is_optional
trigger_to_completion_variable[trigger]: (
_trigger_to_completion_variable[trigger]: (
None if is_required is None else not is_required
)
for trigger, (_, is_required)
Expand Down Expand Up @@ -1136,6 +1138,17 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:
' expressions, use "succeeded or failed".'
)

for alt_qualifier, qualifier in ALT_QUALIFIERS.items():
_alt_compvar = trigger_to_completion_variable(alt_qualifier)
_compvar = trigger_to_completion_variable(qualifier)
if re.search(rf'\b{_alt_compvar}\b', error):
raise WorkflowConfigError(
f'Error in [runtime][{task_name}]completion:'
f'\n {expr}'
f'\nUse "{_compvar}" not "{_alt_compvar}" '
'in completion expressions.'
)

raise WorkflowConfigError(
# NOTE: str(exc) == "name 'x' is not defined" tested in
# tests/integration/test_optional_outputs.py
Expand Down Expand Up @@ -1179,7 +1192,7 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:

# [1] applies only to "submit-failed" and "expired"

trigger = completion_variable_to_trigger[compvar]
trigger = _completion_variable_to_trigger[compvar]

if graph_opt is True and expr_opt is False:
raise WorkflowConfigError(
Expand Down
12 changes: 12 additions & 0 deletions tests/integration/validate/test_outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,18 @@ def test_messages(messages, valid, flow, validate):
'but required in the completion expression',
id='failed-implicitly-optional-in-graph-required-in-completion',
),
pytest.param(
'foo',
'(succeed and x) or failed',
'Use "succeeded" not "succeed" in completion expressions',
id='alt-compvar1',
),
pytest.param(
'foo? & foo:submitted?',
'submit_fail or succeeded',
'Use "submit_failed" not "submit_fail" in completion expressions',
id='alt-compvar2',
),
]
)
def test_completion_expression_invalid(
Expand Down

0 comments on commit a88733a

Please sign in to comment.