Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes.d/6772.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Xtriggers with an OR operator (`|`) between them now correctly fail validation as Cylc does not support this.
12 changes: 11 additions & 1 deletion cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ def generate_edges(self, lexpr, orig_lexpr, left_nodes, right, seq,
# Right is a lone node.
self.edges[seq].add((right, None, suicide, conditional))

conditional_xtriggers = []
for left in left_nodes:
# if left is None:
# continue
Expand All @@ -1856,7 +1857,16 @@ def generate_edges(self, lexpr, orig_lexpr, left_nodes, right, seq,
LOG.error(f"{orig_lexpr} => {right}")
raise WorkflowConfigError(
f"self-edge detected: {left} => {right}")
self.edges[seq].add((left, right, suicide, conditional))
if left.startswith('@') and conditional:
conditional_xtriggers.append(f'{lexpr} => {right}')
else:
self.edges[seq].add((left, right, suicide, conditional))
if conditional_xtriggers:
raise WorkflowConfigError(
'Xtriggers cannot be used in conditional graph'
' expressions:\n' +
'\n * '.join(conditional_xtriggers)
)

def generate_taskdef(self, orig_expr, node):
"""Generate task definition for node."""
Expand Down
6 changes: 1 addition & 5 deletions cylc/flow/prerequisite.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,8 @@ def _eval_satisfied(self) -> bool:
# * the expression is constructed internally
# * https://github.com/cylc/cylc-flow/issues/4403
except (SyntaxError, ValueError) as exc:
err_msg = str(exc)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From 3.10 Python no longer emits the message being searched for here. ref

if str(exc).find("unexpected EOF") != -1:
err_msg += (
" (could be unmatched parentheses in the graph string?)")
raise TriggerExpressionError(
'"%s":\n%s' % (self.get_raw_conditional_expression(), err_msg)
f'"{self.get_raw_conditional_expression}":\n{str(exc)}'
) from None
return res

Expand Down
13 changes: 1 addition & 12 deletions cylc/flow/scripts/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,7 @@ async def run(
# force trigger evaluation now
try:
itask.state.prerequisites_eval_all()
except TriggerExpressionError as exc:
err = str(exc)
if '@' in err:
print(
f"ERROR, {name}: xtriggers can't be in conditional"
f" expressions: {err}",
file=sys.stderr,
)
else:
print(
'ERROR, %s: bad trigger: %s' % (name, err), file=sys.stderr
)
except TriggerExpressionError:
raise WorkflowConfigError("ERROR: bad trigger") from None
except Exception as exc:
print(str(exc), file=sys.stderr)
Expand Down
39 changes: 39 additions & 0 deletions tests/integration/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from cylc.flow.pathutil import get_workflow_run_pub_db_path

Fixture = Any
param = pytest.param


@pytest.mark.parametrize(
Expand Down Expand Up @@ -488,6 +489,44 @@ def test_xtrig_signature_validation(
validate(id_)


@pytest.mark.parametrize(
'left',
(
param('@xrandom | @echo', id='xtrig-or-xtrig'),
param('@xrandom | task', id='xtrig-or-task'),
param('task | @echo', id='task-or-xtrig'),
param('@xrandom | foo & bar', id='xtrig-or-complex'),
param('@xrandom & bar | foo', id='complex-or-xtrig'),
)
)
def test_xtrig_or_fails_validation(
flow: "Fixture",
validate: "Fixture",
left: str
):
"""Xtriggers cannot be chained with the 'or'

https://github.com/cylc/cylc-flow/issues/6771
https://github.com/cylc/cylc-flow/issues/2712
"""
id_ = flow(
{
"scheduling": {
"initial cycle point": "2024",
"xtriggers": {
"xrandom": "xrandom(100)",
"echo": "echo(succeed=True)"
},
"graph": {"R1": f"{left} => fin"},
}
}
)
expected_msg = (
"Xtriggers cannot be used in conditional graph expressions:\n")
with pytest.raises(WorkflowConfigError, match=expected_msg):
validate(id_)


def test_special_task_non_word_names(flow: Fixture, validate: Fixture):
"""Test validation of special tasks names with non-word characters"""
wid = flow({
Expand Down
17 changes: 17 additions & 0 deletions tests/unit/test_prerequisite.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from cylc.flow.cycling.integer import IntegerPoint
from cylc.flow.cycling.loader import ISO8601_CYCLING_TYPE, get_point
from cylc.flow.exceptions import TriggerExpressionError
from cylc.flow.id import Tokens, detokenise
from cylc.flow.prerequisite import Prerequisite, SatisfiedState
from cylc.flow.run_modes import RunMode
Expand Down Expand Up @@ -259,3 +260,19 @@ def test_satisfy_me__override_truthy(

prereq.satisfy_me([Tokens('//1/a:x')], forced=forced, mode=mode)
assert prereq[('1', 'a', 'x')] == existing


@pytest.mark.parametrize(
'expr, err', (
('int("df")', 'invalid literal'),
('7 +', 'invalid syntax'),
))
def test__eval_satisfied_raises(expr, err, monkeypatch):
monkeypatch.setattr(
'cylc.flow.prerequisite.Prerequisite.get_raw_conditional_expression',
lambda _: expr
)
prereq = Prerequisite(IntegerPoint('1'))
prereq.conditional_expression = expr
with pytest.raises(TriggerExpressionError, match=err):
prereq._eval_satisfied()
Loading