Skip to content

Add -f/--framework support to TUnit template with .NET Framework handling#6262

Merged
thomhurst merged 5 commits into
mainfrom
copilot/add-support-for-framework-option
Jul 12, 2026
Merged

Add -f/--framework support to TUnit template with .NET Framework handling#6262
thomhurst merged 5 commits into
mainfrom
copilot/add-support-for-framework-option

Conversation

Copilot AI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

The TUnit template lacked -f/--framework support, making it impossible to scaffold projects targeting specific frameworks — particularly .NET Framework, which requires additional project configuration (polyfills, warning suppression, lang version).

Changes

  • template.json — Added framework choice parameter (net8.0–net10.0, net462–net481) with computed IsNetFramework symbol and conditional source exclusion for the Polyfills/ directory
  • TestProject.csproj — Conditional .NET Framework block: LangVersion=preview, EnableTUnitPolyfills=false, NoWarn for CS0592/CS0436, and Polyfill package reference
  • Polyfills/ExcludeFromCodeCoverageAttribute.cs — Compatibility shim only included for .NET Framework targets
  • Template tests — Added InstantiationTestWithNetFramework and OptionsWithFramework() helper; accepted snapshot

Usage

dotnet new TUnit -n MyTests -f net8.0    # modern .NET (clean project)
dotnet new TUnit -n MyTests -f net48     # .NET Framework (polyfills + warnings suppressed)
dotnet new TUnit -n MyTests              # defaults to net10.0

@codacy-production

codacy-production Bot commented Jun 16, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 21 complexity

Metric Results
Complexity 21

View in Codacy

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.

Copilot AI changed the title [WIP] Add support for -f / --framework with template handling Add -f/--framework support to TUnit template with .NET Framework handling Jun 16, 2026
Copilot AI requested a review from thomhurst June 16, 2026 09:36
Copilot AI temporarily deployed to Pull Requests June 16, 2026 17:24 Inactive
Copilot AI temporarily deployed to Pull Requests June 16, 2026 17:24 Inactive
Copilot AI temporarily deployed to Pull Requests June 16, 2026 17:24 Inactive
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review

