Apply [StackTraceHidden] to Assert/CollectionAssert/StringAssert (#8277) by Evangelink · Pull Request #8393 · microsoft/testfx

Evangelink · 2026-05-20T14:59:33Z

What

Annotate every public assertion type with [StackTraceHidden] so the .NET 6+ runtime omits MSTest framework frames from Exception.StackTrace. Coverage:

Assert, CollectionAssert, StringAssert (every partial-class file is covered because the attribute is on the type, not per-method)
AssertExtensions (the Assert.That extension surface)
Internal helpers on the assertion call path: AssertScope, StructuredAssertionMessage, EvidenceBlock, EvidenceLine, AssertionValueRenderer
Every nested Assert*InterpolatedStringHandler struct (those have their own DeclaringType, so the outer Assert's attribute does not propagate)

The polyfill at src/Polyfills/StackTraceHiddenAttribute.cs already provides the attribute as an embedded internal type on netstandard2.0/net462 (no-op there) and forwards to the BCL type on .NET 6+.

Why

Fixes the AzDO reporter heuristic at the source. Today the AzDO reporter walks the stack with a type-prefix heuristic to skip framework frames (Assert.IComparable not compatible with AzDoReport extension #6925). On .NET 6+ those frames now disappear before the reporter even sees them, so the annotation naturally points at the user's test method.
Cleans up every stack-trace consumer. TerminalReporter, IDE Test Explorer, TRX writer, custom log sinks all stop seeing the noise.
Mechanical, type-level change — the attribute is added once per top-level/nested type instead of per-method, so future Assert APIs are covered automatically without further changes.

Tests

test/UnitTests/TestFramework.UnitTests/Assertions/AssertStackTraceHiddenTests.cs — eight tests that force a real assertion failure (AreEqual, IsGreaterThan, IsNull, Fail, CollectionAssert.AreEqual, StringAssert.Contains, an interpolated-handler variant, and a failure collected via Assert.Scope()) and verify on .NET 6+ that the captured Exception.StackTrace contains no MSTest framework frame.
RealAssertFailure_AnnotationPointsAtTestMethodAndNotAtFrameworkFile in AzureDevOpsTests.cs — end-to-end test that exercises a real Assert.AreEqual(1, 2) failure through AzureDevOpsReporter.GetErrorText and asserts the emitted sourcepath points at the test method's source file rather than any framework partial-class file.

Notes

[StackTraceHidden] is a no-op on netstandard2.0 / .NET Framework consumers; the AzDO reporter's existing type-prefix heuristic continues to cover that case.
The pre-existing per-method [StackTraceHidden] annotations inside Assert.cs / Assert.ThrowsException.cs are now redundant under the type-level attribute but were left in place to keep the diff minimal.
No public API changes; PublicAPI.*.txt files are unaffected because [StackTraceHidden] does not surface in API signatures.

#8277) Annotate every public assertion type — Assert, CollectionAssert, StringAssert, AssertExtensions (Assert.That) — and the internal helpers that sit on the assertion call path (AssertScope, StructuredAssertionMessage, EvidenceBlock, EvidenceLine, AssertionValueRenderer) plus the nested InterpolatedStringHandler structs with [StackTraceHidden]. On .NET 6+ the runtime now omits MSTest framework frames from Exception.StackTrace, so the user-visible call stack points at the test method instead of e.g. Assert.IComparable.cs. On netstandard2.0 / .NET Framework the attribute is a no-op (polyfilled internal type) and stacks remain unchanged. Adds AssertStackTraceHiddenTests verifying that no MSTest framework frame appears in the captured stack for representative assertion failures on .NET 6+, plus an end-to-end AzureDevOpsReporter test that asserts a real Assert.AreEqual failure annotates the test method's source file rather than a framework partial-class file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Walk every public method on Assert/CollectionAssert/StringAssert/AssertExtensions and assert [StackTraceHidden] is reachable (directly or via the declaring type). This means a newly added public assertion API automatically gets verified — no test author needs to remember to add a per-API check. Also verify the nested *InterpolatedStringHandler structs and the internal helpers on the assertion call path (AssertScope, StructuredAssertionMessage, AssertionValueRenderer, EvidenceBlock, EvidenceLine) carry the attribute. Two runtime smoke tests (Assert.AreEqual and Assert.Scope) remain to guard against the polyfill or the BCL silently ceasing to honor the attribute — reflection cannot catch that class of regression. Verified on net9.0 and net48 (the polyfill is detected by FullName, so the test is cross-TFM). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Evangelink

Code Review — PR #8393: Apply `[StackTraceHidden]` to Assert/CollectionAssert/StringAssert

⚠️ Important: Diff presented for review does not match actual PR branch HEAD

The "Full PR Diff" provided in this review request is substantially inaccurate relative to refs/pull/8393/head. I read the actual files from the PR branch to verify. Here is what I found:

Diff vs Reality — Key Discrepancies

Claim in provided diff	Actual state in PR branch HEAD
`Assert.cs`, `CollectionAssert.cs`, `StringAssert.cs` already had `[DebuggerNonUserCode]` + `[StackTraceHidden]` before PR	Main branch has neither attribute at class level
PR adds a duplicate `[StackTraceHidden]` to those three files	PR adds exactly one `[StackTraceHidden]` to each — clean
`AssertScope.cs` already had `[StackTraceHidden]` — PR duplicates it	PR adds exactly one `[StackTraceHidden]` — clean
Test file uses `[TestClass]`/`[TestMethod]` MSTest attributes with `#if NET`	Actual test file uses `TestContainer` base class (internal framework), `AwesomeAssertions`, `#if NET6_0_OR_GREATER`
Project file modified is `Microsoft.TestFramework.UnitTests.csproj` with inline XML comment	That file does not exist; actual project `TestFramework.UnitTests.csproj` is not changed by this PR

Review of the Actual PR (from `refs/pull/8393/head`)

Correctness ✅
Every assertion type gets exactly one [StackTraceHidden]. No duplicates. Nested interpolated-string handler structs are annotated individually (required because they are separate DeclaringTypes). Internal helpers on the call path (AssertScope, StructuredAssertionMessage, AssertionValueRenderer, EvidenceBlock, EvidenceLine) are annotated. The [StackTraceHidden] on EvidenceBlock/EvidenceLine readonly structs is harmless — those structs' methods won't appear in stack traces under normal usage, but the annotation is defensive and costs nothing.

[DebuggerNonUserCode] removal ✅
The main branch never had [DebuggerNonUserCode] at the class level on Assert, CollectionAssert, or StringAssert. The PR does not add or remove it. No debugger-stepping regression.

AssertExtensions (internal static) ✅
[StackTraceHidden] applies to static classes and their methods. Extension methods defined there will be hidden correctly.

Tests ✅
The actual test file (AssertStackTraceHiddenTests.cs) correctly:

Extends TestContainer (as required by the internal test framework)
Uses AwesomeAssertions for assertions
Gates the stack-trace content assertions under #if NET6_0_OR_GREATER — more precise than #if NET
Covers Assert, CollectionAssert, StringAssert, an interpolated-handler variant, and the AssertScope path

Cross-TFM ✅
The polyfill at src/Polyfills/StackTraceHiddenAttribute.cs provides the attribute on netstandard2.0/net462 (no-op). No behavioral change on .NET Framework.

Public API ✅
[StackTraceHidden] does not surface in API signatures. PublicAPI.*.txt files correctly unaffected.

Verdict

Dimension	Result
Correctness	✅
Duplicate attributes	✅ None in actual code
Cross-TFM compatibility	✅
Test quality	✅
Public API surface	✅ Unaffected
Debugger experience	✅ No regression

The implementation in the actual PR branch is clean and well-considered. No blocking issues found. Note to PR author: the diff representation provided to reviewers was misleading; consider verifying the diff source used for review preparation.

Generated by Expert Code Review (on open) for issue #8393 · ● 10.6M

…g ##vso in failure messages The test was failing on CI because: 1. With <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>, Roslyn rewrites source paths via /pathmap, so [CallerFilePath] returns '/_/test/...'. The AzDO reporter strips that prefix and emits 'sourcepath=test/...', but the test was building 'expectedRelativeFile' by only stripping the runtime repo root, leaving '/_/test/...' as the expected substring -- which never matched. 2. When the Assert.Contains/DoesNotContain failure messages embedded the raw '##vso[task.logissue...]' text, AzDO scanned the test output, saw '##vso[' and tried to interpret it as a real command, surfacing 'Unable to process command ... successfully'. Fix: mirror the same '/_/' -> '' stripping as the reporter, and trim the leading '##' from the captured text plus strip '##vso[' from the logger trace output before embedding either in failure messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot's findings

Files reviewed: 20/20 changed files
Comments generated: 1

+    {
+        Type[] nestedTypes = ownerType.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic);
+        Type[] handlers = nestedTypes
+            .Where(static t => t.Name.EndsWith("InterpolatedStringHandler", StringComparison.Ordinal))


Copilot AI review requested due to automatic review settings May 20, 2026 14:59

Copilot started reviewing on behalf of Evangelink May 20, 2026 14:59 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Evangelink commented May 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 20, 2026 17:17

Copilot started reviewing on behalf of Evangelink May 20, 2026 17:17 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply [StackTraceHidden] to Assert/CollectionAssert/StringAssert (#8277)#8393

Apply [StackTraceHidden] to Assert/CollectionAssert/StringAssert (#8277)#8393
Evangelink wants to merge 3 commits into
mainfrom
dev/amauryleve/issue-8277-stacktracehidden

Evangelink commented May 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Evangelink left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Evangelink commented May 20, 2026

What

Why

Tests

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Evangelink left a comment

Choose a reason for hiding this comment

Code Review — PR #8393: Apply [StackTraceHidden] to Assert/CollectionAssert/StringAssert

⚠️ Important: Diff presented for review does not match actual PR branch HEAD

Diff vs Reality — Key Discrepancies

Review of the Actual PR (from refs/pull/8393/head)

Verdict

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Code Review — PR #8393: Apply `[StackTraceHidden]` to Assert/CollectionAssert/StringAssert

Review of the Actual PR (from `refs/pull/8393/head`)