Skip to content

refactor(mocks): clean up Mock API surface#5314

Merged
thomhurst merged 1 commit intomainfrom
refactor/mock-api-surface-cleanup
Mar 30, 2026
Merged

refactor(mocks): clean up Mock API surface#5314
thomhurst merged 1 commit intomainfrom
refactor/mock-api-surface-cleanup

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Move 6 framework-internal methods (RegisterFactory, RegisterMultiFactory, RegisterDelegateFactory, RegisterWrapFactory, TryCreateAutoMock, GetEngine) from Mock to a new [EditorBrowsable(Never)] MockRegistry class — completely removes them from Mock. autocomplete even in project-reference scenarios
  • Move 11 static helper methods that take Mock<T> as first parameter to instance members on Mock<T> (e.g. Mock.GetInvocations(mock)mock.Invocations, Mock.Reset(mock)mock.Reset())
  • Mock. autocomplete now shows only 5 entries: Of, OfDelegate, Wrap, Get, VerifyInOrder

Before → After

Before (static) After (instance)
Mock.GetInvocations(mock) mock.Invocations
Mock.GetBehavior(mock) mock.Behavior
Mock.Get/SetDefaultValueProvider(mock, ...) mock.DefaultValueProvider
Mock.SetupAllProperties(mock) mock.SetupAllProperties()
Mock.Reset(mock) mock.Reset()
Mock.VerifyAll(mock) mock.VerifyAll()
Mock.VerifyNoOtherCalls(mock) mock.VerifyNoOtherCalls()
Mock.GetDiagnostics(mock) mock.GetDiagnostics()
Mock.SetState(mock, s) mock.SetState(s)
Mock.InState(mock, s, a) mock.InState(s, a)

Test plan

  • All 679 TUnit.Mocks.Tests pass
  • All 22 source generator snapshot tests pass (snapshots updated)
  • All Roslyn variant generators build successfully
  • Documentation updated (advanced.md, verification.md, setup.md)

… to MockRegistry and instance helpers to Mock<T>

Move 6 framework-internal methods (RegisterFactory, RegisterMultiFactory, RegisterDelegateFactory,
RegisterWrapFactory, TryCreateAutoMock, GetEngine) to a new [EditorBrowsable(Never)] MockRegistry class.
Move 11 static helpers that take Mock<T> as first parameter to instance members on Mock<T>
(Invocations, Behavior, DefaultValueProvider, SetupAllProperties, Reset, VerifyAll,
VerifyNoOtherCalls, GetDiagnostics, SetState, InState).

Mock. autocomplete now shows only 5 entries: Of, OfDelegate, Wrap, Get, VerifyInOrder.
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review: Mock API Surface Refactor

This is a well-executed refactor that meaningfully improves the ergonomics of the mocking API. The changes are consistently applied across the source generator, tests, and documentation.

What works well

MockRegistry extraction — Moving the 6 framework-internal methods into a dedicated [EditorBrowsable(Never)] class is the right call. The previous approach of hiding generated-code-only members on the user-facing Mock static class was a leaky abstraction. Now Mock. autocomplete genuinely reflects the user-level surface, and MockRegistry acts as a clean internal seam between the generator and the runtime.

Promoting static helpers to instance membersmock.Reset(), mock.Invocations, mock.VerifyAll(), etc. are dramatically more idiomatic. The old Mock.Reset(mock) pattern was essentially simulating instance methods with a static helper, which IDE tooling doesn't optimally surface. The new API is what users will naturally reach for.

IMock delegation kept intact — The explicit interface implementations (void IMock.VerifyAll() => VerifyAll()) correctly preserve any code that goes through the IMock abstraction (e.g. MockRepository), with no behaviour change.

Source generator consistency — Every snapshot file is consistently updated to MockRegistry.RegisterFactory/MockRegistry.GetEngine. No half-migrated call sites remain.

InState thread safety — Saving/restoring _engine.PendingRequiredState in a try/finally is the right pattern for the ambient-context style of state scoping.

Observations and minor suggestions

MockRegistry is public but fully [EditorBrowsable(Never)] — This is the conventional pattern for generated-code entrypoints, but the class itself is still fully public, meaning it shows up in docs generators (DocFX, etc.) and is technically part of the public API contract for binary compatibility. If the intention is truly "internal to the generator", internal visibility would be a stronger guarantee. Since generated code lives in the same assembly it compiles into, internal would work fine and removes the need for the attribute entirely. If public is intentional (e.g. to support external generator extensions), the current approach is correct — just noting the trade-off.

GetEngine remains public on MockRegistryMockRegistry.GetEngine<T>(Mock<T>) is marked [EditorBrowsable(Never)] but is public, because the source-generated extension methods call it. This is the same situation as above. The 4 internal TryGet* helpers that were extracted back to MockRegistry are correctly internal, which is good.

Behavior as a read-only property vs MockBehaviormock.Behavior (new) replaces Mock.GetBehavior(mock) (old). There's no SetBehavior equivalent, which is intentional given behavior is set at construction time. The asymmetry is fine, but worth a quick doc comment note that behavior is construction-time-only (the current summary just says "Returns the mock behavior").

DefaultValueProvider as a read-write property — This is a clean improvement over the old separate GetDefaultValueProvider/SetDefaultValueProvider pair. No issues here.

InState captures this via the Action<Mock<T>> configure delegate — This is correct and necessary. One thing worth considering: if configure throws, PendingRequiredState is correctly restored via finally, which is good. The behaviour is consistent with the existing static version.

Breaking change — Existing user code calling Mock.GetInvocations(mock), Mock.Reset(mock), etc. will fail to compile after upgrading. This is expected and clearly documented in the PR description table. If TUnit.Mocks has not reached a stable 1.0, this is fine. If it has, a migration note in the changelog and a brief deprecation period (keeping the old statics as [Obsolete] for one minor version) would soften the upgrade path for existing consumers.

No [Obsolete] forwarding stubs — Related to the point above: since the old statics are fully removed rather than [Obsolete]-bridged, any downstream code calling the old API will get a hard compile error with no migration hint. Given this appears to be an actively evolving library (not a stable 1.0 release), this is probably fine — but something to consider if the library is already published on NuGet.

Summary

Clean, well-scoped refactor. The separation of MockRegistry (generator-internal) from Mock (user-facing) is architecturally sound, and moving helper methods to instance members on Mock<T> is the natural evolution of the API. All snapshot tests, source generator builders, test files, and documentation are consistently updated. The main consideration is whether to provide a one-version [Obsolete] bridge for existing users before doing the hard removal.

@thomhurst thomhurst enabled auto-merge (squash) March 30, 2026 18:26
@thomhurst thomhurst merged commit b1dd2a6 into main Mar 30, 2026
15 checks passed
@thomhurst thomhurst deleted the refactor/mock-api-surface-cleanup branch March 30, 2026 19:04
This was referenced Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant