fix(playwright): bump per-test timeout 30s -> 60s for graph render#214
fix(playwright): bump per-test timeout 30s -> 60s for graph render#214
Conversation
Latest CI on main showed the diagram-viewer.spec.ts:graph tests still failing despite cc8403e's ?limit=2000 — the graph DID render (page snapshot shows "742 nodes, 1477 edges") but layout + SVG generation took ~25-30s synchronously in render_graph_view, leaving <5s for the .toBeVisible() assertion before the 30s test timeout fired. Bumping the global per-test timeout to 60s gives the heavy graph render breathing room. Most tests complete in <2s so the bump is inert for them; it specifically unblocks the SVG-heavy assertions on the dogfood dataset. A future optimization would move layout/SVG out of the synchronous response (lazy-load via HTMX), but that's a code change for a separate PR. Trace: skip
AI Code Review for PR #214pulseengine/rivet: Summary of ChangesThe PR addresses a timeout issue in the Potential Bugs or Issues
Security ConcernsThere are no security concerns associated with this change as it involves modifying configuration files for testing purposes. Suggestions for Improvement
Overall AssessmentThe change is necessary to unblock CI runs that were failing due to test timeouts. However, it introduces potential issues with increased latency and testing stability. The suggestion to move graph layout/SVG generation into a lazy-load mechanism is the most promising long-term solution. This review was generated by a local AI model. It is advisory only and may contain inaccuracies. Reviewed at |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: e8e79f9 | Previous: 77fa2e0 | Ratio |
|---|---|---|---|
store_lookup/100 |
2103 ns/iter (± 12) |
1685 ns/iter (± 23) |
1.25 |
store_lookup/1000 |
26010 ns/iter (± 141) |
19210 ns/iter (± 128) |
1.35 |
traceability_matrix/1000 |
59942 ns/iter (± 890) |
40182 ns/iter (± 516) |
1.49 |
query/100 |
779 ns/iter (± 2) |
645 ns/iter (± 1) |
1.21 |
query/1000 |
7146 ns/iter (± 27) |
5456 ns/iter (± 91) |
1.31 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Why
Latest CI on main showed the
diagram-viewer.spec.ts:graphtests failing despite the?limit=2000fix in cc8403e. Page snapshot from the failed retry shows the graph did render ("742 nodes, 1477 edges"), but layout + SVG generation took ~25-30s synchronously inrender_graph_view, leaving <5s for the.toBeVisible()assertion before the 30s test timeout fired.Fix
Bump the global per-test timeout from 30s to 60s in
playwright.config.ts. Most tests complete in <2s so the bump is inert; it specifically unblocks the SVG-heavy diagram-viewer assertions.Future work (separate PR)
The real fix is moving graph layout/SVG out of the synchronous response (lazy-load via HTMX). Tracked as a follow-up; this timeout bump unblocks CI now.
Test plan