Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
  • Loading branch information
wxtim and MetRonnie committed Mar 26, 2024
1 parent 50163fc commit 5ad1611
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 25 deletions.
14 changes: 6 additions & 8 deletions cylc/flow/graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,8 @@ def parse_graph(self, graph_string: str) -> None:
pairs.add((chain[i], chain[i + 1]))

# Get a set of RH nodes which are not at the LH of another pair:
terminals = {
right for right in {right for _, right in pairs}
if right not in {left for left, _ in pairs}}
pairs_dict = dict(pairs)
terminals = set(pairs_dict.values()).difference(pairs_dict.keys())

for pair in pairs:
self._proc_dep_pair(pair, terminals)
Expand Down Expand Up @@ -514,7 +513,7 @@ def _proc_dep_pair(
'right' can be one or more node names joined by AND.
'right' can't be None or "".
terminals:
Lits of nodes which are _only_ on the RH end of chains.
Nodes which are _only_ on the RH end of chains.
"""
left, right = pair
# Raise error for right-hand-side OR operators.
Expand All @@ -534,16 +533,15 @@ def _proc_dep_pair(
if right.count("(") != right.count(")"):
raise GraphParseError(mismatch_msg.format(right))

# Ignore/Error cycle point offsets on the right side:
# (Note we can only ban this for nodes at the end of chains)
# Raise error for cycle point offsets at the end of chains
if '[' in right:
if left and right in terminals:
if left and (right in terminals):
# This right hand side is at the end of a chain:
raise GraphParseError(
'ERROR, illegal cycle point offset on the right:'
f' {left} => {right}')
else:
# This right hand side is a left elsewhere:
# This RHS is also a LHS in a chain:
return

# Split right side on AND.
Expand Down
29 changes: 12 additions & 17 deletions tests/unit/test_graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,27 +139,13 @@ def test_graph_syntax_errors_2(seq, graph, expected_err):
'illegal cycle point offset on the right',
id='no-cycle-point-RHS'
),
param(
"foo => bar[1649] => baz",
'!illegal cycle point offset on the right',
id='ignore-not-exclusively-RHS-cycle-point'
),
param(
"foo => bar[1649]\nbar[1649] => baz",
'!illegal cycle point offset on the right',
id='ignore-not-exclusively-RHS-cycle-point2'
),
]
)
def test_graph_syntax_errors(graph, expected_err):
"""Test various graph syntax errors."""
if expected_err[0] == '!':
# Assert method doesn't fail:
with pytest.raises(GraphParseError) as cm:
GraphParser().parse_graph(graph)
else:
with pytest.raises(GraphParseError) as cm:
GraphParser().parse_graph(graph)
assert expected_err in str(cm.value)
assert expected_err in str(cm.value)


def test_parse_graph_simple():
Expand Down Expand Up @@ -397,7 +383,16 @@ def test_line_continuation():
foo => bar
bar:succeed => baz
"""
]
],
[
"""
foo => bar[1649] => baz
""",
"""
foo => bar[1649]
bar[1649] => baz
"""
],
]
)
def test_trigger_equivalence(graph1, graph2):
Expand Down

0 comments on commit 5ad1611

Please sign in to comment.