Assert.That: additional test cases for object-typed sub-expressions (stacks on #8307) by Evangelink · Pull Request #8308 · microsoft/testfx

Evangelink · 2026-05-17T19:53:41Z

Stacks on top of #8307 (dev/amauryleve/assert-that).

Adds an extra test case from @jakubjares exploring Assert.That behavior when sub-expressions evaluate to object references (where the diagnostic message can be confusing). The test That_DoesEvaluateTwice_WhenMethodIsLeaf2 and helper types (Shape, BoxOfShapes) were authored on 2025-11-25.

Warning

Marked as draft because the new test currently asserts WithMessage(""""""non""""""), which is clearly a placeholder. The test expectation needs to be updated to the real expected failure message (and the test renamed if appropriate) before this PR is ready for review.

Merge notes

This branch was rebased onto main (commit 6b0bae4). Same resolution strategy as #8307:

Assert.That.cs: kept the new EvaluateExpression / cache logic; adopted main's StringBuilder.Append(char, int) overload while preserving the MaxConsecutiveParentheses constant.
FrameworkMessages.resx: kept AssertThatFailedToEvaluate alongside all new RFC 012 summary entries from main; xlf files regenerated via dotnet msbuild /t:UpdateXlf.
Guarded [RequiresDynamicCode(...)] attributes with #if NET7_0_OR_GREATER for netstandard2.0 / net462 compatibility.

Validation

dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug -> clean across netstandard2.0, net462, net8.0, net9.0.
Tests not run here because the new test has a placeholder expectation; run them after fixing the expectation.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Evangelink · 2026-05-18T07:05:53Z

@copilot resolve the merge conflicts in this pull request

Copilot · 2026-05-18T07:13:47Z

@copilot resolve the merge conflicts in this pull request

Done in 0ad58372d: I merged dev/amauryleve/assert-that into this branch and resolved the conflicts (the XLF conflicts were resolved to match the base branch).

Evangelink · 2026-05-18T17:53:20Z

Pushed 12c8549:

Merged the latest origin/dev/amauryleve/assert-that (incl. the PR Fix Assert.That to not evaluate expression twice (re-opens #6699) #8307 fix) and resolved the conflict in AssertTests.That.cs.
Fixed the two compile errors on the new Shape test helper: CS8618 (non-nullable Name not initialized — now = string.Empty) and IDE0022 (use expression body — ToString() is now expression-bodied).

The PR is still draft because That_DoesEvaluateTwice_WhenMethodIsLeaf2 still asserts WithMessage("non") placeholder — the real expected message is now:

Assert.That(() => box.GetValueWithSideEffect() != box.GetValueWithSideEffect()) failed.
Details:
  box.GetValueWithSideEffect() = Shape: Circle

(only one entry because the second call mutates the same Shape reference, so the duplicate displayName de-dupes — which is the UX concern this PR was meant to highlight). Left the placeholder as-is so you can decide whether to lock that in or rename/restructure the test.

Evangelink · 2026-05-19T09:20:02Z

@copilot resolve the merge conflicts in this pull request

Copilot · 2026-05-19T09:28:36Z

@copilot resolve the merge conflicts in this pull request

Already resolved in 12c854990 (latest merge from origin/dev/amauryleve/assert-that). I verified there are no remaining merge conflicts (git ls-files -u is empty, no conflict markers, and dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug succeeds).

Evangelink · 2026-05-19T10:53:40Z

@copilot resolve the merge conflicts in this pull request

Evangelink · 2026-05-19T10:56:11Z

@copilot resolve the merge conflicts in this pull request

Copilot · 2026-05-19T11:00:22Z

@copilot resolve the merge conflicts in this pull request

Already resolved in 12c854990. I re-checked this branch: no unresolved merge entries (git ls-files -u = 0) and dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug succeeds.

Copilot · 2026-05-19T11:12:05Z

@copilot resolve the merge conflicts in this pull request

There are no unresolved merge conflicts on this branch (git ls-files -u is empty and no Git conflict markers are present). I also re-ran dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug, and it succeeds.

Evangelink · 2026-05-19T11:26:19Z

@copilot resolve the merge conflicts in this pull request

Copilot · 2026-05-19T11:32:54Z

@copilot resolve the merge conflicts in this pull request

I re-checked the branch: there are no unresolved merge conflicts (git ls-files -u is empty, and no Git conflict markers are present). I also reran dotnet build src/TestFramework/TestFramework/TestFramework.csproj -c Debug, and it succeeds.

…ted reference Resets PR #8308 onto current main and re-adds the unique contribution from @jakubjares as a single test plus its helper types. After PR #8307 was merged via #8306 and PR #8358 added the CA1305 fix, the old branch was hopelessly diverged from main. Instead of trying to thread the needle through 16+ source conflicts, this commit takes main's source/test files verbatim and only adds: * That_TwoCallsReturningSameMutatedReference_DeduplicatesInDetails * Shape (helper) * BoxOfShapes (helper) The test pins down the current UX limitation: when two calls return the SAME mutable reference and the comparison is reference-inequality, the cache stores the same reference for both call expressions; by the time details are extracted the object has been mutated, both slots resolve to the same value, and TryAddExpressionValue's same-value dedupe means only one entry surfaces (`Shape: Circle`). Documenting this behavior gives us a regression guard if we ever decide to improve the diagnostic. Replaces the previous `WithMessage("non")` placeholder with the real expected message. All 76 That_* tests pass. Co-authored-by: Jakub Jareš <me@jakubjares.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Evangelink · 2026-05-19T13:38:19Z

I've reset this branch onto current main and re-added the unique contribution from this PR as a single commit (10450c2):

Cherry-picked That_DoesEvaluateTwice_WhenMethodIsLeaf2 plus its Shape / BoxOfShapes helpers
Renamed the test to That_TwoCallsReturningSameMutatedReference_DeduplicatesInDetails so the intent is visible in the test name
Replaced the WithMessage(""non"") placeholder with the real expected message: box.GetValueWithSideEffect() = Shape: Circle (verified by running the test)

Why a reset rather than a merge: this branch was stacked on dev/amauryleve/assert-that (#8307), which then merged into main via #8306 and got further tweaks (#8358). The result was 16+ source-file conflicts plus xlf churn — the bot kept saying ""no conflicts"" because it was looking at the local working tree after each merge attempt rather than at the GitHub mergeability state. Squashing onto main avoids re-litigating the architectural rewrite.

Marking as ready for review.

…ted reference Resets PR #8308 onto current main and re-adds the unique contribution from @jakubjares as a single test plus its helper types. After PR #8307 was merged via #8306 and PR #8358 added the CA1305 fix, the old branch was hopelessly diverged from main. Instead of trying to thread the needle through 16+ source conflicts, this commit takes main's source/test files verbatim and only adds: * That_TwoCallsReturningSameMutatedReference_DeduplicatesInDetails * Shape (helper) * BoxOfShapes (helper) The test pins down the current UX limitation: when two calls return the SAME mutable reference and the comparison is reference-inequality, the cache stores the same reference for both call expressions; by the time details are extracted the object has been mutated, both slots resolve to the same value, and TryAddExpressionValue's same-value dedupe means only one entry surfaces (`Shape: Circle`). Documenting this behavior gives us a regression guard if we ever decide to improve the diagnostic. Replaces the previous `WithMessage("non")` placeholder with the real expected message. All 76 That_* tests pass. Co-authored-by: Jakub Jareš <me@jakubjares.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new unit test to pin down current Assert.That diagnostic behavior when the same mutable reference is returned from multiple side-effecting sub-expressions, leading to deduped “Details” output.

Changes:

Added a new test asserting the current “Details” output when two calls return the same mutated reference.
Introduced helper types (Shape, BoxOfShapes) to reproduce the side-effecting reference scenario.

Show a summary per file

File	Description
test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs	Adds a regression test (plus helper types) to lock in current `Assert.That` diagnostics for repeated mutable reference values.

Copilot's findings

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

+    // method calls return the same mutable reference. The cache stores reference values, so by
+    // the time details are extracted both slots point to the post-mutation object; with
+    // first-occurrence-by-name and "same value -> dedupe" semantics in TryAddExpressionValue,
+    // only one entry surfaces. This is a known UX limitation worth pinning down with a test.


+    // ---- Object-typed sub-expressions: locks down current behavior when two side-effecting
+    // method calls return the same mutable reference. The cache stores reference values, so by
+    // the time details are extracted both slots point to the post-mutation object; with
+    // first-occurrence-by-name and "same value -> dedupe" semantics in TryAddExpressionValue,
+    // only one entry surfaces. This is a known UX limitation worth pinning down with a test.


+        Action act = () => Assert.That(() => box.GetValueWithSideEffect() != box.GetValueWithSideEffect());
+
+        act.Should().Throw<AssertFailedException>()
+            .WithMessage(
+                """
+                Assert.That(() => box.GetValueWithSideEffect() != box.GetValueWithSideEffect()) failed.


Evangelink marked this pull request as ready for review May 18, 2026 07:05

Copilot started work on behalf of Evangelink May 18, 2026 07:06 View session

Copilot finished work on behalf of Evangelink May 18, 2026 07:14

Base automatically changed from dev/amauryleve/assert-that to main May 19, 2026 09:11

Copilot started work on behalf of Evangelink May 19, 2026 09:20 View session

Copilot finished work on behalf of Evangelink May 19, 2026 09:29

Copilot started work on behalf of Evangelink May 19, 2026 10:53 View session

Copilot finished work on behalf of Evangelink May 19, 2026 11:00

Copilot started work on behalf of Evangelink May 19, 2026 11:00 View session

Copilot finished work on behalf of Evangelink May 19, 2026 11:12

Copilot started work on behalf of Evangelink May 19, 2026 11:26 View session

Copilot finished work on behalf of Evangelink May 19, 2026 11:33

Evangelink force-pushed the assert-that+objects branch from 12c8549 to 10450c2 Compare May 19, 2026 13:38

Copilot AI review requested due to automatic review settings May 19, 2026 13:38

Copilot AI reviewed May 19, 2026

View reviewed changes

Copilot started reviewing on behalf of Evangelink May 19, 2026 14:02 View session

Evangelink force-pushed the assert-that+objects branch from 10450c2 to ff51120 Compare May 19, 2026 14:23

Evangelink merged commit 01f8d9b into main May 19, 2026
9 of 17 checks passed

Evangelink deleted the assert-that+objects branch May 19, 2026 14:36

Evangelink mentioned this pull request May 19, 2026

Assert.That: fix Expression.Assign double-evaluation and assorted cleanups #8363

Open

Conversation

Evangelink commented May 17, 2026

Merge notes

Validation

Uh oh!

Evangelink commented May 18, 2026

Uh oh!

Copilot AI commented May 18, 2026

Uh oh!

Evangelink commented May 18, 2026

Uh oh!

Evangelink commented May 19, 2026

Uh oh!

Copilot AI commented May 19, 2026

Uh oh!

Evangelink commented May 19, 2026

Uh oh!

Evangelink commented May 19, 2026

Uh oh!

Copilot AI commented May 19, 2026

Uh oh!

Copilot AI commented May 19, 2026

Uh oh!

Evangelink commented May 19, 2026

Uh oh!

Copilot AI commented May 19, 2026

Uh oh!

Evangelink commented May 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants