Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail "a => b & !b" in validation. #4335

Merged
merged 2 commits into from
Jul 30, 2021
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
13 changes: 13 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ creating a new release entry be sure to copy & paste the span tag with the
`actions:bind` attribute, which is used by a regex to find the text to be
updated. Only the first match gets replaced, so it's fine to leave the old
ones in. -->
-------------------------------------------------------------------------------
## __cylc-8.0b3 (<span actions:bind='release-date'>???</span>)__

Fourth beta release of Cylc 8.

(See note on cylc-8 backward-incompatible changes, above)

### Enhancements

[#4335](https://github.com/cylc/cylc-flow/pull/4335) - have validation catch
erroneous use of both `expr => bar` and `expr => !bar` in the same graph.


-------------------------------------------------------------------------------
## __cylc-8.0b2 (<span actions:bind='release-date'>Released 2021-07-28</span>)__

Expand Down
13 changes: 13 additions & 0 deletions cylc/flow/graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"""Module for parsing cylc graph strings."""

import re
import contextlib

from cylc.flow.exceptions import GraphParseError
from cylc.flow.param_expand import GraphExpander
Expand Down Expand Up @@ -506,6 +507,18 @@ def _add_trigger(self, orig_expr, rights, expr, info):
else:
members = [right]
for member in members:
with contextlib.suppress(KeyError):
osuicide = self.triggers[member][expr][1]
# This trigger already exists, so we must have both
# "expr => member" and "expr => !member" in the graph,
# or simply a duplicate trigger not recognized earlier
# because of parameter offsets.
if suicide or osuicide:
oexp = re.sub(r'(&|\|)', r' \1 ', orig_expr)
oexp = re.sub(r':succeed', '', oexp)
raise GraphParseError(
f"{oexp} can't trigger both {member} and !{member}"
)
self.triggers.setdefault(member, {})
self.original.setdefault(member, {})
self.triggers[member][expr] = (trigs, suicide)
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/test_graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ def test_parse_graph_fails_with_spaces_in_task_name(self):
with self.assertRaises(GraphParseError):
self.parser.parse_graph("a b => c")

def test_parse_graph_fails_with_suicide_and_not_suicide(self):
"""Test graph parser fails with both "expr => !foo"
and "expr => !foo" in the same graph."""
with self.assertRaises(GraphParseError):
self.parser.parse_graph(
"""(a | b & c) => d
foo => bar
(a | b & c) => !d
""")

def test_parse_graph_fails_with_invalid_and_operator(self):
"""Test that the graph parse will fail when the and operator is not
correctly used."""
Expand Down