fix(dotnet): rtk dotnet test breaks for projects using global.json MTP mode#1380
fix(dotnet): rtk dotnet test breaks for projects using global.json MTP mode#1380danielmarbach wants to merge 4 commits intortk-ai:developfrom
rtk dotnet test breaks for projects using global.json MTP mode#1380Conversation
📊 Automated PR Analysis
SummaryFixes global.json MTP detection to use VsTestBridge mode instead of the removed MtpNative mode, which was injecting --report-trx directly and breaking xUnit v3 projects with 'Unknown option' errors. Also skips -nologo for all MTP modes since it breaks on .NET 10 native MTP, and refactors the project file scanning into a separate function. Review Checklist
Linked issues: #1289 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
F0rty-Tw0
left a comment
There was a problem hiding this comment.
The compatibility matrix in the description made me back off two concerns I'd flagged on first read:
-
-nologoskip across MTP modes: I worried this was over-conservative, until the .NET 10 native MTP "Unknown option--nologo" repro. Skip is correct. -
xUnit v3 "Unknown option" surfacing: initially read as a silent UX downgrade, but the trade-off is conscious. A silent skip would regress token savings for the MSTest/TUnit majority who do bundle TrxReport, and the error is informative once a user knows the package name. NuGet detection from csproj/Directory.Build.props/Directory.Packages.props is impractical for the cited reasons (MSBuild inheritance, central package management), so surfacing the error is the right call.
I also ran a fresh sub-review against .claude/rules/rust-patterns.md and cli-testing.md. Variant deletion is complete: every MtpNative site the diff touches is a removal or rewrite, no residue. The scan_project_files_for_mtp early-return refactor is behavior-preserving (no multi-project regression), and there are no new unwrap() outside tests.
Three small follow-ups follow as inline comments, none blocking.
Open since April 18 with P2-important, three-OS CI green, repro from Altinn/altinn-register. Would be great to see this land.
…mode PR rtk-ai#746 introduced MtpNative mode which injected --report-trx directly for global.json MTP, breaking xUnit v3 projects with 'Unknown option' (exit 5). --report-trx requires Microsoft.Testing.Extensions.TrxReport which xUnit v3 doesn't bundle (it has --report-xunit-trx instead). Both global.json MTP and project-file MTP now use MtpVsTestBridge, injecting -- --report-trx after the separator. This works for MSTest/TUnit which bundle TrxReport. For xUnit v3 without the extension, the error is informative — tells users to add the NuGet package (per xunit/xunit#3456). Also skips -nologo for all MTP mode since it breaks on .NET 10 native MTP. Fixes rtk-ai#1289
Co-authored-by: Artiom Tofan <[email protected]>
rtk dotnet test breaks for projects using global.json MTP mode
…d but flagged during review
Fixes #1289
Problem
PR #746 introduced
MtpNativemode which injected--report-trxas a direct flag for repos withglobal.jsonMTP mode ("test": { "runner": "Microsoft.Testing.Platform" }). This broke xUnit v3 projects with exit code 5:The Altinn/altinn-register repo was affected: .NET 9 SDK, global.json MTP mode, xUnit v3 — no project-file MTP properties at all.
Root cause
--report-trxis not a universal MTP flag. It requires theMicrosoft.Testing.Extensions.TrxReportNuGet package which xUnit v3 doesn't bundle (it has--report-xunit-trxinstead). MSTest and TUnit bundle it, so it worked for them but failed for xUnit v3.Verified behavior
All tested with
global.jsonhaving"test": { "runner": "Microsoft.Testing.Platform" }:-- --report-trx--report-trxThe xUnit v3 issue reporter confirmed (xunit/xunit#3456) that adding
Microsoft.Testing.Extensions.TrxReportNuGet makes--report-trxwork universally.Additionally,
-nologobreaks on .NET 10 native MTP with "Unknown option '--nologo'", so it's now skipped for all MTP mode.Fix
Simplified
TestRunnerModefrom three variants to two:Classic: VSTest runner →--logger trx --results-directory,-nologoinjectedMtpAfterSeparator: Any MTP detection (global.json OR project-file) →-- --report-trx,-nologoskippedRemoved
MtpNativeandMtpGlobalJson. Both global.json MTP and project-file MTP properties now useMtpAfterSeparator, injecting-- --report-trxafter the separator.For xUnit v3 without
Microsoft.Testing.Extensions.TrxReport, this produces an informative "Unknown option" error — the user knows to add the package. This is preferable to silently skipping TRX injection, which would regress token savings for the majority of MTP users (MSTest, TUnit) who do bundle TrxReport and benefit from the optimized TRX parsing path. Detecting the NuGet package from csproj/Directory.Build.props/Directory.Packages.props was ruled out as impractical (MSBuild inheritance, central package management).Test plan
-- --report-trx→ informative error-- --report-trx→ TRX generated-- --report-trx→ TRX generated-nologoskipped for all MTP mode (verified on .NET 10 native MTP)