Conversation
…alization failure System.Text.Json's Utf8JsonWriter limits a single string token to int.MaxValue / 6 characters. Test suites with excessive logging (~380MB+ output per test) would hit this limit during HTML report generation, resulting in: Warning: HTML report generation failed: The JSON value of length 397639975 is too large and not supported. Fix: cap stdout and stderr at 1MB each in ExtractTestResult. If truncated, a message noting the original character count is appended. Fixes #5479"
There was a problem hiding this comment.
Code Review
Overall: Clean, targeted fix for a real user-facing bug. The approach is correct and the implementation is clear.
What the PR does
Caps / at 1 MB each in before they reach , preventing the System.Text.Json hard limit (~357M chars) from silently dropping the entire HTML report.
Observations
1. Missing unit tests for TruncateOutput
The helper is pure and easy to unit test. Without a test, future refactors could silently regress the truncation message format or the boundary condition (value.Length == MaxOutputLength should pass through unmodified).
A minimal test would cover:
null→null- String at exactly the limit → returned as-is (no truncation note appended)
- String one char over the limit → truncated with the note
2. String is allocated twice on the truncation path
return value[..MaxOutputLength] + $"\n[... output truncated — {value.Length:N0} total characters]";value[..MaxOutputLength] materialises a new 1 MB string, then the interpolation concatenates it into a second string. Since this path is only hit for pathologically large outputs this is not a hot-path concern, but string.Concat or string.Create would express the intent more efficiently:
return string.Concat(value.AsSpan(0, MaxOutputLength),
$"\n[... output truncated — {value.Length:N0} total characters]");This still allocates once (the final string) rather than twice.
3. Hardcoded 1 MB limit
1 MB is a very reasonable default, but there is no escape hatch for test suites that intentionally produce dense structured output (e.g. binary-encoded logs) and want more. A possible future improvement: respect an environment variable like TUNIT_HTML_MAX_OUTPUT_LENGTH. Not required for this fix, just something to keep in mind.
4. Surrogate-pair boundary (theoretical)
value[..MaxOutputLength] slices at a UTF-16 char boundary, not a Unicode scalar boundary. If the 1,048,576th char is the high surrogate of a supplementary character, the truncated string ends with an unpaired surrogate — which is technically invalid UTF-16. Utf8JsonWriter will throw on it. In practice this is astronomically unlikely for test output, but char.IsHighSurrogate(value[MaxOutputLength - 1]) → back off by 1 would make the code provably correct.
5. Placement of MaxOutputLength constant
The constant is declared after the method that uses it, sandwiched between two methods. C# allows this, but grouping constants at the top of the class (or at least before the first method) is the prevailing convention in this file and would make it easier to discover if someone wants to adjust the value.
Summary
| Correctness | Fixes the reported crash |
| Safety | Null-safe, handles edge cases |
| Missing | Unit tests for TruncateOutput |
| Minor | Double allocation on truncation path; theoretical surrogate split |
| Non-blocking | Hardcoded limit, constant placement |
The fix does what it says and the PR description is excellent. Addressing the missing tests would make this production-ready with more confidence.
There was a problem hiding this comment.
Code Review
Overall: Clean, targeted fix for a real user-facing bug. The approach is correct and the implementation is clear.
What the PR does
Caps stdout/stderr at 1 MB each in ExtractTestResult before they reach Utf8JsonWriter, preventing the System.Text.Json hard limit (~357M chars) from silently dropping the entire HTML report.
Observations
1. Missing unit tests for TruncateOutput
The helper is pure and easy to unit test. Without a test, future refactors could silently regress the truncation message format or the boundary condition (value.Length == MaxOutputLength should pass through unmodified).
A minimal test would cover:
- null returns null
- String at exactly the limit is returned as-is (no truncation note appended)
- String one char over the limit is truncated with the note
2. String is allocated twice on the truncation path
return value[..MaxOutputLength] + $"\n[... output truncated - {value.Length:N0} total characters]";value[..MaxOutputLength] materialises a new 1 MB string, then the interpolation concatenates it into a second string. Since this path is only hit for pathologically large outputs this is not a hot-path concern, but using string.Concat with a span overload would allocate once rather than twice:
return string.Concat(value.AsSpan(0, MaxOutputLength),
$"\n[... output truncated - {value.Length:N0} total characters]");3. Hardcoded 1 MB limit
1 MB is a very reasonable default, but there is no escape hatch for test suites that intentionally produce dense structured output and want more. A possible future improvement: respect an environment variable like TUNIT_HTML_MAX_OUTPUT_LENGTH. Not required for this fix, just worth noting.
4. Surrogate-pair boundary (theoretical)
value[..MaxOutputLength] slices at a UTF-16 char boundary, not a Unicode scalar boundary. If the 1,048,576th char is the high surrogate of a supplementary character, the truncated string ends with an unpaired surrogate - technically invalid UTF-16, and Utf8JsonWriter may throw on it. In practice this is extremely unlikely for test output, but checking char.IsHighSurrogate(value[MaxOutputLength - 1]) and backing off by 1 would make the code provably correct.
5. Placement of MaxOutputLength constant
The constant is declared after the method that uses it, sandwiched between two methods. Grouping it at the top of the class (or at least before the first method) would make it easier to discover if someone wants to adjust the value.
Summary
| Aspect | Assessment |
|---|---|
| Correctness | Fixes the reported crash |
| Safety | Null-safe, handles edge cases |
| Missing | Unit tests for TruncateOutput |
| Minor | Double allocation on truncation path; theoretical surrogate split |
| Non-blocking | Hardcoded limit, constant placement |
The fix does what it says and the PR description is excellent. Addressing the missing tests would make this production-ready with more confidence.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 12 |
TIP This summary will be updated as you push new changes. Give us feedback
…lloc, add tests - Move MaxOutputLength const to top of class (before fields) - Make TruncateOutput internal so TUnit.UnitTests can access it - Back off cut point by 1 if it lands on a high surrogate to avoid producing invalid UTF-16 that Utf8JsonWriter would reject - Use StringBuilder.Append(str, start, count) to avoid materialising the 1MB intermediate string from the slice (works on netstandard2.0) - Add HtmlReporterTruncateOutputTests covering null, empty, at limit, one-over-limit, large, and surrogate-pair-at-boundary cases"
Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.28.7 to 1.33.0. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.33.0 <!-- Release notes generated using configuration in .github/release.yml at v1.33.0 --> ## What's Changed ### Other Changes * perf: engine-wide performance optimizations by @thomhurst in thomhurst/TUnit#5520 * feat: Add TUnitSettings static API for programmatic configuration by @thomhurst in thomhurst/TUnit#5522 * perf: reduce allocations and improve hot-path performance by @thomhurst in thomhurst/TUnit#5524 * fix: enforce ParallelLimiter semaphore in TestRunner to prevent DependsOn bypass by @thomhurst in thomhurst/TUnit#5526 ### Dependencies * chore(deps): update tunit to 1.32.0 by @thomhurst in thomhurst/TUnit#5513 **Full Changelog**: thomhurst/TUnit@v1.32.0...v1.33.0 ## 1.32.0 <!-- Release notes generated using configuration in .github/release.yml at v1.32.0 --> ## What's Changed ### Other Changes * fix: auto-register correlated logging for minimal API hosts (#5503) by @thomhurst in thomhurst/TUnit#5511 * fix: cascade HookExecutorAttribute from class/assembly to hooks (#5462) by @thomhurst in thomhurst/TUnit#5512 ### Dependencies * chore(deps): update dependency polyfill to 10.3.0 by @thomhurst in thomhurst/TUnit#5508 * chore(deps): update tunit to 1.31.0 by @thomhurst in thomhurst/TUnit#5510 * chore(deps): update dependency polyfill to 10.3.0 by @thomhurst in thomhurst/TUnit#5509 **Full Changelog**: thomhurst/TUnit@v1.31.0...v1.32.0 ## 1.31.0 <!-- Release notes generated using configuration in .github/release.yml at v1.31.0 --> ## What's Changed ### Other Changes * feat(reporters): overhaul GitHub Actions step summary by @thomhurst in thomhurst/TUnit#5483 * fix: truncate large stdout/stderr in HTML report to prevent JSON serialization failure by @thomhurst in thomhurst/TUnit#5485 * feat(html-report): add failure clustering to test report by @thomhurst in thomhurst/TUnit#5490 * feat(html-report): add chevron affordance to failure cluster headers by @thomhurst in thomhurst/TUnit#5492 * feat(reporters): group GitHub summary failures by exception type by @thomhurst in thomhurst/TUnit#5491 * feat(reporters): add minimap sidebar navigator to HTML report by @thomhurst in thomhurst/TUnit#5494 * feat(html-report): add category/tag filter pills to toolbar by @thomhurst in thomhurst/TUnit#5496 * feat(html-report): omit redundant test body span from trace timeline by @thomhurst in thomhurst/TUnit#5497 * fix(tests): clear reporter env vars before each GitHubReporterTest to fix flaky CI on macOS/Windows by @thomhurst in thomhurst/TUnit#5499 * feat: add TestContext.MakeCurrent() for console output correlation by @thomhurst in thomhurst/TUnit#5502 * feat(html-report): add flaky test detection and summary section by @thomhurst in thomhurst/TUnit#5498 * fix: smarter stack trace filtering that preserves TUnit-internal traces by @thomhurst in thomhurst/TUnit#5506 * feat: add Activity baggage-based test context correlation by @thomhurst in thomhurst/TUnit#5505 ### Dependencies * chore(deps): update actions/github-script action to v9 by @thomhurst in thomhurst/TUnit#5476 * chore(deps): update tunit to 1.30.8 by @thomhurst in thomhurst/TUnit#5477 * chore(deps): update dependency polyfill to 10.2.0 by @thomhurst in thomhurst/TUnit#5482 * chore(deps): update dependency polyfill to 10.2.0 by @thomhurst in thomhurst/TUnit#5481 * chore(deps): update actions/upload-artifact action to v7.0.1 by @thomhurst in thomhurst/TUnit#5495 **Full Changelog**: thomhurst/TUnit@v1.30.8...v1.31.0 ## 1.30.8 <!-- Release notes generated using configuration in .github/release.yml at v1.30.8 --> ## What's Changed ### Other Changes * feat(mocks): migrate to T.Mock() extension syntax by @thomhurst in thomhurst/TUnit#5472 * feat: split TUnit.AspNetCore into Core + meta package by @thomhurst in thomhurst/TUnit#5474 * feat: add async Member() overloads for Task-returning member selectors by @thomhurst in thomhurst/TUnit#5475 ### Dependencies * chore(deps): update aspire to 13.2.2 by @thomhurst in thomhurst/TUnit#5464 * chore(deps): update dependency polyfill to 10.1.1 by @thomhurst in thomhurst/TUnit#5468 * chore(deps): update dependency polyfill to 10.1.1 by @thomhurst in thomhurst/TUnit#5467 * chore(deps): update tunit to 1.30.0 by @thomhurst in thomhurst/TUnit#5469 * chore(deps): update dependency microsoft.playwright to 1.59.0 by @thomhurst in thomhurst/TUnit#5473 **Full Changelog**: thomhurst/TUnit@v1.30.0...v1.30.8 ## 1.30.0 <!-- Release notes generated using configuration in .github/release.yml at v1.30.0 --> ## What's Changed ### Other Changes * perf: eliminate locks from mock invocation and verification hot paths by @thomhurst in thomhurst/TUnit#5422 * feat: TUnit0074 analyzer for redundant hook attributes on overrides by @thomhurst in thomhurst/TUnit#5459 * fix(mocks): respect generic type argument accessibility (#5453) by @thomhurst in thomhurst/TUnit#5460 * fix(mocks): skip inaccessible internal accessors when mocking Azure.Response by @thomhurst in thomhurst/TUnit#5461 * fix: apply CultureAttribute and STAThreadExecutorAttribute to hooks (#5452) by @thomhurst in thomhurst/TUnit#5463 ### Dependencies * chore(deps): update tunit to 1.29.0 by @thomhurst in thomhurst/TUnit#5446 * chore(deps): update react to ^19.2.5 by @thomhurst in thomhurst/TUnit#5457 * chore(deps): update opentelemetry to 1.15.2 by @thomhurst in thomhurst/TUnit#5456 * chore(deps): update dependency qs to v6.15.1 by @thomhurst in thomhurst/TUnit#5458 **Full Changelog**: thomhurst/TUnit@v1.29.0...v1.30.0 ## 1.29.0 <!-- Release notes generated using configuration in .github/release.yml at v1.29.0 --> ## What's Changed ### Other Changes * 🤖 Update Mock Benchmark Results by @thomhurst in thomhurst/TUnit#5420 * fix(mocks): resolve build errors when mocking Azure SDK clients by @thomhurst in thomhurst/TUnit#5440 * fix: deduplicate virtual hook overrides across class hierarchy (#5428) by @thomhurst in thomhurst/TUnit#5441 * fix(mocks): unique __argArray locals per event in RaiseEvent dispatch (#5423) by @thomhurst in thomhurst/TUnit#5442 * refactor(mocks): extract MockTypeModel.Visibility helper by @thomhurst in thomhurst/TUnit#5443 * fix(mocks): preserve nullable annotations on generated event implementations by @thomhurst in thomhurst/TUnit#5444 * fix(mocks): preserve nullability on event handler types (#5425) by @thomhurst in thomhurst/TUnit#5445 ### Dependencies * chore(deps): update tunit to 1.28.7 by @thomhurst in thomhurst/TUnit#5416 * chore(deps): update dependency polyfill to v10 by @thomhurst in thomhurst/TUnit#5417 * chore(deps): update dependency polyfill to v10 by @thomhurst in thomhurst/TUnit#5418 * chore(deps): update dependency mockolate to 2.4.0 by @thomhurst in thomhurst/TUnit#5431 * chore(deps): update mstest to 4.2.1 by @thomhurst in thomhurst/TUnit#5433 * chore(deps): update dependency microsoft.net.test.sdk to 18.4.0 by @thomhurst in thomhurst/TUnit#5435 * chore(deps): update microsoft.testing to 2.2.1 by @thomhurst in thomhurst/TUnit#5432 * chore(deps): update dependency microsoft.testing.extensions.codecoverage to 18.6.2 by @thomhurst in thomhurst/TUnit#5437 * chore(deps): update dependency @docusaurus/theme-mermaid to ^3.10.0 by @thomhurst in thomhurst/TUnit#5438 * chore(deps): update docusaurus to v3.10.0 by @thomhurst in thomhurst/TUnit#5439 **Full Changelog**: thomhurst/TUnit@v1.28.7...v1.29.0 Commits viewable in [compare view](thomhurst/TUnit@v1.28.7...v1.33.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.30.8 to 1.33.0. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.33.0 <!-- Release notes generated using configuration in .github/release.yml at v1.33.0 --> ## What's Changed ### Other Changes * perf: engine-wide performance optimizations by @thomhurst in thomhurst/TUnit#5520 * feat: Add TUnitSettings static API for programmatic configuration by @thomhurst in thomhurst/TUnit#5522 * perf: reduce allocations and improve hot-path performance by @thomhurst in thomhurst/TUnit#5524 * fix: enforce ParallelLimiter semaphore in TestRunner to prevent DependsOn bypass by @thomhurst in thomhurst/TUnit#5526 ### Dependencies * chore(deps): update tunit to 1.32.0 by @thomhurst in thomhurst/TUnit#5513 **Full Changelog**: thomhurst/TUnit@v1.32.0...v1.33.0 ## 1.32.0 <!-- Release notes generated using configuration in .github/release.yml at v1.32.0 --> ## What's Changed ### Other Changes * fix: auto-register correlated logging for minimal API hosts (#5503) by @thomhurst in thomhurst/TUnit#5511 * fix: cascade HookExecutorAttribute from class/assembly to hooks (#5462) by @thomhurst in thomhurst/TUnit#5512 ### Dependencies * chore(deps): update dependency polyfill to 10.3.0 by @thomhurst in thomhurst/TUnit#5508 * chore(deps): update tunit to 1.31.0 by @thomhurst in thomhurst/TUnit#5510 * chore(deps): update dependency polyfill to 10.3.0 by @thomhurst in thomhurst/TUnit#5509 **Full Changelog**: thomhurst/TUnit@v1.31.0...v1.32.0 ## 1.31.0 <!-- Release notes generated using configuration in .github/release.yml at v1.31.0 --> ## What's Changed ### Other Changes * feat(reporters): overhaul GitHub Actions step summary by @thomhurst in thomhurst/TUnit#5483 * fix: truncate large stdout/stderr in HTML report to prevent JSON serialization failure by @thomhurst in thomhurst/TUnit#5485 * feat(html-report): add failure clustering to test report by @thomhurst in thomhurst/TUnit#5490 * feat(html-report): add chevron affordance to failure cluster headers by @thomhurst in thomhurst/TUnit#5492 * feat(reporters): group GitHub summary failures by exception type by @thomhurst in thomhurst/TUnit#5491 * feat(reporters): add minimap sidebar navigator to HTML report by @thomhurst in thomhurst/TUnit#5494 * feat(html-report): add category/tag filter pills to toolbar by @thomhurst in thomhurst/TUnit#5496 * feat(html-report): omit redundant test body span from trace timeline by @thomhurst in thomhurst/TUnit#5497 * fix(tests): clear reporter env vars before each GitHubReporterTest to fix flaky CI on macOS/Windows by @thomhurst in thomhurst/TUnit#5499 * feat: add TestContext.MakeCurrent() for console output correlation by @thomhurst in thomhurst/TUnit#5502 * feat(html-report): add flaky test detection and summary section by @thomhurst in thomhurst/TUnit#5498 * fix: smarter stack trace filtering that preserves TUnit-internal traces by @thomhurst in thomhurst/TUnit#5506 * feat: add Activity baggage-based test context correlation by @thomhurst in thomhurst/TUnit#5505 ### Dependencies * chore(deps): update actions/github-script action to v9 by @thomhurst in thomhurst/TUnit#5476 * chore(deps): update tunit to 1.30.8 by @thomhurst in thomhurst/TUnit#5477 * chore(deps): update dependency polyfill to 10.2.0 by @thomhurst in thomhurst/TUnit#5482 * chore(deps): update dependency polyfill to 10.2.0 by @thomhurst in thomhurst/TUnit#5481 * chore(deps): update actions/upload-artifact action to v7.0.1 by @thomhurst in thomhurst/TUnit#5495 **Full Changelog**: thomhurst/TUnit@v1.30.8...v1.31.0 Commits viewable in [compare view](thomhurst/TUnit@v1.30.8...v1.33.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Fixes #5479
System.Text.Json'sUtf8JsonWriterlimits a single string token toint.MaxValue / 6 ≈ 357,913,941characters (worst-case UTF-16 →\uXXXXescaping). Test suites with excessive per-test logging (~380MB+) hit this limit during HTML report serialization, producing:\
Warning: HTML report generation failed: The JSON value of length 397639975 is too large and not supported.
\\
Fix
In
ExtractTestResult, stdout and stderr are now passed throughTruncateOutput()which caps each at 1 MB (1,048,576 chars). If a value is truncated, a note with the original character count is appended:\
[... output truncated — 397,639,975 total characters]
\\
This ensures the HTML report always generates successfully, even for test suites with extremely verbose output.
Files changed
TUnit.Engine/Reporters/Html/HtmlReporter.cs