feat(mocks): shorter, more readable generated mock type names#5334
feat(mocks): shorter, more readable generated mock type names#5334
Conversation
IGreeter.Mock() now returns IGreeterMock instead of IGreeter_Mock. Namespaced types use sub-namespaces for disambiguation instead of flattening the namespace into the type name (e.g. ExternalServiceMock in TUnit.Mocks.Generated.ExternalLib instead of ExternalLib_ExternalService_Mock in TUnit.Mocks.Generated).
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 6 minor |
🟢 Metrics 13 complexity
Metric Results Complexity 13
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
Code Review: feat(mocks): shorter, more readable generated mock type names
This is a well-executed DX improvement. The naming changes make generated types feel much more like natural C# code.
What's Good
Naming strategy is correct. Moving from MyApp_IGreeter_Mock → IGreeterMock aligns with how users would naturally name these types. The sub-namespace approach for namespaced types (TUnit.Mocks.Generated.MyApp.IGreeterMock) preserves scoping semantics cleanly instead of the old namespace-flattening that polluted TUnit.Mocks.Generated with noise.
Extension class naming split is right. Using the full FQN-safe name for the static extension holder (MyApp_IGreeter_MockStaticExtension) while exposing the short name via the return type is the correct trade-off — the extension class lives in the global TUnit.Mocks namespace and must be unique across all mocked types, while the wrapper type lives in a scoped namespace.
Generic encoding is improved. Collapsing IRepository_string__Mock → IRepository_string_Mock by eliminating redundant underscores from adjacent brackets is a nice cleanup.
Issues / Concerns
1. Trailing underscore in generic type names (minor inconsistency)
For non-generic types: IGreeterMock (clean)
For generic types: IRepository_string_Mock (trailing _ before suffix)
This comes from GetShortSafeName not stripping trailing underscores after sanitization. The name IRepository<string> → sanitize brackets → IRepository_string_ → suffix appended → IRepository_string_Mock.
A simple fix would be to trim trailing underscores from the result in GetShortSafeName:
// After the while loop that collapses __
result = result.Trim('_');
return result;This would yield IRepository_string_Mock → IRepository_stringMock. Though the PR description seems to accept the current output, so this may be intentional.
2. Code duplication: GetShortSafeName and StripNamespaceFromFqn
Both methods do essentially the same sanitization (strip namespace, replace <>/,.[] with _, collapse __). The only difference is that GetShortSafeName uses model.Namespace to know what to strip, while StripNamespaceFromFqn does it by finding the last . outside angle brackets.
These could be unified into a single helper, reducing the maintenance surface:
private static string GetSanitizedTypeName(string fqn, string? knownNamespace = null)
{
// shared stripping + sanitization logic
}Not critical, but the duplication is a maintenance risk if the sanitization rules need to change (e.g., adding support for a new generic syntax).
3. Breaking change — documented but worth flagging
Any user who holds a generated mock in a field/variable with the explicit type name (e.g. IFoo_Mock myMock = ...) will need to update to IFooMock myMock = .... Since these types are in TUnit.Mocks.Generated and the .Mock() extension returns the concrete typed wrapper, most users should be shielded by var inference. Still worth a note in the release/changelog.
Summary
The design is sound and the implementation is clean. The snapshot tests cover the full range of cases (generics, events, static abstracts, multi-interface, partial/wrap). The two code concerns above are minor — the trailing-underscore issue in generic names is the only thing that creates a minor inconsistency in the public type names, but it may be an intentional stylistic choice. Overall this is a good improvement to the mocking DX.
…pers Deduplicate the sanitization logic that was copy-pasted between GetShortSafeName and StripNamespaceFromFqn. Also use string.Join instead of += in a loop for GetCompositeShortSafeName.
There was a problem hiding this comment.
Review: feat(mocks): shorter, more readable generated mock type names
The goal here is clearly a good one — stripping namespace prefixes from generated type names significantly improves the user-facing DX for typed fields/variables and error messages. The snapshot tests and integration tests passing gives good confidence the mechanical changes are correct. Below are a few architectural observations and concerns worth discussing before merge.
Name collision risk: the core concern
The old strategy encoded the full FQN into the type name (e.g. MyApp_IGreeter_Mock) within a single flat namespace, making collisions structurally impossible. The new strategy drops to short names and relies on sub-namespaces for disambiguation. This trades one guarantee for another and introduces a subtle collision surface.
Scenario 1 — Two types with the same simple name in different namespaces, same test assembly:
namespace Company.Billing { interface IService { ... } }
namespace Company.Shipping { interface IService { ... } }
Both would generate IServiceMock in TUnit.Mocks.Generated.Company.Billing and TUnit.Mocks.Generated.Company.Shipping respectively. That works fine. But the MockStaticExtensionBuilder intentionally keeps using GetCompositeSafeName for the extension class (noted with a comment), placing it in the flat TUnit.Mocks namespace — so the extension classes still avoid collision. That asymmetry is correct but worth documenting more prominently.
Scenario 2 — Type whose namespace IS a prefix of another type's namespace:
If someone has namespace Foo with IBar AND namespace Foo.IBar (an unusual but legal namespace that matches a type name), the sub-namespace strategy could theoretically route both to TUnit.Mocks.Generated.Foo.IBar. This is an extreme edge case, but the old strategy was immune to it by construction.
Scenario 3 — GetCompositeShortSafeName for multi-interface mocks still uses _ separator:
name += "_" + string.Join("_", model.AdditionalInterfaceNames.Select(StripNamespaceFromFqn));So a multi-interface mock like IFoo + IBar becomes IFoo_IBar (with a leading underscore separator). This is inconsistent with the single-type rename which removes all underscores (e.g. IGreeterMock not IGreeter_Mock). The multi-interface combined names would produce IFoo_IBarMultiMockFactory — mixing styles. Consider IFooIBarMultiMockFactory or introducing a different separator like And (IFooAndIBarMock).
SanitizeIdentifier uses a while loop that could be slow
while (result.Contains("__"))
result = result.Replace("__", "_");For most names this runs 0–1 iterations, so it's fine in practice. But for deeply nested generics like IDictionary<string, List<IDictionary<string, object>>> this could iterate several times. A single Regex.Replace(@"_{2,}", "_", result) or a StringBuilder-based approach would handle it in one pass. Minor, but worth noting for correctness at extremes.
Awkward generated namespace in StaticAbstractMemberTests.cs
using TUnit.Mocks.Generated.TUnit.Mocks.Tests;This is a direct consequence of GetMockNamespace appending model.Namespace verbatim:
$"TUnit.Mocks.Generated.{model.Namespace}"When the original namespace is TUnit.Mocks.Tests, the generated namespace becomes TUnit.Mocks.Generated.TUnit.Mocks.Tests, which is redundant and looks like a nesting mistake to a reader of the test file. A consumer encountering this for the first time would reasonably assume it's a bug.
Possible mitigations:
- Strip common
TUnit.Mocks.*prefixes (project-specific, fragile). - Use only the last segment of the namespace rather than the full namespace (e.g.
TUnit.Mocks.Generated.Testsinstead ofTUnit.Mocks.Generated.TUnit.Mocks.Tests). This reintroduces some collision risk but is much more readable. - Keep the full sub-namespace but accept it as a necessary evil and document it.
The current behaviour is a regression in readability specifically for types that live in namespaces beginning with TUnit.Mocks.*, which is exactly where the test utilities live.
GetShortSafeName uses string prefix matching, not proper namespace segmentation
if (!IsGlobalNamespace(model.Namespace) && name.StartsWith(model.Namespace + "."))
name = name.Substring(model.Namespace.Length + 1);This is correct for the normal case but would silently produce an unexpected result if model.FullyQualifiedName (after stripping global::) doesn't actually start with model.Namespace + "." (e.g. if the model is constructed in an unusual way). A debug assertion or a guard here would help catch such issues during development.
Positive aspects
- The public
XxxMockwrapper type in a sub-namespace is a real UX win — typed variables in test code becomeIGreeterMock mock = IGreeter.Mock()which is idiomatic and auto-complete-friendly. - The
SanitizeIdentifierhelper is a clean extraction that consolidates what was previously scattered replace chains. - Keeping the extension class name FQN-based (in
MockStaticExtensionBuilder) shows good awareness of the global-uniqueness constraint forTUnit.Mocks-namespace types. - Snapshot tests cover the changed output correctly and the
__→_collapse for generics is a nice polish.
Summary
The direction is right. The two things I'd recommend addressing before merge are:
- The
TUnit.Mocks.Generated.TUnit.Mocks.Testsnamespace — it's the most visible readability regression, visible directly in user-facing test files, and directly contradicts the stated goal of the PR. - The mixed separator style in multi-interface names (
IFoo_IBarMockvsIFooMock) — either go underscore-free everywhere or be consistent about when underscores appear.
The collision concern is real but manageable; the existing test coverage mitigates the most common cases.
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.24.13 to 1.24.18. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.24.18 <!-- Release notes generated using configuration in .github/release.yml at v1.24.18 --> ## What's Changed ### Other Changes * feat(mocks): shorter, more readable generated mock type names by @thomhurst in thomhurst/TUnit#5334 * Fix DisposeAsync() ordering for nested property injection by @Copilot in thomhurst/TUnit#5337 ### Dependencies * chore(deps): update tunit to 1.24.13 by @thomhurst in thomhurst/TUnit#5331 **Full Changelog**: thomhurst/TUnit@v1.24.13...v1.24.18 Commits viewable in [compare view](thomhurst/TUnit@v1.24.13...v1.24.18). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
IGreeter.Mock()now returnsIGreeterMockinstead ofIGreeter_Mock— cleaner for typed field/variable declarationsExternalServiceMockinTUnit.Mocks.Generated.ExternalLibinstead ofExternalLib_ExternalService_MockinTUnit.Mocks.Generated)IRepository_string_Mockinstead ofIRepository_string__Mock(collapsed double underscores)Naming changes
MyApp_IGreeter_MockIGreeterMockMyApp_IGreeter_MockImplIGreeterMockImplMyApp_IGreeter_MockFactoryIGreeterMockFactoryMyApp_IFoo_MockableIFooMockableIRepo_string__MockIRepo_string_MockNamespace strategy
TUnit.Mocks.Generated(unchanged)TUnit.Mocks.Generated.{OriginalNamespace}TUnit.Mocks.Generatedwith full FQN-safe names (not user-facing).Mock()extension class → stays inTUnit.Mockswith full FQN-safe name (not user-facing)Test plan
GetCompositeShortSafeName