Просмотр исходного кода

test: pin the all-failed loop rule, and fix two cases that never tested it

body_failure_continue_on_error_true and _false read the loop item as
`loop.item.n` inside a code node. There is no bare `loop` global there - a body
node gets the item on its `input` - so the throw fired on every item and the
cases had been asserting "one of three items fails" while actually running "all
three do", since the day they were written.

They passed anyway, because the behaviour they were meant to pin was that an
all-failed loop reports success. A test whose setup is broken in the same
direction as the bug tests nothing. Both now use input.item.n, and the two new
cases pin each half of the rule: all-failed fails the run, some-failed does not.

executionTime is also stripped from the recorded output now. It wobbles between
0 and 1 ms run to run and appears nested inside collected loop results, so four
of thirteen cases differed on every diff - noise that a real change hides in.
With it gone, this change diffs as exactly one case.
fszontagh 1 месяц назад
Родитель
Сommit
42c32c8812
1 измененных файлов с 58 добавлено и 2 удалено
  1. 58 2
      scripts/characterise-loops.py

+ 58 - 2
scripts/characterise-loops.py

@@ -180,7 +180,7 @@ def _():
             # completed anyway"; do not read the case name as a promise that
             # exactly one item fails.
             {"id": "boom", "type": "code", "name": "boom", "position": {"x": 480, "y": 0},
-             "config": {"code": "if (loop.item.n === 'b') { throw new Error('planned'); } return { ok: loop.item.n }"}},
+             "config": {"code": "if (input.item.n === 'b') { throw new Error('planned'); } return { ok: input.item.n }"}},
             setf("after", "done", "yes", x=480),
         ],
         "connections": [
@@ -196,7 +196,47 @@ def _():
             seed([{"n": "a"}, {"n": "b"}, {"n": "c"}]),
             loop_node(cont=False),
             {"id": "boom", "type": "code", "name": "boom", "position": {"x": 480, "y": 0},
-             "config": {"code": "if (loop.item.n === 'b') { throw new Error('planned'); } return { ok: loop.item.n }"}},
+             "config": {"code": "if (input.item.n === 'b') { throw new Error('planned'); } return { ok: input.item.n }"}},
+            setf("after", "done", "yes", x=480),
+        ],
+        "connections": [
+            conn("seed", "loop"), conn("loop", "boom", "loop"), conn("loop", "after", "done"),
+        ],
+    }
+
+
+# continue_on_error is a per-item policy, not a per-run one. A loop that
+# tolerates failures should still fail the run when NOTHING succeeded - a dead
+# dependency made every item fail for hours while the run reported success.
+# A code node reaches the loop item through `input` - `input.item`, `input.index`,
+# `input.loop`. There is no bare `loop` global; using one throws "'loop' is not
+# defined" on EVERY item, which is how two cases here spent their life asserting
+# "one item fails" while actually testing "all of them do".
+@case("tolerated_all_items_fail_fails_the_run")
+def _():
+    return {
+        "nodes": [
+            seed([{"n": "a"}, {"n": "b"}, {"n": "c"}]),
+            loop_node(cont=True),
+            {"id": "boom", "type": "code", "name": "boom", "position": {"x": 480, "y": 0},
+             "config": {"code": "throw new Error('every item fails');"}},
+            setf("after", "done", "yes", x=480),
+        ],
+        "connections": [
+            conn("seed", "loop"), conn("loop", "boom", "loop"), conn("loop", "after", "done"),
+        ],
+    }
+
+
+# The other half of the same rule: tolerating SOME failures must be unchanged.
+@case("tolerated_some_items_fail_still_succeeds")
+def _():
+    return {
+        "nodes": [
+            seed([{"n": "a"}, {"n": "b"}, {"n": "c"}]),
+            loop_node(cont=True),
+            {"id": "boom", "type": "code", "name": "boom", "position": {"x": 480, "y": 0},
+             "config": {"code": "if (input.item.n === 'b') { throw new Error('planned'); } return { ok: input.item.n }"}},
             setf("after", "done", "yes", x=480),
         ],
         "connections": [
@@ -277,6 +317,21 @@ def run_case(name, spec):
             if detail.get("status") not in ("running", "pending"):
                 break
 
+        def drop_timings(value):
+            """executionTime is how long a node took, not what it did.
+
+            It wobbles between 0 and 1 ms run to run, and it appears nested
+            inside collected loop results too - so left in, four of thirteen
+            cases differ on every single diff and a real change hides among
+            them. Timing belongs in a benchmark, not a characterisation.
+            """
+            if isinstance(value, dict):
+                return {k: drop_timings(v) for k, v in value.items()
+                        if k != "executionTime"}
+            if isinstance(value, list):
+                return [drop_timings(v) for v in value]
+            return value
+
         records = []
         for rec in detail.get("nodeExecutions", []):
             out = rec.get("output")
@@ -284,6 +339,7 @@ def run_case(name, spec):
             # internal loop bookkeeping, which is noisy and not behaviour.
             if isinstance(out, dict):
                 out = {k: v for k, v in out.items() if not k.startswith("_")}
+            out = drop_timings(out)
             records.append({
                 "node": rec.get("nodeId"),
                 "status": rec.get("status"),