feat(html-report): add failure clustering to test report

[thomhurst]

Summary

Adds Failure Clustering to the HTML test report, grouping test failures by exception type and top stack frame to make it easier to identify systemic failures at a glance.

Changes

• Failure Clusters section: A new collapsible section appears above the test list when there are 2+ failed tests. Failures are grouped by [ExceptionType, topStackFrame] and sorted by count (most frequent first).
• Each cluster shows the exception type, the top stack frame, the failure count badge, and a snippet of the first error message.
• Clicking a cluster header expands/collapses the list of affected tests.
• Clicking an individual test name scrolls to that test in the full results.

Screenshot

The Failure Clusters section renders between the Failed Tests summary and the Slowest Tests section, using the same collapsible card style as the rest of the report.

[claude]

Code Review: feat(html-report): failure clustering + parallel execution timeline

Both features are a great addition — failure clustering in particular will be very useful for diagnosing flaky infrastructure failures at a glance, and the Gantt timeline gives genuine insight into test parallelism. The implementation is frontend-only with no data-model changes, which is exactly the right scope for this.

No previous review comments to compare against.

---

Bug: Math.max.apply stack overflow on large test suites

In renderParallelTimeline:

constglobalMax=Math.max.apply(null,allTests.map(function(f){returnf.end;}));

Function.prototype.apply pushes all array elements as arguments onto the call stack. For large test suites (typically >~100k elements but can trigger earlier), this throws Maximum call stack size exceeded. Prefer:

constglobalMax=allTests.reduce((m,f)=>Math.max(m,f.end),-Infinity);// or: Math.max(...allTests.map(f => f.end))

The reduce form avoids the stack risk entirely. This is worth fixing since TUnit can easily produce thousands of tests.

---

Bug: Unescaped test IDs in HTML attributes

In both new functions, test IDs are injected raw into attribute values:

h+='<div class="fc-test" data-scroll-tid="'+f.t.id+'">';h+='... data-pt-tid="'+f.t.id+'"';

If a test ID contains " or > (which is possible with parameterized tests), this breaks the HTML attribute or creates unexpected markup. The existing esc() helper should be used here:

h+='<div class="fc-test" data-scroll-tid="'+esc(f.t.id)+'">';

---

Tooltip not hidden when cursor exits the section

The mouseout handler only hides the tooltip when leaving a .pt-bar:

sec.addEventListener('mouseout',function(e){constbar=e.target.closest('.pt-bar');if(bar&&tip){tip.style.display='none';}});

If the user moves quickly from a bar directly to outside the .pt-canvas (without passing over another .pt-bar), the tooltip stays visible. A more robust pattern is mouseleave on the container:

sec.addEventListener('mouseleave',function(){if(tip)tip.style.display='none';});

---

Dead CSS: .pt-lane class is defined but never used

.pt-lane is in the stylesheet but no .pt-lane divs are emitted — bars are positioned absolutely within .pt-chart with inline top values instead. If this is intentional (the CSS is vestigial from an earlier design), it can be removed.

---

Minor: P90 threshold breaks down with small test counts

constp90=durs[Math.floor(durs.length*0.9)]||0;

With exactly 10 tests Math.floor(10 * 0.9) === 9, which is index of the slowest test — so only the single slowest test is highlighted. With fewer than 10 tests the p90 can be surprisingly high. Consider flooring at the 80th percentile for small counts, or capping at e.g. Math.min(Math.floor(durs.length * 0.9), durs.length - 2) so at least the top 2 get highlighted.

---

Minor: Clustering key collision

constkey=type+'||'+frame;

If type itself contains ||, two different (type, frame) pairs could map to the same key. Low probability in practice, but using a Map with a tuple key or a JSON-serialised object avoids the ambiguity entirely:

constkey=JSON.stringify([type,frame]);

---

Minor: data-fci attribute is set but never read

Each cluster gets data-fci="'+ci+'" but the click handler navigates from the event target directly via .closest('.fc-cluster') and never reads this attribute. It can be removed.

---

Summary

Severity	Issue
Important	Math.max.apply stack overflow potential
Important	Unescaped test IDs in HTML attributes
Moderate	Tooltip not hidden on fast cursor exit
Minor	Dead .pt-lane CSS class
Minor	P90 edge case with small test counts
Minor	Clustering key collision possibility
Minor	Unused data-fci attribute

The overall design and UX is solid. The greedy lane-packing algorithm is correct and efficient. The esc() usage in tooltip HTML is good. The two Important items are worth fixing before merge; the rest are polish.

[codacy-production]

Up to standards ✅

🟢 Issues0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics0 complexity

Metric	Results
Complexity	0

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Follow-up Review: All Previous Issues Addressed

The PR has been updated since the initial review (commit 8b53da42d). All seven previously raised issues have been fixed — good turnaround.

Previous Issues — Status

