Conversation
…#1107) Implements the hub half of #1088, the change the fleet's dotnet repos are waiting on so they can keep using the hub-hosted reusable workflow. ## Why On the .NET 10 SDK and later an MTP-based test project refuses to run through the VSTest target, so the reusable validator's `dotnet test --collect:"XPlat Code Coverage"` step fails outright for any caller whose test project has become MTP-based, in practice at xunit.v3 4.0.0. That step is hub-hosted, so it is the one file such a caller cannot patch for itself. PhotoCleaner is already in exactly that bind, and every repo converging onto the reusable workflow inherits it. ## What Changed - `.github/workflows/validate-task.yml` - the `unit-test` job's .NET step moves to the MTP-native `--coverage` form driving `Microsoft.Testing.Extensions.CodeCoverage`, and normalizes each report's filename before the unchanged Codecov upload step reads `./coverage`. - `WORKFLOW.md` D1.6 - the canonical invocation, plus the consumer-side prerequisites the hub cannot supply (root `global.json` MTP opt-in, the coverage package, dropping `xunit.runner.visualstudio`). - `spec/project-types.json` - the `csharp.coverage.codecov` assert kept in sync with D1.6. - `.gitignore` - the coverage-output comment no longer names coverlet. Scope is the hub only. Per-repo migration PRs, and the mechanical audit check for the `global.json` prerequisite, are follow-up work. ## Coverage Engine Standardized on `Microsoft.Testing.Extensions.CodeCoverage` rather than `coverlet.MTP`, per the maintainer's call on #1088's open decision: it is the direction the platform is heading, and Utilities' in-flight PRs already chose it. The output stays Cobertura under `./coverage`, so `codecov.yml` semantics are unchanged. ## Two Load-Bearing Details, Both Verified Neither of these failures reds the job on its own, so both were measured rather than assumed. 1. **`--coverage-output` stays unset.** Pinning one filename gives every test project in the solution the same path. A two-project probe confirmed the second silently overwrote the first: both tests pass, exit 0, and one project's coverage is simply gone. 2. **The reports are prefixed to `coverage-<guid>.cobertura.xml`.** The default name that dropping the flag produces, `<guid>.cobertura.xml`, is one `codecov-cli`'s own file finder does not match, its patterns being `*coverage*.*` and an exact `cobertura.xml`. Left alone it uploads nothing at all, silently, because `fail_ci_if_error: false`. ## Verification On SDK 10.0.400, against a constructed two-project solution: - The step writes one report per test project, both packages survive, with real per-method data (a covered method at `hits="1"`, an uncovered one at `hits="0"`). - `codecov-cli`'s own `FileFinder`, run over the tree the step actually produced, returns both files. - **Version floor:** at 18.0.4, which is what a `>= 18.0.0` range resolves to, the run throws a `TypeLoadException` against the MTP 2.x that xunit.v3 4.0.0 carries, runs zero tests, and *still writes a well-formed Cobertura file reporting full coverage*. Only the non-zero exit distinguishes it. 18.9.0 is the first release on MTP 2.3.x, where every test project writes into the one shared results directory the invocation names. Gates: 879 unit tests, prose lint, spec validate, audit selftest, repo gate, build_dist, actionlint, markdownlint, and cspell all clean. ## Sequencing Callers pin this workflow by SHA, so nothing moves until a repo bumps its own pin. The prerequisites are written as binding on a repo whose test project is MTP-based rather than on every C# repo, so a repo still on VSTest holding its existing pin reads as a migration owed rather than as a letter failure this same change would call legitimate. Refs #1088 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Updated .NET test coverage reporting to use Microsoft Testing Platform and generate Cobertura reports. * Coverage files now follow standardized naming for automatic Codecov discovery and upload. * Improved coverage compatibility for projects using the modern testing platform. * **Documentation** * Documented migration requirements, supported coverage tooling, report naming, and upload behavior. * Updated project guidance and coverage configuration references for .NET projects. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
PR Summary by QodoPromote MTP-native .NET coverage workflow to main
AI Description
Diagram
High-Level Assessment
Files changed (4) |
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 (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe pull request changes C# coverage validation from XPlat Code Coverage to built-in Changes.NET coverage handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This change updates the shared validation workflow and its related documentation and specifications for the MTP-native coverage path. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 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. (4 skipped: 4 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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.
🔵 Needs a closer look
This is a high-impact change to the hub-hosted CI workflow and its cross-repo contract, so it should receive final human review despite no issues found here.
Pull request overview
Promotes develop to main, carrying the hub’s migration of the reusable validator’s .NET unit-test coverage step from VSTest/--collect to Microsoft.Testing.Platform’s native --coverage flow, and updates the workflow contract/spec text to match.
Changes:
- Switch the reusable
unit-testjob todotnet test --coverage ...and normalize Cobertura filenames under./coveragefor Codecov discovery. - Update the workflow contract (D1.6) to document MTP prerequisites and the load-bearing coverage/report-naming details.
- Update the C# type assertion and
.gitignorecoverage comment to align with the new coverage mechanism.
File summaries
| File | Description |
|---|---|
.github/workflows/validate-task.yml | Migrates the unit-test step to MTP-native coverage and prefixes Cobertura reports for Codecov file-finding. |
WORKFLOW.md | Updates D1.6 to document the new invocation, prerequisites, and failure modes. |
spec/project-types.json | Updates the csharp.coverage.codecov letter-tier assertion to match the new workflow contract. |
.gitignore | Renames the coverage-output comment to reflect the new .NET coverage producer. |
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 0
- 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.
…packages xunit.v3 4.0.0 drops the VSTest bridge, so `dotnet test --collect:"XPlat Code Coverage"` fails outright on the .NET 10 SDK with "Testing with VSTest target is no longer supported by Microsoft.Testing.Platform". That is what has been blocking the nuget-deps bumps in #545 and #546, and it blocks every later bump until the runner moves. Follow WORKFLOW.md D1.6 as the hub settled it in ptr727/ProjectTemplate#1111, which chose Microsoft.Testing.Extensions.CodeCoverage over coverlet: - `global.json` opts into the `Microsoft.Testing.Platform` runner. It carries no `sdk` section, so SDK resolution and roll-forward are untouched. - `xunit.runner.visualstudio` is dropped, the VSTest adapter having no role under native MTP. - `coverlet.collector` becomes `Microsoft.Testing.Extensions.CodeCoverage` 18.9.0, whose predecessor's VSTest data collector MTP ignores without failing. The floor is load-bearing rather than cautionary: below 18.1.0 the extension is built against Microsoft.Testing.Platform 1.x and throws a `TypeLoadException` against the 2.x platform xunit.v3 4.0.0 carries, running zero tests while still writing a well-formed Cobertura file that reports full coverage. - The CI unit-test step becomes `dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, then prefixes each report to `coverage-<guid>.cobertura.xml`. Both halves are load-bearing and neither reds the job on its own: `--coverage-output` stays unset so a second test project could not overwrite the first, and the default `<guid>.cobertura.xml` that produces is a name codecov-cli's finder does not match, its patterns being `*coverage*.*` and an exact `cobertura.xml`. Bundle the four package bumps the runner change unblocks: AwesomeAssertions 9.5.0 -> 9.6.0, xunit.analyzers 1.27.0 -> 2.0.0, xunit.v3 3.2.2 -> 4.0.0, and `xunit.runner.visualstudio` removed rather than bumped to 4.0.0. `.gitignore` gains the hub's coverage block. The output was untracked and unignored, so a blanket `git add -A` after a local coverage run would have staged it. Verified against the real invocation, not the documented one: 21 tests ran and passed (not the zero the version-floor trap produces), the extension resolved at 18.9.0 on Microsoft.Testing.Platform 2.3.3, the run wrote `<guid>.cobertura.xml` and the prefix step renamed it as intended, both filename shapes are ignored while nothing tracked is, and the build, CSharpier, `dotnet format style --verify-no-changes`, editorconfig-checker, actionlint, markdownlint and cspell gates are all clean.
…packages (#553) Unblocks the Dependabot nuget PRs (#545, #546) and every later nuget bump. **Replaces #549**, which GitHub closed automatically when its base branch `resync/eol-lf` was deleted on #547's merge. Same branch, same content, same review history on the closed PR. This one targets `develop` directly. The branch carries a `-s ours` merge of `develop` recorded after #547 squashed. That is lossless here and was verified before recording: `git diff origin/develop 2efaeb2` is empty, so the squash reproduced this branch's own ancestor exactly, and that ancestor is reachable from this branch, so `develop` carries nothing the branch lacks. The diff against `develop` is the five files below and nothing else. > Reworked after ptr727/ProjectTemplate#1111 merged. The first revision used > `coverlet.MTP`, which was the open recommendation at the time. The hub has since > settled WORKFLOW.md D1.6 on `Microsoft.Testing.Extensions.CodeCoverage`, and this > PR now follows that instead. ## The problem `xunit.v3` 4.0.0 removed the VSTest bridge, so the CI unit-test step's `dotnet test --collect:"XPlat Code Coverage"` now fails outright on the .NET 10 SDK: ```text error : Testing with VSTest target is no longer supported by Microsoft.Testing.Platform on .NET 10 SDK and later. If you use dotnet test, you should opt-in to the new dotnet test experience. ``` That is what #545 and #546 have been red on, and it blocks every later bump until the runner moves. ## The fix, per WORKFLOW.md D1.6 | Change | Why | | --- | --- | | `global.json` with `{"test":{"runner":"Microsoft.Testing.Platform"}}` | opts into the native MTP runner. No `sdk` section, so SDK resolution and roll-forward are untouched. | | drop `xunit.runner.visualstudio` | the VSTest adapter MTP replaces | | `coverlet.collector` -> `Microsoft.Testing.Extensions.CodeCoverage` 18.9.0 | coverlet's VSTest data collector is ignored under MTP without failing | | CI step -> `dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, then prefix each report to `coverage-<guid>.cobertura.xml` | matches the hub's `validate-task.yml` byte for byte | Three details are load-bearing, and none of them reds the job on its own: - **The 18.9.0 floor.** Below 18.1.0 the extension is built against Microsoft.Testing.Platform 1.x and throws `TypeLoadException` against the 2.x platform xunit.v3 4.0.0 carries. It then runs **zero tests** and still writes a well-formed Cobertura file reporting full coverage. - **`--coverage-output` stays unset.** Pinning one filename would give every test project in a solution the same path, and the last to finish would overwrite the rest. - **The prefix rename.** The default `<guid>.cobertura.xml` that the unset flag produces is a name codecov-cli's finder does not match (its patterns are `*coverage*.*` and an exact `cobertura.xml`), so an unprefixed report uploads nothing while the step still exits green. Package bumps the runner change unblocks: AwesomeAssertions 9.5.0 -> 9.6.0, xunit.analyzers 1.27.0 -> 2.0.0, xunit.v3 3.2.2 -> 4.0.0. `Microsoft.NET.Test.Sdk` stays at 18.9.0 (already current). `.gitignore` gains the hub's dotnet coverage block verbatim. The output was untracked and unignored, so a blanket `git add -A` after a local coverage run would have staged it. ## Verification Against the real invocation, not the documented one: - 21 tests **ran** and passed, which is the check that matters given the zero-test failure mode above. - Resolved graph (from `obj/project.assets.json`, not the csproj text): `Microsoft.Testing.Extensions.CodeCoverage/18.9.0`, `Microsoft.Testing.Platform/2.3.3`, `xunit.v3/4.0.0` with the `mtp-v2` variants. No coverlet, no `xunit.runner.visualstudio`. - The run wrote `bbfde807-....cobertura.xml` and the prefix step renamed it to `coverage-bbfde807-....cobertura.xml`, confirming the rename is genuinely needed rather than defensive. - `git check-ignore` covers both filename shapes; `git ls-files -z | xargs -0 git check-ignore -v` is empty. - `validate-task.yml` is the only `dotnet test` caller; `publish-release.yml` and `test-pull-request.yml` both reach it via `uses:`, so the publish gate and the PR gate move together. No `--collect` survives anywhere. - Build, CSharpier, `dotnet format style --verify-no-changes`, editorconfig-checker, actionlint, markdownlint and cspell all clean. **One thing worth knowing:** the reported coverage number moves, because the engine does. Line rate goes from 0.59 under coverlet to 0.26 here, with lines-valid 1350 -> 3143, since this engine instruments more of the graph. It cannot gate anything: `codecov.yml` sets `informational: true` on both project and patch, and the only ruleset-required check is `Check pull request workflow status job`. ## Notes - Dependabot dual-targets `develop` and `main`, so #546 (the `main` copy) stays blocked until this reaches `main` via the promotion PR. `main`'s tree is not broken in the meantime, it just cannot take the bump. - The fleet audit flags this repo's local `validate-task.yml` as `hub-only`. Now that ptr727/ProjectTemplate#1111 has landed MTP support in the hub's reusable workflow, that migration is unblocked, but it is a separate interface change covering five workflow files and belongs in its own PR rather than here. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved automated test execution and code coverage reporting in continuous integration. * Coverage reports are now generated in a standardized format for more reliable analysis. * **Documentation** * Added guidance for running tests locally and understanding coverage validation. * **Chores** * Updated testing tools and configuration to use the modern test platform. * Added rules to keep generated test and coverage files out of source control. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Promotes the ProjectTemplate resync to `main`. Five pull requests, merged to `develop` in order: #547, #553, #550, #551, #552. **Replaces #555.** That one opened `develop` directly into `main` and went `DIRTY`, which is the spurious EOL-only promotion conflict the fleet's own branching guidance names: `develop` flipped the `.editorconfig` line-ending default to LF and `main` has not caught up, so every renormalized path conflicts whole-file. `develop`'s squash-only ruleset and required linear history forbid resolving on `develop` itself, so the resolution is prepared on this throwaway branch off `main`, exactly as that guidance prescribes. Five files conflicted. Each was checked with the documented test rather than resolved on the shape of the conflict. `CreateMatrix/CreateMatrix.csproj`, `Make/Matrix.json` and `Make/Version.json` are content-identical modulo EOL. `CODESTYLE.md` and `CreateMatrixTests/CreateMatrixTests.csproj` genuinely differ, so each was proved a superset instead: every line present only on `main` is a superseded version, being the `AGENTS.md` routing that `develop` repointed to `GOVERNANCE.md`, the CRLF line-endings item, two list items renumbered when the encoding rule was inserted, and the pre-MTP test packages. **The merged tree is byte-identical to `develop`**, so nothing on `main` survives that `develop` had not already replaced. ## What lands | | | | --- | --- | | **Line endings** | `.gitattributes` becomes `* text=auto eol=lf` and `.editorconfig` declares `end_of_line = lf` on `[*]`, with `*.bat`/`*.cmd` the one CRLF exception. 85 tracked files renormalized. The `CreateMatrix` generator moves with them, or the codegen App would rewrite its own outputs back to CRLF on its next scheduled run. | | **Test runner** | Native Microsoft.Testing.Platform per WORKFLOW.md D1.6, as the hub settled it in ptr727/ProjectTemplate#1111. Unblocks the Dependabot bumps that had been red since xunit.v3 4.0.0 dropped the VSTest bridge. | | **Carried content** | The hub's `.github/skills/` tree (37 files, digest-verified), `host-tools.json`, `CLAUDE.md`, `GOVERNANCE.md`, and the shared lint config. | | **Instruction set** | `AGENTS.md` split onto the router model, from 243 lines to 115, with the repo-specific half moved to new `ARCHITECTURE.md` and `OPERATIONS.md`. `repo-config/` retired. | | **README** | Restructured to the fleet shape, eleven `readme-structure` findings closed, and the 2.15 release documented. | | **Security** | `validate-task.yml` and `test-pull-request.yml` given least privilege. They ran with the repository default, which is `write` on this repo with `can_approve_pull_request_reviews` true, while only ever reading the tree. | ## Reviewing the diff `git diff --ignore-cr-at-eol origin/main origin/develop` reduces the 143-file diff to the substantive changes. Everything else is the CRLF-to-LF renormalization, which is the EOL-only promotion conflict the fleet's own branching guidance names. ## Checks before opening this `main` carries no content `develop` lacks. `git diff --ignore-cr-at-eol origin/develop origin/main` shows 980 lines present only on `main`, and every one is a superseded version of something `develop` deliberately changed: `main`'s CRLF `.editorconfig` pins, its `* -text` `.gitattributes`, its pre-split 243-line `AGENTS.md`, its `AppendLineCrlf` generator, its `set -euo` run blocks. No file has content on `main` that is not either carried forward or intentionally replaced. The three commits labelled main-only were checked individually rather than assumed, since those are the ones the develop-staleness rule exists for: the cspell CI scope from #515/#517 is byte-identical on both branches, `CODESTYLE.md`'s "Spelling CI scope" from #516 is present, and the author-identity rule from #511 is in the carried `GOVERNANCE.md`. The commit counts look alarming and are not: `develop` is 19 ahead and 99 behind, but 70 of those 99 are bot merges that dual-target both branches as separate commits, and most of the rest are `main`'s own promotion merge commits, which never flow back by construction. ## Not in this promotion - **Two `interface` audit findings stay open by decision.** `publish-release.yml` missing job `publish` and `merge-bot-pull-request.yml` missing job `merge-bot` both resolve to adopting hub-hosted task workflows this repo has not adopted, and `spec/divergences.json` states adoption "is a separate, later change per repo". Renaming jobs to satisfy the checker would report conformance without adopting anything. - **The publish chain's workflow permissions**, filed as #554. Least privilege there is not `contents: read`, a wrong scope breaks publishing rather than failing a lint, and it would surface on a weekly scheduled run. - **An inert xUnit sequential collection**, filed as #548. ## Filed upstream Four issues against `ptr727/ProjectTemplate` from work this resync exposed: promotion candidates and two misleading interface findings (#1116, including my own correction retracting six of ten), the MTP zero-tests-locally behaviour (#1122), the version-floor guardrail (#1124), and 18 findings against carried canonical content that cannot be fixed downstream (#1131). ## Owed after merge The GitHub About description still carries the retired tagline. It feeds the Docker Hub short description through the docker-readme task, so that surface disagrees with the README until it is set by hand. `registry/repos.json` declares no `description` for this repo, so the configure script cannot write it. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Reorganized project documentation around NxWitness, including build, distribution, releases, product variants, troubleshooting, and release notes. - Added architecture, governance, operations, and comprehensive development guidance. - **Developer Experience** - Added editor tasks for building, formatting, validation, testing, and linting. - Standardized line endings and formatting across project files. - **CI and Quality** - Improved shell error reporting and workflow permission safety. - Updated coverage generation and test tooling. - Added stricter Markdown and spelling validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Promotes
developtomain, carrying one change.What Is Being Promoted
#1107 - Migrate the Hub's Unit-Test Step to Native Microsoft.Testing.Platform
On the .NET 10 SDK and later an MTP-based test project refuses to run through the VSTest target, so the reusable validator's
dotnet test --collect:"XPlat Code Coverage"step failed outright for any caller whose test project had become MTP-based, in practice at xunit.v3 4.0.0. That step is hub-hosted, so it was the one file such a caller could not patch for itself. This unblocks the fleet's dotnet repos, which need it to keep using the hub-hosted workflow..github/workflows/validate-task.yml- theunit-testjob's .NET step moves to the MTP-native--coverageform drivingMicrosoft.Testing.Extensions.CodeCoverage, and normalizes each report's filename before the unchanged Codecov upload step reads./coverage.WORKFLOW.mdD1.6 - the canonical invocation, plus the consumer-side prerequisites the hub cannot supply.spec/project-types.json- thecsharp.coverage.codecovassert kept in sync with D1.6..gitignore- the coverage-output comment no longer names coverlet.Three Silent-Green Failure Modes, All Measured
Each of these passes CI while reporting nothing or reporting wrongly, so each was verified against a constructed two-project solution on SDK 10.0.400 rather than reasoned about.
--coverage-outputgives every test project in the solution the same path. The second overwrote the first: both tests pass, exit 0, one project's coverage simply absent.<guid>.cobertura.xml, is onecodecov-cli's own file finder does not match, its patterns being*coverage*.*and an exactcobertura.xml. Underfail_ci_if_error: falsethat uploads nothing, silently. The step therefore prefixes each report tocoverage-<guid>.cobertura.xml, verified by runningcodecov-cli'sFileFinderover the tree the step actually produces.Microsoft.Testing.Extensions.CodeCoveragebelow 18.1.0 throws aTypeLoadExceptionagainst the MTP 2.x xunit.v3 4.0.0 carries, runs zero tests, and still writes a well-formed Cobertura file reporting full coverage. Only the non-zero exit distinguishes it. D1.6 states 18.9.0, the first release on MTP 2.3.x where every test project writes into the one shared results directory.Sequencing
Callers pin this workflow by SHA, so no repo moves until it bumps its own pin. The prerequisites bind a repo whose test project is MTP-based rather than every C# repo, so a repo still on VSTest holding its existing pin reads as a migration owed rather than as a letter failure this same change calls legitimate. An un-migrated caller that bumps its pin anyway fails loudly (
MSBUILD : error MSB1001, exit 1, non-merging) rather than going quietly uncovered, which was a deliberate call over adding a VSTest fallback.Review Disposition on #1107
Five findings, all closed: three qodo findings fixed (an over-long comment block, change-framing in a comment, and semicolons in D1.6's
Preventsclause), and two CodeRabbit findings declined with evidence, with the accurate observation under the second filed as #1110. Copilot's coverage stayed PARTIAL at 3/4 files across all rounds, the known terminal condition, taken as a maintainer call.Not closing #1088, which still owns the per-repo migrations and its remaining plan items. Follow-ups filed: #1108 (audit check for the
global.jsonprerequisite) and #1110 (intent-tier checks have no mechanical floor).Refs #1088
Summary by CodeRabbit
Chores
Documentation