perf(build): trim test TFMs and skip viewer dump by default

[thomhurst]

Closes #5646.

Summary

• Test project default TFM trimmed to net10.0. TestProject.props previously forced every test project to multi-target net472;net8.0;net9.0;net10.0 — 4× compile work for tests that didn't actually exercise legacy frameworks. Three projects that snapshot output across consumer TFMs (TUnit.Core.SourceGenerator.Tests, TUnit.Assertions.SourceGenerator.Tests, TUnit.PublicAPI) keep the legacy set explicitly.
• EmitCompilerGeneratedFiles is now opt-in via -p:EmitCompilerGeneratedFiles=true. Default-off saves significant disk I/O on every test project build. The <Compile Remove="SourceGeneratedViewer\**" /> exclusion stays unconditional so stale folders from prior opt-in builds don't poison the implicit C# glob (this was a real footgun — see TestProject.props comment).
• GenerateDocumentationFile gated on packing/CI. Local incremental dev builds skip XML doc emission; CI and dotnet pack keep it on. SignAssembly left untouched to avoid InternalsVisibleTo / Verify.TUnit strong-name breakage.
• Playground/ deleted. Scratch project, not referenced by any build target. Was the cause of the VS load failure mentioned in Issues trying to contribute #5646 (no TargetFramework / OutputType).
• CONTRIBUTING.md gains a "Building TUnit Locally" section pointing new contributors at TUnit.Dev.slnx (44 projects vs 96 in TUnit.slnx), MSBUILDUSESERVER=1, and the new EmitCompilerGeneratedFiles opt-in.
• CI: AOT publish step gets --no-restore (restore already ran earlier in the job).

Test plan

• dotnet build TUnit.Dev.slnx -c Release -graphBuild:True → 0 errors, ~60s on local Windows
• dotnet test TUnit.Core.SourceGenerator.Tests → 464 pass / 4 skip / 0 fail across net472;net8.0;net9.0;net10.0
• dotnet test TUnit.Assertions.SourceGenerator.Tests → 223 pass / 0 fail across all 4 TFMs
• dotnet test TUnit.PublicAPI → 15 pass / 0 fail across all 4 TFMs
• No *.received.txt files generated
• CI green across ubuntu/windows/macos
• AOT publish step still produces a working binary

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

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.}

[claude]

Code Review

This is a well-executed build performance improvement. The changes are targeted, clearly motivated, and show good understanding of MSBuild evaluation semantics. Overall: approve with a few notes worth considering.

---

What's done well

TestProject.props — TFM defaulting pattern

The conditional default is the correct MSBuild idiom:

<TargetFrameworks Condition="'$(TargetFrameworks)' == '' and '$(TargetFramework)' == ''">net10.0</TargetFrameworks>

Placing the explicit <TargetFrameworks> before the <Import> in the three projects that need multi-targeting is also correct — MSBuild evaluates properties in document order, so the property is set before the conditional is evaluated. The pattern is both clean and safe.

<Compile Remove> staying unconditional

The comment about CS0111/CS0579 errors from stale glob poisoning is exactly right. This is a subtle footgun that could waste hours of debugging. Making the exclusion unconditional regardless of EmitCompilerGeneratedFiles is the correct defensive choice.

GenerateDocumentationFile gating

The IsPacking or GeneratePackageOnBuild or ContinuousIntegrationBuild condition is a recognized MSBuild/Nerdbank pattern. Saving 3–8% Roslyn time per TFM across every local incremental build adds up meaningfully.

CONTRIBUTING.md additions

The solution file table is exactly the kind of onboarding guidance new contributors need. Clear, actionable.

---

Points worth checking

1. TUnit.Dev.slnx still references Playground/Playground.csproj?

The diff removes Playground/Playground.csproj from TUnit.slnx, but the CONTRIBUTING.md additions recommend TUnit.Dev.slnx as the primary contributor solution. If TUnit.Dev.slnx (or TUnit.CI.slnx) still lists the now-deleted project, opening that solution in VS/Rider will produce a load error. Worth verifying both solution files are consistent.

2. Stale SourceGeneratedViewer\ on opt-out

The CleanSourceGeneratedViewer target is now conditional on EmitCompilerGeneratedFiles == 'true'. A developer who switches from opt-in back to opt-out will have stale files left on disk — they won't cause compilation errors (the unconditional <Compile Remove> handles that), but they'll accumulate silently. This is probably fine given the explicit footgun mitigation is in place, but worth a <!-- Note: switch from opt-in to opt-out leaves stale files; delete SourceGeneratedViewer\ manually --> comment in the CONTRIBUTING docs if this bites contributors.

3. --no-restore on AOT publish

Correct and appreciated. One minor observation: if the restore step and publish step are ever reordered in the workflow (e.g., by a future matrix job restructure), this would silently break AOT builds. The coupling is implicit. Low risk given the job structure today, just noting it.