Scaffolding logic (computed IsNetFramework symbol, Polyfills/** exclusion modifier, conditional <!--#if--> blocks in the csproj) is sound and the new snapshot test gives good representative coverage of the .NET Framework path. Two things worth tightening before merge:

1. Polyfill version pinned in the template doesn't match the repo's own pin (medium)

TUnit.Templates/content/TUnit/TestProject.csproj hardcodes:

<PackageReference Include="Polyfill" Version="10.8.*" PrivateAssets="all" />

but Directory.Packages.props centrally pins Polyfill to 10.11.2 for every other project in the repo (TUnit.Engine, TUnit.UnitTests, etc.). That means the net4x scaffolds this template produces are never validated against the same Polyfill version the rest of the codebase builds/tests against — any bug fixed between 10.8 and 10.11 could ship to template users while the repo's own suite stays green. Suggest sourcing the template's version from the same pin (or bumping it to match 10.11.2) so the two can't silently drift.

2. Project-wide NoWarn for CS0436 is broader than the actual problem (architecture)

<NoWarn>$(NoWarn);CS0592;CS0436</NoWarn>

This suppresses "type conflicts with imported type" for the entire scaffolded project, but the only intentional conflict is the ExcludeFromCodeCoverageAttribute shim in Polyfills/. A user who later introduces their own accidental type-name collision with a BCL type will get no diagnostic at all, since the warning is gone project-wide. Scoping the suppression to just the shim file instead — #pragma warning disable CS0436 / #pragma warning restore CS0436 around the type declaration in Polyfills/ExcludeFromCodeCoverageAttribute.cs — resolves the expected conflict without silencing the diagnostic anywhere else in the user's own code. Same idea would apply to CS0592 if it's only needed for that file.

Minor / non-blocking

  • The framework symbol's "replaces": "net10.0" does a literal string replace across all generated content. Today there's only one match (TestProject.csproj), so it's safe, but it's a footgun for future maintainers who might add "net10.0" as plain text elsewhere (docs/comments) without realizing it'll get silently rewritten too.
  • GlobalHooksstatic class in HooksAndLifecycle.cs is a reasonable cleanup (the class only has static members) but is unrelated to framework support — consider splitting drive-by cleanups like this into their own PR/commit for a cleaner history, though it's low-risk as-is.

Nice addition overall — the .NET Framework story has been a real gap for template users.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Code review (follow-up)

Re-reviewed after the latest commits. Both issues from my previous review are now resolved:

  1. Polyfill version mismatch — fixed in 8875a1bd. The template now pins Polyfill to 10.11.2, matching the central pin in Directory.Packages.props, so netfx scaffolds are validated against the same version the rest of the repo builds against.
  2. **Overly broad NoWarn for CS0436/CS0592** — also fixed in 8875a1bd, and via a better approach than the pragma-suppression I suggested: the shim was moved into its own TestProject.Polyfillsnamespace (referenced via a fully-qualified[assembly: TestProject.Polyfills.ExcludeFromCodeCoverage]) instead of reusing System.Diagnostics.CodeAnalysis`. That removes the type collision entirely rather than suppressing the diagnostic — a user who accidentally introduces their own conflicting type will still get a real CS0436 in their own code, with no leftover suppression footprint. Cleaner and more maintainable than scoping a pragma around the shim.

No new issues introduced by these changes; snapshots were regenerated and committed correctly per the repo's snapshot-testing rule.

The one remaining minor note from before — the framework symbol's "replaces": "net10.0" doing a literal string substitution across all template content — is unchanged and still non-blocking, just something to watch if future template content adds plain-text "net10.0" mentions outside the csproj.

Nothing further blocking from my side. (Note: this PR is still marked as a draft.)

@thomhurst
thomhurst marked this pull request as ready for review July 12, 2026 22:14
@thomhurst
thomhurst force-pushed the copilot/add-support-for-framework-option branch from 8875a1b to b596128 Compare July 12, 2026 22:15
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Code review (follow-up #2)

Confirmed both previously-flagged issues are resolved and stayed resolved in the current head:

  • Polyfill is pinned to 10.11.2 in the template, matching Directory.Packages.props.
  • The NoWarn CS0436/CS0592 project-wide suppression is gone; the shim now lives in its own TestProject.Polyfills namespace and is referenced via the fully-qualified [assembly: TestProject.Polyfills.ExcludeFromCodeCoverage], so there's no more collision to suppress. Also confirmed EnableTUnitPolyfills=false for net4x is intentional and correct here — TUnit.Core.props auto-injects TUnit's own Polyfill reference for .NETFramework/netstandard2.x TFMs by default, so leaving it enabled alongside the template's own explicit PackageReference Include="Polyfill" would risk a duplicate package reference. Good catch keeping that flag.

One small new nit (non-blocking)

TUnit.Templates/content/TUnit/HooksAndLifecycle.cs:

using System.Diagnostics.CodeAnalysis;

#if (IsNetFramework)
[assembly: TestProject.Polyfills.ExcludeFromCodeCoverage]
#else
[assembly: ExcludeFromCodeCoverage]
#endif

The using System.Diagnostics.CodeAnalysis; is only needed for the #else (non-netfx) branch, but it isn't wrapped in the same conditional. The net48 snapshot (InstantiationTestWithNetFramework.../HooksAndLifecycle.cs) confirms it: the generated netfx project keeps using System.Diagnostics.CodeAnalysis; even though nothing in that file uses it anymore (it uses the fully-qualified TestProject.Polyfills.ExcludeFromCodeCoverage instead). It's harmless (unused-using is IDE-only, not a build warning by default) but scaffolds a project with a dead using statement out of the box. Could wrap it in the same #if (!IsNetFramework) / #endif for a clean generated file, if you want to polish it.

Still open from earlier (minor, unchanged)

The framework symbol's "replaces": "net10.0" does a literal string substitution across all generated content — still just one match today (TestProject.csproj), still a footgun if future template content adds plain-text "net10.0" elsewhere. Not blocking.

No new correctness or architectural issues found. The conditional-generation approach (computed IsNetFramework symbol, Polyfills/** source exclusion, XML-comment #if for csproj + real #if for .cs) is idiomatic for dotnet new templates and the snapshot coverage for the net48 path is solid. Good to merge from my side.

This was referenced Jul 15, 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.

2 participants