Issue	Status
Math.max.apply stack overflow	✅ Fixed — now uses reduce
Unescaped test IDs in HTML attributes	✅ Fixed — esc(f.t.id) used throughout
Tooltip not hidden on fast cursor exit	✅ Fixed — mouseleave on container
Dead .pt-lane CSS class	✅ Fixed — removed
P90 edge case with small test counts	✅ Fixed — Math.min(..., durs.length - 2)
Clustering key collision via `
Unused data-fci attribute	✅ Fixed — removed

Remaining Minor Observations

1. f.t.status used unescaped as a CSS class

Both in failure cluster badges and timeline bars:

h+='<span class="t-badge '+f.t.status+'">'// failure clusterh+='<div class="pt-bar '+f.t.status+'"'// timeline

Since status values are emitted by TUnit's engine (a controlled enum: passed, failed, error, etc.), this is very low risk in practice. Just worth noting for completeness — if a future status ever contained a space or quote, it would break class attribute parsing.

2. Tooltip div is never removed from the DOM

if(!tip){tip=document.createElement('div');tip.className='pt-tooltip';document.body.appendChild(tip);}

tip is let-scoped inside renderParallelTimeline, so if the function were ever called multiple times (e.g., from a hot-reload or re-render path), each call would orphan the previous tip element in document.body. For a static HTML report rendered once this is a non-issue, but it is worth being aware of.

---

Both remaining observations are genuinely minor given the single-render nature of the HTML report. The implementation is solid — the greedy lane-packing is correct and efficient, the esc() discipline is consistent throughout, and the section-level mouseleave tooltip handling is clean. Ready to merge.

[claude]

Third-Round Review: Second-Round Observations Addressed

The third commit () resolves both remaining observations from the previous review.

Previous Observations — Status

Observation	Status
f.t.status used unescaped as a CSS class	✅ Fixed — safeClass() helper added and applied in both render functions
Tooltip div orphaned on re-render	✅ Addressed — eager creation at render time removes the lazy-per-hover allocation

---

One Minor Note

const ptData = allTests is a redundant alias

ptData is only used in the event listener closures that are set up in the same function, where allTests is directly accessible. This alias adds no clarity and can simply be removed, referencing allTests[idx] directly in the mouseover handler. Purely cosmetic — not a blocker.

---

On the Tooltip Orphaning Fix

Worth noting: moving creation from inside mouseover to eager/synchronous doesn't fully prevent orphaning if renderParallelTimeline were called multiple times (each call still appends a new div to body). However, the commit message acknowledges this is a static report rendered once, so the fix is correct in scope and the observation is closed.

---

Summary

All previously raised issues have been addressed across three iterations. The safeClass() helper is a clean, reusable addition. The greedy lane-packing, esc() discipline, and mouseleave tooltip handling all remain solid. Ready to merge.

[claude]

Third-Round Review: Second-Round Observations Addressed

The third commit (31410be) resolves both remaining observations from the previous review.

Previous Observations — Status

Observation	Status
f.t.status used unescaped as a CSS class	Fixed — safeClass() helper added and applied in both render functions
Tooltip div orphaned on re-render	Addressed — eager creation at render time removes the lazy-per-hover allocation

---

One Minor Note

const ptData = allTests is a redundant alias

ptData is only used in the event listener closures set up in the same function, where allTests is directly accessible. It adds no clarity and can simply be removed, referencing allTests[idx] directly in the mouseover handler. Purely cosmetic — not a blocker.

---

On the Tooltip Orphaning Fix

Worth noting: moving creation from inside mouseover to eager/synchronous doesn't fully prevent orphaning if renderParallelTimeline were called multiple times — each call still appends a new div to body. However, the commit message acknowledges this is a static report rendered once, so the fix is correct in scope and the observation is closed.

---

Summary

All previously raised issues have been addressed across three iterations. The safeClass() helper is a clean, reusable addition. The greedy lane-packing, esc() discipline, and mouseleave tooltip handling all remain solid. Ready to merge.

[claude]

Fourth-Round Review: Round-3 Observations Resolved

The final outstanding observation from the previous review — const ptData = allTests redundant alias — is confirmed resolved: ptData does not appear anywhere in the current file.

Previous Observations — Final Status

Round	Observation	Status
1	Math.max.apply stack overflow	✅ Fixed
1	Unescaped test IDs in HTML attributes	✅ Fixed
1	Tooltip not hidden on fast cursor exit	✅ Fixed
1	Dead .pt-lane CSS class	✅ Fixed
1	P90 edge case with small test counts	✅ Fixed
1	Clustering key collision via `
1	Unused data-fci attribute	✅ Fixed
2	f.t.status unescaped as CSS class	✅ Fixed — safeClass() applied
2	Tooltip div orphaned on re-render	✅ Addressed
3	const ptData = allTests redundant alias	✅ Fixed — not present

---

One New Minor Observation

Individual cluster items have no expand/collapse affordance

The section-level header correctly uses tlArrow as a visual cue. However, each individual .fc-cluster item's .fc-hd is entirely clickable to expand/collapse, but carries no arrow or chevron icon — users have only the :hover background change as a hint that the item is interactive.

This is consistent with how other sections in the report may behave, and since the cursor becomes a pointer via cursor:pointer in the CSS, it is discoverable. But adding a small rotating chevron (matching the tl-arrow pattern) to .fc-hd would make the affordance explicit and consistent with the section-level toggle:

/* existing tl-arrow rotates on .open — same pattern works for fc */
.fc-cluster.open .fc-arrow { transform:rotate(90deg); }

This is a polish item only and not a blocker.

---

Summary

The implementation is clean: safeClass() is applied throughout, esc() discipline is consistent, the cluster key uses JSON.stringify, and ptData is gone. The failure clustering feature is production-ready. Ready to merge.