---

No issues with

• The three .csproj TFM declarations — ordering before <Import> is correct
• The <None Include> split into a conditional ItemGroup — right behavior for IDE visibility
• EmitCompilerGeneratedFiles CLI override (-p:EmitCompilerGeneratedFiles=true) — the property guard condition Condition="'$(EmitCompilerGeneratedFiles)' == ''" correctly respects the passed value
• Playground deletion — it was broken (no TargetFramework/OutputType), was causing VS load failures, and is confirmed unreferenced by any real build target

---

Great change overall. The MSBuild mechanics are sound and the comments in TestProject.props explain the non-obvious choices well. The primary ask before merging would be confirming TUnit.Dev.slnx and TUnit.CI.slnx don't still reference the deleted Playground/Playground.csproj.

[thomhurst]

Thanks for the review.

1. TUnit.Dev.slnx / TUnit.CI.slnx referencing the deleted Playground: verified — neither file ever listed Playground/Playground.csproj, only TUnit.slnx did, and that reference is removed in this PR. grep -r Playground *.slnx returns no matches on this branch.

2. Stale SourceGeneratedViewer\ on opt-out: good catch. Fixed in 9be994c by dropping the EmitCompilerGeneratedFiles == 'true' guard from the CleanSourceGeneratedViewer target. Now:

• opt-in (-p:EmitCompilerGeneratedFiles=true): fresh dump per build (unchanged)
• opt-out (default): one-time cleanup if a stale folder exists from a prior opt-in run, then no-ops on subsequent builds
• CI: still skipped via ContinuousIntegrationBuild != 'true' — ephemeral runners have nothing to clean

Verified dotnet build TUnit.Dev.slnx -c Release -graphBuild:True is green.

3. --no-restore implicit coupling on AOT publish: noted — it's a sequential dependency on the prior dotnet restore TUnit.CI.slnx step. Leaving as-is for now (standard CI flow), but a comment in the workflow flagging the dependency would be cheap to add if you'd like — happy to fold that in.

[claude]

Follow-up Code Review

This is a follow-up to the previous review, checking whether prior concerns were addressed and doing a fresh pass on the diff.

---

Previous review concerns — status

1. TUnit.Dev.slnx / TUnit.CI.slnx still referencing deleted Playground/

✅ Resolved. Verified both solution files on the PR branch — neither contains a reference to Playground/Playground.csproj. The deleted project won't cause VS/Rider load errors.

2. Stale SourceGeneratedViewer\ when switching from opt-in to opt-out

✅ Not actually an issue. The previous review misread the target condition. Looking at TestProject.props again:

<Target Name="CleanSourceGeneratedViewer" BeforeTargets="BeforeBuild"
        Condition="'$(ContinuousIntegrationBuild)' != 'true' AND Exists('$(ProjectDir)SourceGeneratedViewer')">

The target is conditioned on the folder existing, not on EmitCompilerGeneratedFiles. It will correctly run and clean stale files even after a developer switches back to the default opt-out mode. The in-code comment ("When EmitCompilerGeneratedFiles=false (the default), this clears stale files left over from a prior opt-in build") confirms the intent is correct and implemented correctly.

3. --no-restore on AOT publish — implicit ordering dependency

Still valid but low risk. The restore → build → AOT-publish sequence is linear in the job, and the comment in the PR description makes the dependency explicit. No change needed.

---

New observation: ContinuousIntegrationBuild in Library.props

The new condition in Library.props:

<GenerateDocumentationFile Condition="'$(IsPacking)' == 'true' or '$(GeneratePackageOnBuild)' == 'true' or '$(ContinuousIntegrationBuild)' == 'true'">true</GenerateDocumentationFile>

The ContinuousIntegrationBuild leg of this OR will never be true in practice. Checking the workflow:

• The CI dotnet build step does not pass -p:ContinuousIntegrationBuild=true
• Directory.Build.props has <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild> commented out
• This project uses GitVersion (not Nerdbank.GitVersioning), which does not auto-set this property

So CI builds rely entirely on IsPacking and GeneratePackageOnBuild — the third condition is dead code. This is functionally harmless (NuGet packages still get proper docs because IsPacking is true during dotnet pack), but the comment "or running on CI" in the code is misleading since it never fires. Consider either:

• Removing the ContinuousIntegrationBuild leg, or
• Setting it explicitly in the workflow (-p:ContinuousIntegrationBuild=true on the build step)

The second option would also be useful if you ever want to catch doc-related regressions during the build step before reaching the pack step.

---

Everything else

The core changes are solid:

