Skip to content

fix: drain pending WhichNode in chained ForWhich calls#955

Merged
vbreuss merged 2 commits into
mainfrom
fix/forwhich-loses-pending-whichnode
May 17, 2026
Merged

fix: drain pending WhichNode in chained ForWhich calls#955
vbreuss merged 2 commits into
mainfrom
fix/forwhich-loses-pending-whichnode

Conversation

@vbreuss
Copy link
Copy Markdown
Member

@vbreuss vbreuss commented May 17, 2026

A second call to ExpectationBuilder.ForWhich overwrote the pending _whichNode field, silently dropping the previous projection's parent constraints. Drain the pending node into the graph before creating the new one in both the sync and async overloads (GetRootNode still performs the final drain when no further ForWhich is called).

A second call to `ExpectationBuilder.ForWhich` overwrote the pending `_whichNode` field, silently dropping the previous projection's parent constraints. Drain the pending node into the graph before creating the new one in both the sync and async overloads (`GetRootNode` still performs the final drain when no further `ForWhich` is called).
@vbreuss vbreuss self-assigned this May 17, 2026
Copilot AI review requested due to automatic review settings May 17, 2026 04:05
@vbreuss vbreuss added the bug Something isn't working label May 17, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a graph-building bug in ExpectationBuilder.ForWhich where chaining multiple ForWhich calls could overwrite a pending WhichNode, causing earlier projection parent constraints to be silently dropped.

Changes:

  • Drain any pending _whichNode at the start of both sync and async ForWhich overloads before creating a new WhichNode.
  • Add regression tests covering chained ForWhich behavior (ordering and honoring constraints across levels).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Tests/aweXpect.Core.Tests/Core/ExpectationBuilderTests.cs Adds regression tests to validate chained ForWhich projections and expectation text ordering.
Source/aweXpect.Core/Core/ExpectationBuilder.cs Ensures pending WhichNode is drained into the node graph before starting another ForWhich chain (sync + async).

Comment thread Source/aweXpect.Core/Core/ExpectationBuilder.cs
Comment thread Source/aweXpect.Core/Core/ExpectationBuilder.cs
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

Test Results

     23 files   - 27       23 suites   - 27   7m 51s ⏱️ +42s
 19 809 tests  - 22   19 808 ✅  - 22  1 💤 ±0  0 ❌ ±0 
102 404 runs   - 57  102 403 ✅  - 57  1 💤 ±0  0 ❌ ±0 

Results for commit 3825a78. ± Comparison against base commit 891b1b7.

This pull request removes 3189 and adds 3167 tests. Note that renamed tests count towards both.
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message3a41dcdb-1f90-4e8f-8bc8-bef350532f8c")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message3e6ac730-f17e-4ad8-a08b-d3e4f0424e7f")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message7c740528-737a-4db8-851c-a54cc4fca28f")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message96ec101f-51fe-4f52-bcb7-bc1cbd6b1604")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "messaged4823deb-4682-4bd4-87e7-d2908cd468c8")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "messagee8ca14fb-6792-477e-9ef1-f008a884d8e3")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "message28fba540-c065-4498-b7ca-ef7741868b5e")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "message4942e092-3e9f-4a82-b57c-bce2561187f3")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "messagea4d21278-09db-44d9-98b1-ea079aaad753")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "messagea7c4c349-d16f-439b-956e-6a3ca1b332d7")
…
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message068d0e8e-832f-4aea-88b0-869e5892efdc")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message1ce99e5f-1264-4361-b735-ca780b7ed571")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message2b544fd3-5ec9-4d99-83d9-b63fe3e0ed7c")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "message9dfa69bb-abce-4001-ac3e-8c24aa256b9e")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "messagecfb392d8-3d92-4497-8c76-2bcd1b7b097d")
aweXpect.Core.Tests.Core.Exceptions.FailExceptionTests ‑ Message_ShouldBeSet(message: "messageec41a521-449c-401a-a3df-bdc3fce4b131")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "message0d23b8b2-7f12-4444-b09a-6e0f71cf714f")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "message972a158c-7446-4b32-9d2c-7e78b8d00a12")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "message97d0f2d7-5a16-42fe-8291-94f172a295aa")
aweXpect.Core.Tests.Core.Exceptions.SkipExceptionTests ‑ Message_ShouldBeSet(message: "messagee290db36-998e-44ba-9595-d21d38249cf8")
…

♻️ This comment has been updated with latest results.

The previous regression tests only exercised the sync `ForWhich`
overload. Add an analogous test that uses two chained async `ForWhich`
calls so the drain logic in the async overload is also covered.
@vbreuss vbreuss force-pushed the fix/forwhich-loses-pending-whichnode branch from 55c65a6 to 3825a78 Compare May 17, 2026 04:15
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

🚀 Benchmark Results

