Summary
tests/integration/check.test.ts > checkNoNewCycles > excludeSpeculative has no effect on cycles formed entirely of static import edges is non-deterministically flaky — it fails intermittently with no code changes, comparing the same cycle's node order:
AssertionError: expected [ { …(2) } ] to deeply equal [ { nodes: [ …(2) ], …(1) } ]
- Expected
+ Received
[
{
"nodes": [
- "src/utils.js",
"src/math.js",
+ "src/utils.js",
],
"speculative": false,
},
]
Reproduced locally by running the single test in a loop with zero code changes between runs — passed 2/4, failed 2/4:
$ for i in 1 2 3 4; do npx vitest run tests/integration/check.test.ts -t "excludeSpeculative has no effect on cycles formed entirely of static import edges"; done
# pass, pass, fail, fail
Likely cause
The test asserts withoutSpeculative.cycles deep-equals withSpeculative.cycles for the exact same underlying cycle (src/math.js <-> src/utils.js). The cycle's nodes array order appears to depend on Map/Set iteration order inside the cycle-detection algorithm (Tarjan SCC or similar), which can differ between the two checkNoNewCycles invocations depending on incidental insertion-order differences introduced by the speculative parameter — the set of nodes in the cycle is correct, but the order isn't guaranteed to be stable, and the test uses toEqual (order-sensitive) rather than an order-independent comparison.
Suggested fix
Either:
- Make the test assertion order-independent (e.g. sort each cycle's
nodes array before comparing), or
- Make the underlying cycle-detection code deterministic in node ordering (e.g. sort nodes within a detected cycle before returning it), if a stable order is otherwise relied upon elsewhere.
Found while running the full test suite to validate an unrelated JS/TS extractor fix (#1929) — filed separately per this repo's scope-discipline convention since it's unrelated to that change.
Summary
tests/integration/check.test.ts > checkNoNewCycles > excludeSpeculative has no effect on cycles formed entirely of static import edgesis non-deterministically flaky — it fails intermittently with no code changes, comparing the same cycle's node order:Reproduced locally by running the single test in a loop with zero code changes between runs — passed 2/4, failed 2/4:
Likely cause
The test asserts
withoutSpeculative.cyclesdeep-equalswithSpeculative.cyclesfor the exact same underlying cycle (src/math.js<->src/utils.js). The cycle'snodesarray order appears to depend onMap/Setiteration order inside the cycle-detection algorithm (Tarjan SCC or similar), which can differ between the twocheckNoNewCyclesinvocations depending on incidental insertion-order differences introduced by thespeculativeparameter — the set of nodes in the cycle is correct, but the order isn't guaranteed to be stable, and the test usestoEqual(order-sensitive) rather than an order-independent comparison.Suggested fix
Either:
nodesarray before comparing), orFound while running the full test suite to validate an unrelated JS/TS extractor fix (#1929) — filed separately per this repo's scope-discipline convention since it's unrelated to that change.