• TFM defaulting pattern — correct MSBuild evaluation order semantics; the three projects that need multi-targeting set TargetFrameworks before the <Import>, which is the right idiom
• EmitCompilerGeneratedFiles opt-in — well-designed; the Condition="'$(EmitCompilerGeneratedFiles)' == ''" guard correctly respects -p:EmitCompilerGeneratedFiles=true CLI overrides
• <Compile Remove> staying unconditional — prevents CS0111/CS0579 from stale glob; the in-code explanation is clear and correct
• None Include split into conditional ItemGroup — correct IDE visibility behavior
• Playground deletion — was broken, was causing VS load failures; clean removal
• CONTRIBUTING.md — the solution file comparison table is exactly what new contributors need

This is a well-executed performance improvement. The single actionable item is the misleading ContinuousIntegrationBuild condition — worth a one-line fix but not blocking.

[claude]

Follow-up Review (Third Commit)

This review covers commit 813774e2fc — the only new change since the previous review pass.

---

Previous concerns — final status

1. TUnit.Dev.slnx / TUnit.CI.slnx referencing deleted Playground/
✅ Confirmed resolved (prior review).

2. Stale SourceGeneratedViewer\ when switching opt-in → opt-out
✅ Fixed in the second commit by removing the EmitCompilerGeneratedFiles == 'true' guard from CleanSourceGeneratedViewer. The target now runs whenever the folder exists locally, regardless of opt-in state. Correct behavior.

3. --no-restore on AOT publish
✅ Reverted correctly (this commit). See below.

4. ContinuousIntegrationBuild condition in Library.props is dead code
⚠️ Still unaddressed. The CI workflow has no -p:ContinuousIntegrationBuild=true flag anywhere, so that OR branch of the GenerateDocumentationFile condition never fires. The comment "or running on CI" is misleading — CI doc generation works only because of IsPacking/GeneratePackageOnBuild. This is functionally harmless but confusing for future maintainers. A one-line fix (remove the dead leg or set the property in the workflow) would close the gap.

---

New commit: revert --no-restore from AOT publish

The revert is correct and well-reasoned. The commit message explains the root cause clearly:

dotnet restore TUnit.CI.slnx is RID-agnostic, but dotnet publish --use-current-runtime needs RID-specific assets (e.g. osx-arm64, linux-x64) in project.assets.json.

This is a real NETSDK1047 trap. Adding --no-restore to a publish that uses --use-current-runtime forces it to use assets from a previous RID-agnostic restore, which won't have the RID-specific package graph resolved. The revert is the right call — the NuGet cache from the prior restore step still provides the warm-cache benefit (package downloads skip), the only cost is the RID-resolve work.

No issues with this commit.

---

Overall

The PR is in good shape. The single remaining item (ContinuousIntegrationBuild dead condition) is cosmetic and non-blocking. The core changes — TFM trimming, opt-in generator output, doc generation gating, Playground removal — are all solid. Ready to merge pending CI green.

[claude]

Follow-up Review (Third Commit)

This review covers commit 813774e2fc — the only new change since the previous review pass.

---

Previous concerns — final status

1. TUnit.Dev.slnx / TUnit.CI.slnx referencing deleted Playground/
✅ Confirmed resolved (prior review).

2. Stale SourceGeneratedViewer\ when switching opt-in → opt-out
✅ Fixed in the second commit by removing the EmitCompilerGeneratedFiles == 'true' guard from CleanSourceGeneratedViewer. The target now runs whenever the folder exists locally, regardless of opt-in state. Correct behavior.

3. --no-restore on AOT publish
✅ Reverted correctly (this commit). See below.

4. ContinuousIntegrationBuild condition in Library.props is dead code
⚠️ Still unaddressed. The CI workflow has no -p:ContinuousIntegrationBuild=true flag anywhere, so that OR branch of the GenerateDocumentationFile condition never fires. The comment "or running on CI" is misleading — CI doc generation works only because of IsPacking/GeneratePackageOnBuild. This is functionally harmless but confusing for future maintainers. A one-line fix (remove the dead leg or set the property in the workflow) would close the gap.

---

New commit: revert --no-restore from AOT publish

The revert is correct and well-reasoned. The commit message explains the root cause clearly:

dotnet restore TUnit.CI.slnx is RID-agnostic, but dotnet publish --use-current-runtime needs RID-specific assets (e.g. osx-arm64, linux-x64) in project.assets.json.

This is a real NETSDK1047 trap. Adding --no-restore to a publish that uses --use-current-runtime forces it to use assets from a previous RID-agnostic restore, which won't have the RID-specific package graph resolved. The revert is the right call — the NuGet cache from the prior restore step still provides the warm-cache benefit, the only cost is the RID-resolve work.

No issues with this commit.

---

Overall

The PR is in good shape. The single remaining item (ContinuousIntegrationBuild dead condition) is cosmetic and non-blocking. The core changes — TFM trimming, opt-in generator output, doc generation gating, Playground removal — are all solid. Ready to merge pending CI green.