Details

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
AMD EPYC 7763 2.45GHz, 1 CPU, 4 logical and 2 physical cores
.NET SDK 10.0.300
[Host] : .NET 8.0.27 (8.0.27, 8.0.2726.22922), X64 RyuJIT x86-64-v3

Job=InProcess Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=1 WarmupCount=10

Method Mean Error StdDev Gen0 Gen1 Allocated
Bool_aweXpect 262.7 ns 2.49 ns 2.20 ns 0.0415 - 696 B
Bool_FluentAssertions 247.3 ns 4.58 ns 4.29 ns 0.0567 - 952 B
Equivalency_aweXpect 312,035.5 ns 1,497.76 ns 1,401.01 ns 20.0195 0.4883 335444 B
Equivalency_FluentAssertions 2,629,412.5 ns 12,535.34 ns 11,725.57 ns 285.1563 46.8750 4804906 B
Int_GreaterThan_aweXpect 259.7 ns 4.56 ns 4.26 ns 0.0515 - 864 B
Int_GreaterThan_FluentAssertions 267.4 ns 1.54 ns 1.20 ns 0.0730 - 1224 B
ItemsCount_AtLeast_aweXpect 466.7 ns 2.14 ns 1.89 ns 0.0811 - 1360 B
ItemsCount_AtLeast_FluentAssertions 457.9 ns 8.69 ns 8.13 ns 0.1197 - 2008 B
String_aweXpect 457.2 ns 2.02 ns 1.89 ns 0.0672 - 1128 B
String_FluentAssertions 1,226.3 ns 9.25 ns 7.22 ns 0.2346 - 3944 B
StringArray_aweXpect 1,930.8 ns 7.50 ns 6.27 ns 0.1564 - 2624 B
StringArray_FluentAssertions 1,325.2 ns 10.37 ns 9.70 ns 0.2480 - 4152 B
StringArrayInAnyOrder_aweXpect 2,537.9 ns 10.61 ns 9.40 ns 0.1678 - 2816 B
StringArrayInAnyOrder_FluentAssertions 87,679.3 ns 396.41 ns 331.02 ns 3.4180 - 57481 B

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 17, 2026

👽 Mutation Results

Mutation testing badge

aweXpect

Details
File Score Killed Survived Timeout No Coverage Ignored Compile Errors Total Detected Total Undetected Total Mutants

The final mutation score is NaN%

Coverage Thresholds: high:80 low:60 break:0

aweXpect.Core

Details
File Score Killed Survived Timeout No Coverage Ignored Compile Errors Total Detected Total Undetected Total Mutants
Core/ExpectationBuilder.cs 88.30% 83 9 0 2 41 19 83 11 154

The final mutation score is 88.30%

Coverage Thresholds: high:80 low:60 break:0

@sonarqubecloud
Copy link
Copy Markdown

@vbreuss vbreuss enabled auto-merge (squash) May 17, 2026 04:25
@vbreuss vbreuss merged commit 17f0b7e into main May 17, 2026
13 checks passed
@vbreuss vbreuss deleted the fix/forwhich-loses-pending-whichnode branch May 17, 2026 04:28
github-actions Bot added a commit that referenced this pull request May 17, 2026
…ed `ForWhich` calls (#955) by Valentin Breuß
github-actions Bot added a commit that referenced this pull request May 17, 2026
…ed `ForWhich` calls (#955) by Valentin Breuß
vbreuss added a commit that referenced this pull request May 17, 2026
`ExpectationBuilder.ForWhich` (since #955) drains a pending `_whichNode`
into the graph before creating a new one, so two consecutive `ForWhich`
calls produce two sibling `WhichNode`s. At evaluation time both nodes
received the original outer subject — including the inner one, whose
`TSource` is the *outer* node's `TMember`. That made any chained
`ForWhich` whose accessor consumes the previous projection (e.g.
`Which.X.Which.Y` or `Which.X.And.WhoseParent`) throw "The member type
for the actual value in the which node did not match".

`WhichNode<TSource, TMember>.IsMetBy` now, when the outer value is not
itself a `TSource`, asks the parent's `ConstraintResult.TryGetValue`
for a `TSource`. `WhichConstraintResult.TryGetValue` already exposes
the projection produced by the previous `WhichNode`, so the chained
accessor consumes that value instead of the unrelated outer subject.
The original type-mismatch error is still thrown when neither the
outer value nor the parent chain can supply a `TSource`.

Adds coverage in `WhichNodeTests` (direct, two- and three-level chains
plus a regression test for the fallback path) and in
`ExpectationBuilderTests` (`ForWhich` and `ForWhich`-async called
twice where the second projection consumes the first's result, plus a
three-level integration variant).
@github-actions
Copy link
Copy Markdown
Contributor

This is addressed in release v2.34.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working state: released The issue is released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants