feat: split TUnit.AspNetCore into Core + meta package#5474
Conversation
TUnit.AspNetCore depended on TUnit (the meta package with engine/runner), which meant library authors couldn't reference it without their project being treated as a test project. Create TUnit.AspNetCore.Core containing all types, depending on TUnit.Core instead of TUnit. This allows library authors to reference TUnit.AspNetCore.Core without pulling in the test engine. TUnit.AspNetCore remains as a meta package for test projects, depending on TUnit + TUnit.AspNetCore.Core.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
PR Review: feat: split TUnit.AspNetCore into Core + meta package
Summary
The motivation here is solid: TUnit.AspNetCore previously depended on TUnit (the meta-package including the engine/runner), which poisoned any library wanting to provide shared ASP.NET Core test infrastructure — NuGet would flag it as a test project. The architecture correctly mirrors the established TUnit.Core (lightweight) vs TUnit (meta with engine) split, which is a consistent and well-understood pattern in this codebase.
Issues and Concerns
1. InternalsVisibleTo on TUnit.Engine grants access to a non-engine project (potential dead grant)
TUnit.Engine now grants InternalsVisibleTo to TUnit.AspNetCore.Core, but TUnit.AspNetCore.Core depends only on TUnit.Core and TUnit.Logging.Microsoft. None of the 17 moved source files appear to import or consume TUnit.Engine internals. This grant was presumably inherited from the old TUnit.AspNetCore which did transitively pull in the engine via the TUnit meta-package.
Suggested action: Audit whether anything in TUnit.AspNetCore.Core actually uses engine internals:
grep -r "TUnit\.Engine\." TUnit.AspNetCore.Core/If nothing does, the InternalsVisibleTo in TUnit.Engine.csproj should be reverted. Unnecessary InternalsVisibleTo grants increase coupling and reduce the clarity of the module boundary.
2. Analyzer delivery to meta-package consumers — worth explicit verification
TUnit.AspNetCore.Core packs all three Roslyn variants of TUnit.AspNetCore.Analyzers into its analyzers/dotnet/... paths. The meta-package TUnit.AspNetCore dropped its direct TUnit.AspNetCore.Analyzers project reference entirely. NuGet should propagate analyzer assets from transitive PackageReference dependencies — but if the TUnit.AspNetCore.NugetTester.csproj only references TUnit.AspNetCore and doesn't validate that analyzer diagnostics actually fire, a regression in this delivery path might not be caught by CI.
Suggested action: Verify the nightly NuGet tester exercises the analyzer-from-meta-package path, and consider adding a test variant that references TUnit.AspNetCore.Core directly (the library-author use case) to confirm analyzers work in that scenario too.
3. VersionOverride pattern could use a comment
The existing pattern (copied verbatim from the old project) is somewhat non-obvious:
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="10.0.5" VersionOverride="8.0.0" />Having both Version and VersionOverride on the same element is unusual. The intent is "use major-aligned versions per TFM even though CPM pins to 10.0.5". This is pre-existing but now lives in a new, more prominent project. A short comment would help future maintainers understand this is intentional.
What Works Well
- The architecture cleanly mirrors the
TUnit.Core/TUnitsplit — this is the right model. InternalsVisibleTois updated in all three affected projects (TUnit.Core,TUnit.Engine,TUnit.Logging.Microsoft).- All three solution files (
TUnit.Dev.slnx,TUnit.CI.slnx,TUnit.slnx) are updated consistently. - The pipeline
GetPackageProjectsModuleis updated soTUnit.AspNetCore.Coregets packed and published. RootNamespaceis overridden toTUnit.AspNetCoreinTUnit.AspNetCore.Core— this preserves backward source compatibility so existing code won't need namespace changes.- The
net8.0/9.0/10.0TFM override (nonetstandard2.0) is correct givenMicrosoft.AspNetCore.Mvc.Testingconstraints, and the comment explains why.
Summary
This is a well-structured change with a clear, valid motivation. The main items worth following up on: confirming the TUnit.Engine InternalsVisibleTo grant is actually needed, and ensuring CI coverage for the analyzer delivery path through the meta-package. Neither is a blocker, but the engine internals grant in particular is worth cleaning up if it's genuinely unused — unnecessary coupling across module boundaries tends to grow over time.
There was a problem hiding this comment.
Follow-up Review (second commit: fix: update public API snapshots)
What the second commit addresses
The snapshot files in TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.*.verified.txt correctly reflect the InternalsVisibleTo rename from TUnit.AspNetCore → TUnit.AspNetCore.Core in TUnit.Core.csproj. Committing .verified.txt files is the right approach per project conventions (CLAUDE.md) — this is clean and correct.
Status of issues raised in the initial review
The three suggestions from the first review remain open (the second commit does not address them):
-
TUnit.EngineInternalsVisibleTo grant — Still worth confirming with:grep -r "TUnit\.Engine\." TUnit.AspNetCore.Core/If nothing in the new project touches engine internals, the grant is dead weight.
-
Analyzer delivery through the meta-package — No new test coverage added for the library-author path (referencing
TUnit.AspNetCore.Coredirectly). Still a nice-to-have follow-up. -
VersionOverridecomment — Still absent. Low priority, but the pattern is non-obvious to new contributors.
Overall
The second commit is a correct, necessary fix — snapshot tests would have blocked the build without it. The PR as a whole is in good shape. The remaining items above are non-blocking suggestions, not regressions.
The split moved WebApplicationTest (with its [Before(HookType.Test)] and [After(HookType.Test)] hooks) from TUnit.AspNetCore to TUnit.AspNetCore.Core, but the source generator reference was not carried over. Without the source generator, the hook registration code for InitializeFactoryAsync and DisposeFactoryAsync was never generated into the TUnit.AspNetCore.Core assembly. This caused the Factory property to remain uninitialized because the BeforeTest hook never ran.
There was a problem hiding this comment.
Review — Third Pass (all 3 commits)
What's new since last review
The third commit — "fix: add source generator to TUnit.AspNetCore.Core for hook registration" — is the most important addition and a critical correctness fix. Moving (which carries and attributes) to without also bringing the source generator would have silently broken hook execution: the Factory property would remain null at test time because InitializeFactoryAsync / DisposeFactoryAsync were never wired up. Good catch.
Status of items from previous reviews
1. TUnit.Engine InternalsVisibleTo grant — still open
TUnit.Engine.csproj now grants InternalsVisibleTo to TUnit.AspNetCore.Core, but TUnit.AspNetCore.Core.csproj only references TUnit.Core and TUnit.Logging.Microsoft — it has no reference to TUnit.Engine. A grant lets the named assembly see internals of the granting assembly, but only if the named assembly actually loads the granting one. Since TUnit.AspNetCore.Core never loads TUnit.Engine, this grant is dead. It won't break anything, but it's misleading — a future reader will wonder what engine internals the core package uses. Worth removing.
2. VersionOverride comment — partially addressed
The XML comment <!-- Framework-specific package versions for Microsoft.AspNetCore.Mvc.Testing --> explains the multi-ItemGroup structure, but the non-obvious part is still unexplained: why Version="10.0.5" VersionOverride="8.0.0" appear together on the same element when targeting net8.0. That combination is the Central Package Management (CPM) idiom — Version is the CPM-pinned global version, VersionOverride overrides it per-TFM. A single line like:
<!-- VersionOverride: CPM pins 10.0.5 globally; override to match each target framework -->would make this self-documenting. Minor; not blocking.
3. Analyzer delivery — resolved by NuGet PackageReference semantics
My earlier concern about analyzers not flowing from TUnit.AspNetCore.Core to consumers of TUnit.AspNetCore is moot in modern SDK-style projects: with PackageReference, analyzers from the full transitive package closure are applied automatically (NuGet 4.0+). The pattern exactly mirrors how TUnit.Core carries TUnit.Analyzers while TUnit (meta) does not re-pack them. Architecture is correct.
Overall
The split is well-executed. The core problem (library authors pulling in the test runner) is cleanly solved by mirroring the established TUnit.Core/TUnit pattern. All three solution files are updated, the pipeline registers the new package, RootNamespace is preserved for source compatibility, and the snapshot files are correct. The hooks regression introduced in commit 1 was caught and fixed in commit 3.
The one remaining actionable item: remove the dead InternalsVisibleTo Include="TUnit.AspNetCore.Core" from TUnit.Engine/TUnit.Engine.csproj. Everything else is non-blocking.
There was a problem hiding this comment.
Review — Third Pass (all 3 commits)
What's new since last review
The third commit — "fix: add source generator to TUnit.AspNetCore.Core for hook registration" — is the most important addition and a critical correctness fix. Moving WebApplicationTest.cs (which carries [Before(HookType.Test)] and [After(HookType.Test)] attributes) to TUnit.AspNetCore.Core without also bringing the source generator would have silently broken hook execution: the Factory property would remain null at test time because InitializeFactoryAsync / DisposeFactoryAsync were never wired up. Good catch.
Status of items from previous reviews
1. TUnit.Engine InternalsVisibleTo grant — still open
TUnit.Engine.csproj now grants InternalsVisibleTo to TUnit.AspNetCore.Core, but TUnit.AspNetCore.Core.csproj only references TUnit.Core and TUnit.Logging.Microsoft — it has no reference to TUnit.Engine. A grant lets the named assembly see internals of the granting assembly, but only if the named assembly actually loads the granting one. Since TUnit.AspNetCore.Core never loads TUnit.Engine, this grant is dead. It won't break anything, but it's misleading — a future reader will wonder what engine internals the core package uses. Worth removing.
2. VersionOverride comment — partially addressed
The XML comment <!-- Framework-specific package versions for Microsoft.AspNetCore.Mvc.Testing --> explains the multi-ItemGroup structure, but the non-obvious part is still unexplained: why Version="10.0.5" VersionOverride="8.0.0" appear together on the same element when targeting net8.0. That combination is the Central Package Management (CPM) idiom — Version is the CPM-pinned global version, VersionOverride overrides it per-TFM. A single line like:
<!-- VersionOverride: CPM pins 10.0.5 globally; per-TFM override to match each target framework -->would make this self-documenting. Minor; not blocking.
3. Analyzer delivery — resolved by NuGet PackageReference semantics
My earlier concern about analyzers not flowing from TUnit.AspNetCore.Core to consumers of TUnit.AspNetCore is moot in modern SDK-style projects: with PackageReference, analyzers from the full transitive package closure are applied automatically (NuGet 4.0+). The pattern exactly mirrors how TUnit.Core carries TUnit.Analyzers while TUnit (meta) does not re-pack them. Architecture is correct.
Overall
The split is well-executed. The core problem (library authors pulling in the test runner) is cleanly solved by mirroring the established TUnit.Core/TUnit pattern. All three solution files are updated, the pipeline registers the new package, RootNamespace is preserved for source compatibility, and the snapshot files are correct. The hooks regression introduced in commit 1 was caught and fixed in commit 3.
The one remaining actionable item: remove the dead InternalsVisibleTo Include="TUnit.AspNetCore.Core" from TUnit.Engine/TUnit.Engine.csproj. Everything else is non-blocking.
TUnit.AspNetCore.Core never references TUnit.Engine, so this grant was dead code and misleading.
There was a problem hiding this comment.
Review — Fourth Pass (all 4 commits)
What's new since last review
The fourth commit — "fix: remove unused InternalsVisibleTo for TUnit.AspNetCore.Core" — directly addresses the one remaining actionable item from previous reviews: the dead InternalsVisibleTo Include="TUnit.AspNetCore.Core" grant in TUnit.Engine/TUnit.Engine.csproj.
Looking at the current cumulative diff, TUnit.Engine.csproj correctly removes the old TUnit.AspNetCore grant and does NOT add a new grant for TUnit.AspNetCore.Core — exactly right, since TUnit.AspNetCore.Core depends only on TUnit.Core and has no reason to access engine internals.
Status of all previously raised items
| # | Issue | Status |
|---|---|---|
| 1 | Dead TUnit.Engine InternalsVisibleTo grant |
✅ Fixed in commit 4 |
| 2 | Analyzer delivery through meta-package | ✅ Resolved (NuGet PackageReference semantics; mirrors TUnit.Core/TUnit pattern) |
| 3 | VersionOverride comment (CPM idiom is non-obvious) |
The only remaining minor nit: the VersionOverride pattern in the new .csproj still has no inline comment explaining why Version="10.0.5" VersionOverride="8.0.0" appear on the same element. This is pre-existing behaviour copied from the old project and is genuinely minor — the right time to add it is the next time someone touches the dependency versioning, not now.
Final Assessment
The PR is clean and all substantive feedback from previous reviews has been addressed. The architecture correctly mirrors the TUnit.Core/TUnit split, RootNamespace is preserved for source compatibility, all three solution files are updated, the CI pipeline packs the new package, snapshot tests are correct, and the source generator inclusion (commit 3) prevents the hooks regression that would have silently broken WebApplicationTest subclasses.
This is ready to merge.
|
Thank you for getting this done in record time! 🚀 |
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.30.0 to 1.30.8. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.30.8 <!-- Release notes generated using configuration in .github/release.yml at v1.30.8 --> ## What's Changed ### Other Changes * feat(mocks): migrate to T.Mock() extension syntax by @thomhurst in thomhurst/TUnit#5472 * feat: split TUnit.AspNetCore into Core + meta package by @thomhurst in thomhurst/TUnit#5474 * feat: add async Member() overloads for Task-returning member selectors by @thomhurst in thomhurst/TUnit#5475 ### Dependencies * chore(deps): update aspire to 13.2.2 by @thomhurst in thomhurst/TUnit#5464 * chore(deps): update dependency polyfill to 10.1.1 by @thomhurst in thomhurst/TUnit#5468 * chore(deps): update dependency polyfill to 10.1.1 by @thomhurst in thomhurst/TUnit#5467 * chore(deps): update tunit to 1.30.0 by @thomhurst in thomhurst/TUnit#5469 * chore(deps): update dependency microsoft.playwright to 1.59.0 by @thomhurst in thomhurst/TUnit#5473 **Full Changelog**: thomhurst/TUnit@v1.30.0...v1.30.8 Commits viewable in [compare view](thomhurst/TUnit@v1.30.0...v1.30.8). </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
TUnit.AspNetCore.Corecontaining all types, depending onTUnit.Core(notTUnit), so library authors can reference it without pulling in the test engine/runnerTUnit.AspNetCoreremains as a meta package for test projects, depending onTUnit+TUnit.AspNetCore.CoreTUnit.AspNetCore.Core(matching howTUnit.CorepacksTUnit.Analyzers)Motivation
TUnit.AspNetCoredepended onTUnit(the meta package with engine/runner), which meant any library referencing it would be treated as a test project. This prevented library authors from building shared ASP.NET Core test infrastructure on top ofTUnit.AspNetCore.Changes
TUnit.AspNetCore.Core— all 17 source files moved here, depends onTUnit.Core+TUnit.Logging.Microsoft+Microsoft.AspNetCore.Mvc.TestingTUnit.AspNetCore— stripped to meta package (TUnit + TUnit.AspNetCore.Core + dev-time analyzers/source generators)InternalsVisibleToinTUnit.Core,TUnit.Engine,TUnit.Logging.Microsoft→TUnit.AspNetCore.CoreTest plan
TUnit.AspNetCore.Corebuilds for net10.0TUnit.AspNetCoremeta package builds for net10.0TUnit.Example.Asp.Net.TestProjectbuilds for net10.0