fix(mocks): WasCalled/WasNeverCalled assertions via [GenerateAssertion] for all ICallVerification types#6176
Conversation
…eAssertion] Replace hand-written WasCalledAssertion/WasNeverCalledAssertion classes and IAssertionSource extension overloads with generic [GenerateAssertion] methods constrained to ICallVerification. Fixes Assert.That(mock.Method(Any())).WasCalled(...) failing to compile for generated mock call types (MockMethodCall<T>, PropertyMockCall<T> struct, VoidMockMethodCall). Add MockAssertionExtensionsTests: 32 positive/negative tests covering the assertion pipeline across all Times variants, arg matching, void methods, property getters/setters, and null verification targets.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 32 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
The core refactoring is the right approach — replacing hand-written WasCalledAssertion/WasNeverCalledAssertion classes with [GenerateAssertion] generic methods is exactly what this pattern was designed for, and the fix for struct types (PropertyMockCall<T>) is genuine. Test coverage is thorough.
Two issues to address:
Bug: Inner exception is silently dropped
MockAssertionExtensions.cs — WasCalled<T> and WasNeverCalled<T>
// New code:
catch (MockVerificationException ex)
{
return AssertionResult.Failed(ex.Message); // inner exception lost
}The deleted WasCalledAssertion was:
return Task.FromResult(AssertionResult.Failed(ex.Message, ex)); // passed exAssertionResult.Failed(string, Exception?) exists (I checked). Dropping ex means anyone inspecting the assertion failure's inner exception (e.g., a debugger, a diagnostics tool, TUnit's own failure reporting) gets null instead of the real exception. This is a silent regression. Both catch blocks in the new code need ex threaded through:
catch (MockVerificationException ex)
{
return AssertionResult.Failed(ex.Message, ex);
}Design issue: IGreeter declared inside VerificationTests shadows the top-level one
VerificationTests.cs lines 253–256
There is already a top-level public interface IGreeter in BasicMockTests.cs (same TUnit.Mocks.Tests namespace). The PR adds a second, identically-shaped nested IGreeter inside the VerificationTests class.
Inside VerificationTests, all unqualified IGreeter references — including the pre-existing Verify_String_Method_On_Different_Interface test — will silently resolve to the new nested type rather than the top-level one. The code compiles and works because the signatures are identical, but this is a shadowing trap: if either definition ever diverges, the test will start testing the wrong type without a compile error.
Better approach: reuse the existing top-level IGreeter; it has the same Greet(string name) signature and is already used by other tests in the file via BasicMockTests.IGreeter. No new type is needed.
Minor: WasCalled() shorthand expectation message is vague
[GenerateAssertion(ExpectationMessage = "to have been called")]
public static AssertionResult WasCalled<T>(T verification) where T : ICallVerificationThis assertion checks Times.AtLeastOnce, so when it fails the error reads: "Expected X to have been called but was called 0 time(s)". "to have been called at least once" would be more precise and match the other Times-based messages in style.
Good things worth noting
- Eliminating
WasCalledAssertion/WasNeverCalledAssertionis the right simplification — the source generator handles all that boilerplate. - Using a generic constraint
where T : ICallVerificationinstead ofIAssertionSource<ICallVerification>is the correct fix for the struct types. - The 32-test suite covers boundary cases, all
Timesvariants, and all mock call types comprehensively. WasCalled(Times)andWasNeverCalledare correctly kept as extension methods so the non-assertion formmock.Add(1, 2).WasCalled(Times.Once)continues to work.
- Preserve MockVerificationException as inner exception in AssertionResult.Failed - Remove nested IGreeter in VerificationTests that shadowed the top-level interface - Clarify WasCalled() expectation message to "to have been called at least once"
|
Addressed all three review items in 4df31cc:
Ran |
There was a problem hiding this comment.
Follow-up Review — Previous items all addressed
All three items from the first review round have been correctly fixed in 4df31cc:
- Both
catchblocks now passextoAssertionResult.Failed(ex.Message, ex)— inner exception preserved. - The nested
IGreetershadow has been removed —VerificationTestsandMockAssertionExtensionsTestsboth use the top-levelIGreeterfromBasicMockTests.cs. WasCalled()shorthand expectation message is now"to have been called at least once".
One minor cleanup remaining
MockAssertionExtensions.cs still has:
using System.Runtime.CompilerServices;This was needed for [CallerArgumentExpression] on the old WasCalled(Times, string?) overloads. Those overloads were deleted in this PR and CallerArgumentExpression is no longer referenced anywhere in the file. The import is now unused and should be removed.
Everything else looks good
- The
[GenerateAssertion]approach correctly handles thewhere T : ICallVerificationconstraint, fixing the struct type (PropertyMockCall<T>) limitation. WasCalled(Times)andWasNeverCalled()are correctly preserved as plain extension methods so the non-assertion formmock.Add(1, 2).WasCalled(Times.Once)continues to work without going through the assertion pipeline.IPropertyHolderbeing nested insideMockAssertionExtensionsTestsis fine — it's a test-only interface specific to that class.- The 32-test suite is comprehensive and well-structured.
The stale import is the only remaining item. Happy to approve once that's cleaned up, or it can be left for a follow-up — it's low severity.
Summary
WasCalledAssertion/WasNeverCalledAssertionclasses andIAssertionSource<T>extension overloads with generic[GenerateAssertion]methods constrained toICallVerificationawait Assert.That(mock.Method(Any())).WasCalled(Times.Once)not compiling for generated mock call types (MockMethodCall<T>,PropertyMockCall<T>struct,VoidMockMethodCall,PropertySetterMockCall<T>)WasCalled(Times)andWasNeverCalledremain direct extension methods so the non-assertion formmock.Add(1, 2).WasCalled(Times.Once)keeps workingTest coverage
New
MockAssertionExtensionsTests(32 tests, all through the assertion pipeline):WasCalled()shorthand, allTimesvariants incl. boundaries (Once/Exactly/AtLeast/AtMost/Between/Never),WasNeverCalled(), arg-specific counting,Any()matcher, void methods, property getters (structPropertyMockCall<T>— exercises the generic constraint path), property setters via.Setterand.Set(value)Timesvariant with wrong count assertsAssertionExceptionwithMock verification failed/called N time(s)message,WasNeverCalledafter call, non-matching args, nullICallVerificationtargetAlso cleaned up
VerificationTests.Greet_Returns_Configured_Value(stale 'does not compile' comment removed; result now asserted).Test results
MockAssertionExtensionsTests: 32/32 pass (net10.0)VerificationTests: 15/15 pass (net10.0)