Skip to content

Comments

feat: HTML test report generation#4946

Merged
thomhurst merged 2 commits intomainfrom
feat/html-reports
Feb 19, 2026
Merged

feat: HTML test report generation#4946
thomhurst merged 2 commits intomainfrom
feat/html-reports

Conversation

@thomhurst
Copy link
Owner

Closes #4887. Adds HtmlReporter with --report-html option.

@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Code Review

Overall, this is a clean, well-structured implementation that follows the existing JUnitReporter pattern closely. The approach is solid — opt-in via CLI flags, self-contained HTML with no external dependencies, and correct HTML encoding of user-controlled data. No bugs were found.

A few suggestions for improvement:


1. Use BCL's WebUtility.HtmlEncode instead of a custom implementation

HtmlReporter.cs lines 437–450:

private static string HtmlEncode(string? value)
{
    if (string.IsNullOrEmpty(value)) return string.Empty;
    return value
        .Replace("&", "&")
        .Replace("<", "&lt;")
        .Replace(">", "&gt;")
        .Replace("\"", "&quot;")
        .Replace("'", "&#39;");
}

Why this matters: The chained .Replace() calls allocate 4 intermediate strings per invocation, and the method is called multiple times per test row. More importantly, rolling a custom HTML encoder risks edge-case misses over time. System.Net.WebUtility.HtmlEncode() is already in the BCL (no new dependencies), handles a broader set of characters correctly, and produces a single allocation. Suggested replacement:

private static string HtmlEncode(string? value)
    => string.IsNullOrEmpty(value) ? string.Empty : System.Net.WebUtility.HtmlEncode(value);

2. ConsumeAsync should return ValueTask per CLAUDE.md performance guidelines

HtmlReporter.cs lines 165–173:

public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
    ...
    return Task.CompletedTask;
}

CLAUDE.md Rule 4 (Performance First) states: "Use ValueTask for potentially-sync operations." This method is always synchronous and returns Task.CompletedTask, which causes a heap allocation on every test result update (a hot path during test execution). If the IDataConsumer interface in Microsoft.Testing.Platform supports ValueTask, this should be changed. (Note: JUnitReporter.cs has the same pattern — fixing it here and there in tandem would be ideal.)


3. Modern C# index syntax (^1)

HtmlReporter.cs line ~194:

lastUpdates[kvp.Key] = kvp.Value[kvp.Value.Count - 1];

CLAUDE.md requires modern C# features. C# 8+ supports ^1 for last-element access:

lastUpdates[kvp.Key] = kvp.Value[^1];

(Same pre-existing pattern exists in JUnitReporter.cs.)


None of these are blocking — the implementation is correct and the overall design is good.

Add an HtmlReporter that generates a self-contained HTML file with test
results, enabled via --report-html or --report-html-filename flags.

The report includes a summary section with pass/fail/skip counts, a
detailed results table with test names, status badges, durations, and
error messages, all with inline CSS styling and color coding.
Replace the custom HtmlEncode method with System.Net.WebUtility.HtmlEncode
which handles all HTML encoding edge cases properly.
This was referenced Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: HTML and Markdown test report generation

1 participant