Migrate the Test Project to Microsoft.Testing.Platform - #95
Merged
Conversation
The hub's reusable validator moved its .NET unit-test step to the MTP-native `dotnet test --coverage` form in ptr727/ProjectTemplate#1107, promoted in #1111. This repo's test project is on xunit.v3 4.0.0, which is MTP-based, and was held on the VSTest adapter by an explicit `IsTestingPlatformApplication` opt-out because the .NET 10 SDK refuses to run an MTP project through classic `dotnet test`. Taking the new validator pin without migrating would fail the unit-test job outright, so the two move together. - `global.json` declares the Microsoft.Testing.Platform test runner. It carries no `sdk` key, so it pins no SDK version and affects nothing but test execution. - `PhotoCleanerTests.csproj` drops the `IsTestingPlatformApplication` opt-out and `xunit.runner.visualstudio`, the VSTest adapter MTP replaces, and swaps `coverlet.collector` for `Microsoft.Testing.Extensions.CodeCoverage`, whose VSTest data collector MTP would otherwise ignore without failing. - `Directory.Packages.props` pins that extension at 18.10.0, above the 18.9.0 floor `WORKFLOW.md` D1.6 states. It resolves to Microsoft.Testing.Platform 2.3.3, the same platform version `xunit.v3` 4.0.0 and `Microsoft.NET.Test.Sdk` 18.9.0 resolve to, so the three carry no skew. - The four hub reusable-workflow pins move to 2.0.526 together, keeping the whole chain on one hub commit. - `codecov.yml` ignores `**/obj/**`. The MTP extension instruments the source generators' output, which coverlet did not, and that output exists in no checkout Codecov can map a path to. - `.gitignore` covers the report shapes the new collector can write. - `WORKFLOW.md` D1.6 is re-vendored from the hub canonical at 2.0.526, and `OPERATIONS.md` drops the now-inverted Dependabot runbook entry describing the opt-out this change removes. Verified locally on SDK 10.0.400: `dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage` exits 0 and runs 373 tests (368 passed, 5 Docker-gated skips), writing one `<guid>.cobertura.xml` at 74.5% line rate that names only `PhotoCleaner/` sources. Build, csharpier, `dotnet format style`, `dotnet husky run`, markdownlint, cspell, and editorconfig-checker are all clean.
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates reusable CI workflow revisions and migrates test coverage collection from Coverlet and VSTest integration to Microsoft Testing Platform with Cobertura reporting. ChangesMicrosoft Testing Platform coverage
Reusable workflow revisions
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk:⚪ Minimal · up to The test project is migrated to Microsoft.Testing.Platform with updated coverage handling and workflow pins, and the reported validation checks pass; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoMigrate test execution and coverage to Microsoft.Testing.Platform
AI Description
Diagram
High-Level Assessment
Files changed (10) |
Code Review by Qodo🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR |
There was a problem hiding this comment.
🟢 Approval recommended
The migration is consistent across runner selection, package pins, docs, and workflow pins, with only minor doc-grammar nits noted.
Pull request overview
Migrates PhotoCleaner’s xUnit v3 test project to run under Microsoft.Testing.Platform (MTP) so it remains compatible with the hub-hosted reusable validator’s updated dotnet test --coverage ... invocation and continues producing uploadable Cobertura coverage for Codecov.
Changes:
- Added a root
global.jsonselecting theMicrosoft.Testing.Platformtest runner fordotnet test. - Updated the test project + central package pins to use
Microsoft.Testing.Extensions.CodeCoverage(and removed VSTest-only adapters/collectors). - Updated Codecov ignore rules, gitignore coverage outputs, operational docs, and bumped hub reusable-workflow pins together.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Updates the CI contract (D1.6) to describe the MTP-based coverage collection flow and its version floor. |
| PhotoCleanerTests/PhotoCleanerTests.csproj | Removes the VSTest opt-out/adapter and switches coverage collection to the MTP extension package. |
| OPERATIONS.md | Updates local/CI verification and Dependabot runbook guidance to match the MTP + new coverage behavior. |
| global.json | Selects Microsoft.Testing.Platform as the repo’s dotnet test runner (no SDK pin). |
| Directory.Packages.props | Pins Microsoft.Testing.Extensions.CodeCoverage and removes VSTest-only coverage/adapter pins. |
| codecov.yml | Ignores **/obj/** to exclude source-generator outputs newly instrumented by the MTP collector. |
| .gitignore | Ignores coverage output shapes produced by the MTP coverage extension/tooling. |
| .github/workflows/test-pull-request.yml | Bumps hub reusable workflow pins used for PR validation/smoke build. |
| .github/workflows/publish-release.yml | Bumps hub reusable workflow pins used for plan/validate/build-release in publishing. |
| .github/workflows/merge-bot-pull-request.yml | Bumps the hub reusable workflow pin for merge-bot runs. |
Review details
- Files reviewed: 9/10 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The `Lint sources job` failed on `codecov.yml:15: comment-wrap`, because the comment added for the `**/obj/**` ignore wrapped one sentence across two lines and the gate wants one sentence per line. Resplit so each line holds exactly one sentence, and dropped the path-final `obj/.` the resplit had left reading as part of the path. Copilot raised the same wording on `WORKFLOW.md:152` and `OPERATIONS.md:20`. `WORKFLOW.md` D1.6 is hub canonical carried verbatim, so it is answered on the pull request rather than edited here. `OPERATIONS.md` is this repo's own prose, so its clause now names the two patterns `codecov-cli` actually matches instead of asserting only that the default name is unmatched. Also split the three sentences this branch had pushed past the 25-word cap (`OPERATIONS.md` lines 20, 47, and 101). `sentence-length` is not in the gate's default rule set, so none of them failed CI, but the fleet style is to write new prose under the cap and all three were prose this branch introduced. Verified: the hub's `prose_lint.py` over the branch diff exits 0, and the `sentence-length` check now reports nothing on any line this branch authored. markdownlint, `dotnet build` (0 warnings), and the 373-test suite are unchanged and clean.
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently align the test project, coverage tooling, and hub reusable-workflow pins with the new MTP-based validator contract without leaving behind references to the prior VSTest-based setup.
Review details
- Files reviewed: 9/10 changed files
- Comments generated: 0 new
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
ptr727 added a commit
that referenced
this pull request
Aug 30, 2026
* Correct the MTP Dependency and Runner Claims in OPERATIONS.md Answers three findings raised on #96, the promotion of #95. `PhotoCleanerTests.csproj` takes `PrivateAssets="all"` on `Microsoft.Testing.Extensions.CodeCoverage`, and deliberately not the `IncludeAssets` list its sibling references carry. That list omits the `compile` assets, and MTP's generated `SelfRegisteredExtensions.cs` then fails to build with `CS0234: The type or namespace name 'CodeCoverage' does not exist`. A comment records this, since the next reader has the same list five lines below to copy from. `OPERATIONS.md` had two wrong claims, both introduced by #95. The Dependabot runbook said `Microsoft.Testing.Extensions.CodeCoverage`, `xunit.v3`, and `Microsoft.NET.Test.Sdk` all resolve Microsoft.Testing.Platform 2.3.3. Only the first two declare it: the coverage extension directly, and `xunit.v3` through `xunit.v3.mtp-v2`. `Microsoft.NET.Test.Sdk` 18.9.0 declares `Microsoft.TestPlatform.TestHost` and `Microsoft.CodeCoverage`, the VSTest stack, and no MTP dependency at all. The skew hazard is therefore between two packages rather than three, and the paragraph now says so while keeping the point that `nuget-deps` bumps all three together. That paragraph also now prescribes `dotnet nuget why` rather than `dotnet list package --include-transitive`. The flat list cannot detect this skew: NuGet unifies the platform to one version per target framework, so it prints a single healthy row whichever major each package was built against, which is a false all-clear on exactly the failure being guarded. The `global.json` entry said `dotnet test` reaches the suite "rather than the VSTest host the .NET 10 SDK no longer runs it under". The SDK did not drop VSTest. What needs .NET 10 is the `global.json` `test.runner` key itself, since that is the SDK which reads it, and MTP predates the key. The entry now says that, and names all three things a return to VSTest would take rather than only the adapter. Verified: `dotnet nuget why PhotoCleanerTests/PhotoCleanerTests.csproj Microsoft.Testing.Platform` prints exactly the two roots the prose describes. Build (0 warnings), csharpier, `dotnet format style`, `dotnet husky run`, markdownlint, and the hub's `prose_lint.py` over the diff are clean, and the suite runs 373 tests at an unchanged 74.5% line rate. * Complete the List of What Puts This Repo on MTP Qodo raised, against the previous commit, that the `global.json` entry named three things as selecting Microsoft.Testing.Platform and omitted the coverage package. Correct: with `Microsoft.Testing.Extensions.CodeCoverage` in place, a project carrying only the other three would run under VSTest with no coverage collector at all, so the list understated what the choice rests on. The entry now names four rather than three, and no longer frames them as a rollback recipe, since a recipe stated in a runbook rots into a wrong one. It describes the current state instead, which carries the same information without promising a procedure. It also records that a move back is not this repo's alone to make. The `dotnet test` invocation that collects coverage lives in the hub-hosted validator, so the runner choice is only partly local. Verified: build (0 warnings), 373 tests at an unchanged 74.5% line rate, markdownlint clean, and the hub's `prose_lint.py` clean over the diff, with the one remaining `sentence-length` hit on `OPERATIONS.md:47` being the pre-existing opening sentence this branch does not touch.
ptr727 added a commit
that referenced
this pull request
Aug 30, 2026
The doc-drift half of the hub resync against ptr727/ProjectTemplate at 2.0.526, separated from the MTP migration (#95, #97) because the two are different drift classes. Audit run stamp 2026-08-30T20:48:19Z, hub f3b4cc9. - `GOVERNANCE.md`, two `verbatim` sections re-vendored byte-exact. "Durable Knowledge and Self-Improvement" had lost the bullet on when a durable rule earns a mechanical hook, and "Repository Boundaries and Write Safety" trailed the hub on the worktree-isolation bullet, which now names the `gh-write-guard` hook backstop and the standalone-clone fallback's grant. - `WORKFLOW.md`, two dropped hub clauses restored. D8.3 regains the sentence on a tracker setting `auto-merge: false`, and D1.2 regains the parenthetical naming both `needs:` edges on the validation job. Every repo-specific annotation on D1.1, D1.2, D1.4, D1.5, D4.1 and the S1 to S11 rows is kept. - `AGENTS.md`, the intro said the file holds "two things". It has held three since the Fleet Bootstrap section was carried, so it now says so. - `.editorconfig-checker.json`, the hub's `Exclude` block carried. This repo has no Python tree, but `uv` creates those cache directories when running a Python script from the hub against this checkout, so the excludes are reachable here. - `.github/skills/repo-worktree/SKILL.md` and `resync-a-repo/SKILL.md`, verbatim re-vendors. Deliberately not changed. `WORKFLOW.md` Section 3 keeps its local "and the hub's `repo-config/README.md`" pointer, which the hub does host and which matches how `AUDIT.md` already refers to hub-hosted tooling. Verified: every repo-specific block in the three mixed-fidelity files survives the re-vendor, checked by full-file diff against both the hub and the pre-change copy. All 18 verbatim `GOVERNANCE.md` sections and all 3 `AGENTS.md` sections now byte-match hub f3b4cc9, and `diff -rq` over `.github/skills/` is empty. markdownlint and the hub's `prose_lint.py` over the diff are clean.
ptr727 added a commit
that referenced
this pull request
Aug 30, 2026
* Re-Vendor Drifted Governance Sections and Skill Files from the Hub The doc-drift half of the hub resync against ptr727/ProjectTemplate at 2.0.526, separated from the MTP migration (#95, #97) because the two are different drift classes. Audit run stamp 2026-08-30T20:48:19Z, hub f3b4cc9. - `GOVERNANCE.md`, two `verbatim` sections re-vendored byte-exact. "Durable Knowledge and Self-Improvement" had lost the bullet on when a durable rule earns a mechanical hook, and "Repository Boundaries and Write Safety" trailed the hub on the worktree-isolation bullet, which now names the `gh-write-guard` hook backstop and the standalone-clone fallback's grant. - `WORKFLOW.md`, two dropped hub clauses restored. D8.3 regains the sentence on a tracker setting `auto-merge: false`, and D1.2 regains the parenthetical naming both `needs:` edges on the validation job. Every repo-specific annotation on D1.1, D1.2, D1.4, D1.5, D4.1 and the S1 to S11 rows is kept. - `AGENTS.md`, the intro said the file holds "two things". It has held three since the Fleet Bootstrap section was carried, so it now says so. - `.editorconfig-checker.json`, the hub's `Exclude` block carried. This repo has no Python tree, but `uv` creates those cache directories when running a Python script from the hub against this checkout, so the excludes are reachable here. - `.github/skills/repo-worktree/SKILL.md` and `resync-a-repo/SKILL.md`, verbatim re-vendors. Deliberately not changed. `WORKFLOW.md` Section 3 keeps its local "and the hub's `repo-config/README.md`" pointer, which the hub does host and which matches how `AUDIT.md` already refers to hub-hosted tooling. Verified: every repo-specific block in the three mixed-fidelity files survives the re-vendor, checked by full-file diff against both the hub and the pre-change copy. All 18 verbatim `GOVERNANCE.md` sections and all 3 `AGENTS.md` sections now byte-match hub f3b4cc9, and `diff -rq` over `.github/skills/` is empty. markdownlint and the hub's `prose_lint.py` over the diff are clean. * Drop the D1.2 Parenthetical Again, Which Is False for This Repo Both reviewers independently flagged the D1.2 clause restored in the previous commit. They are right, and the restore was a mistake. The hub's parenthetical reads "(both the aggregator and `smoke-build`, which `needs:` the validation job by name)". In this repo `smoke-build` declares `needs: [changes]` and nothing else, per `.github/workflows/test-pull-request.yml`, so the clause asserts an edge that does not exist here. The bullet's own repo-specific annotation two sentences later already says as much: the aggregator `needs:` validate directly, independent of `changes`/`smoke-build`. So its absence from this copy was a deliberate local adaptation rather than drift from the hub, and re-vendoring it put a false claim about this repo's own workflow into this repo's own contract doc. D1.2 goes back to the form it had on develop. This does not generalize to the D8.3 clause in the same commit, which stays. That one describes a tracker this repo does not have, rather than contradicting a workflow it does have, and `WORKFLOW.md` carries the whole D8 section as inapplicable-but-present already. Verified against the workflow rather than the doc: `grep` for the `needs:` lines in `test-pull-request.yml` gives `smoke-build` -> `[changes]` and `check-workflow-status` -> `[changes, validate, smoke-build]`. markdownlint and the hub's `prose_lint.py` over the diff stay clean.
This was referenced Aug 30, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes the hub's MTP-native unit-test step from ptr727/ProjectTemplate#1107, promoted in ptr727/ProjectTemplate#1111, and migrates this repo's test project onto the runner that step requires.
Why the Two Move Together
The hub's reusable
validate-task.ymlnow runsdotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage. That step is hub-hosted, so a caller cannot patch it.This repo's test project is on
xunit.v34.0.0, which is MTP-based, and was held on the VSTest adapter by an explicit<IsTestingPlatformApplication>false</IsTestingPlatformApplication>opt-out, because the .NET 10 SDK refuses to run an MTP project through classicdotnet test. Bumping the validator pin without migrating fails theunit-testjob outright, and migrating without bumping the pin leaves the old--collect:"XPlat Code Coverage"argument pointed at a project that no longer answers it. So the pin bump and the migration are one change.What Changed
global.json(new) declares the Microsoft.Testing.Platform test runner. It carries nosdkkey, so it pins no SDK version, and it affects nothing but test execution: the Docker build never copies it,nbgvdoes not read it, anddotnet publishis unaffected.PhotoCleanerTests.csprojdrops theIsTestingPlatformApplicationopt-out andxunit.runner.visualstudio, the VSTest adapter MTP replaces, and swapscoverlet.collectorforMicrosoft.Testing.Extensions.CodeCoverage, whose VSTest data collector MTP would otherwise ignore without failing.Directory.Packages.propspins that extension at 18.10.0, above the 18.9.0 floor D1.6 states.2.0.526together, keeping the whole chain on one hub commit.codecov.ymlignores**/obj/**, see below..gitignorecovers the report shapes the new collector can write.WORKFLOW.mdD1.6 is re-vendored from the hub canonical at2.0.526, andOPERATIONS.mdreplaces the now-inverted Dependabot runbook entry that told a future reader to set the opt-out this change removes.The Platform Version Has No Skew
All three test-stack packages resolve to Microsoft.Testing.Platform 2.3.3, read from
PhotoCleanerTests/obj/project.assets.json:xunit.v34.0.0,Microsoft.NET.Test.Sdk18.9.0, andMicrosoft.Testing.Extensions.CodeCoverage18.10.0. A skew between them is the silent-green failure D1.6 documents, where the run throwsTypeLoadException, executes zero tests, and still writes a well-formed Cobertura file reporting full coverage. TheOPERATIONS.mdrunbook entry now names that class in the direction Dependabot can actually produce it, a bump upward onto a newer platform major, rather than the downward one the first draft described, which Dependabot cannot reach from a 18.10.0 pin.The Coverage Denominator Moved, and One Part of It Is Fixed Here
The MTP extension is not a drop-in for coverlet on what it instruments.
IncludeTestAssemblydefaults false, and the concern was that the MTP extension's own default exclusions do not namePhotoCleanerTests.dll. Measured rather than assumed: the produced report contains zeroPhotoCleanerTestsand zeroPhotoCleanerBenchmarksentries, so noignoreentry is owed for either.PhotoCleaner/obj/Debug/net10.0/**/*.g.cs(theSystem.Text.Json,Microsoft.Gen.Logging, andRegexGeneratoroutputs). That is build product rather than reviewed source, and it exists in no checkout Codecov can map a path to, socodecov.ymlnow ignores**/obj/**.Verification
On SDK 10.0.400, in a clean worktree:
dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverageexits 0, running 373 tests: 368 passed, 5 skipped (the Docker-gatedVerifyTaskTestscases).coverage/<guid>.cobertura.xmlatline-rate="0.7447",branch-rate="0.6926", 5014 of 6733 lines covered. Real data, which also rules out the zero-tests-full-coverage failure mode above.dotnet build(0 warnings),dotnet csharpier check .,dotnet format style --verify-no-changes --severity=info, anddotnet husky runare clean.obj/andbin/, which a CI checkout does not carry.Not in This PR
The resync also found hub drift unrelated to MTP: two
GOVERNANCE.mdverbatim sections trailing the canonical, aWORKFLOW.mdD8.3 sentence and a Section-3 reference, theAGENTS.mdintro still saying the file holds "two things" after the Fleet Bootstrap section made it three, a missing.editorconfig-checker.jsonexclude block, and two stale.github/skills/*/SKILL.mdfiles. Those are a separate drift class and follow in their own PR off this one once it merges.One item needs a human check rather than a CI run: dropping
xunit.runner.visualstudioremoves the VSTest adapter the C# Dev Kit Test Explorer has historically discovered through. Recent builds support MTP natively, and the CLI is unaffected either way, but the VS Code Testing panel is worth opening once before this merges.Summary by CodeRabbit
Chores
Documentation