Add code coverage estimate to CI - #873
Open
Tess Gauthier (tgauth) wants to merge 8 commits into
Open
Conversation
Replace the archived OpenCppCoverage layer with the VS 2022 Microsoft.CodeCoverage.Console tool (native C/C++ static instrumentation). Add a Debug + /PROFILE build job and consume its artifacts in a PR-only, non-gating coverage job. Pure aggregation/overlap helpers and their Pester tests are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Drop Invoke-OpenSSHCodeCoverage.ps1 (the build+run+merge local driver); coverage is driven exclusively by Invoke-AzDOCodeCoverage.ps1 in CI. Update README to document local reproduction via the AzDO entry point. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds Windows (Win32-OpenSSH) native C/C++ coverage collection to PR CI using Microsoft.CodeCoverage.Console, with per-suite collection and merged (de-duplicated) reporting.
Changes:
- Introduces PowerShell module + Pester tests to parse/merge Cobertura and compute suite overlap.
- Adds a CI entry script to instrument installed binaries, collect per-suite coverage in server mode, and publish merged Cobertura + summary artifacts.
- Updates Azure DevOps build/test pipeline to produce a Debug+/PROFILE build for PRs and run a PR-only, non-gating coverage job.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| contrib/win32/openssh/code_coverage/README.md | Documents coverage approach, local repro, artifacts, and CI wiring. |
| contrib/win32/openssh/code_coverage/OpenSSHCodeCoverage.tests.ps1 | Adds Pester tests for the pure merge/overlap/path-normalization helpers. |
| contrib/win32/openssh/code_coverage/OpenSSHCodeCoverage.psm1 | Implements Cobertura parsing/merge/summary + CodeCoverage.Console orchestration. |
| contrib/win32/openssh/code_coverage/Invoke-AzDOCodeCoverage.ps1 | CI entry point to instrument, collect per suite, merge, and emit summaries. |
| .azdo/templates/build-win32-openssh-job.yml | Adds configurable build configuration + linker options for the coverage build. |
| .azdo/ci.yml | Adds PR-only coverage build job and PR-only non-gating coverage collection/publish job. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Two fixes for the bash coverage step: - Copy-CoverageRuntime searched only the CodeCoverage.Console tool dir, but static_covrun*.dll ships under '<VS root>\Team Tools\Dynamic Code Coverage Tools'. Walk up to the VS install root and search there so the runtime is copied beside the instrumented binaries; the sshd/ssh-agent services (no inherited PATH) can then load and record coverage. - The core suite ends with the uninstall test, which removes the sshd and ssh-agent services and the install dir. Re-install the coverage build before the bash coverage step so Invoke-OpenSSHBashTestsOnly finds the services, mirroring the standalone bash test job. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
- ConvertTo-NormalizedCoverageSourcePath stripped the repository root with a bare StartsWith, which could wrongly strip a sibling that merely shares a prefix (root 'C:/repo' vs path 'C:/repository/...'). Require a directory boundary (root followed by '/' or end-of-string). Adds a regression test. - Invoke-CoverageSession now terminates the server-mode collector (best-effort, with a warning) if it does not exit within 120s of shutdown, so a hung collector is not leaked into subsequent CI steps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Running Core and Bash coverage sequentially in one job shared a single C:\OpenSSH install and agent, causing two failures: the core flow's uninstall test removed the sshd/ssh-agent services and left the collector/instrumented processes holding libcrypto.dll, so the mid-job re-install hit a file lock, and the bash suite ran without services. Restructure to mirror the normal pipeline: separate PR-only, non-gating CodeCoverageCore and CodeCoverageBash jobs each run on their own agent with a clean install (no lock/state contention, and they run in parallel), publishing per-suite artifacts. A dependent CodeCoverageAggregate job merges them. - Extract the aggregation/merge/summary logic into an exported module function Invoke-CoverageAggregation. - Add -SkipAggregation to Invoke-AzDOCodeCoverage.ps1 (per-suite jobs) and a new Invoke-AzDOCoverageAggregate.ps1 for the dependent job. - Rewrite the ci.yml coverage jobs and update the README. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Tess Gauthier (tgauth)
requested review from
Vivian Thiebaut (vthiebaut10)
and
a lite review from Copilot
August 27, 2026 17:15
Vendored dependencies built through vcpkg (e.g. zlib) are linked into the OpenSSH binaries and appear in the raw coverage data, but they are not OpenSSH code and only dilute the estimate. Filter them out both from the summary (Import-CoberturaCoverage skips excluded paths) and from the published merged.cobertura.xml (new Remove-ExcludedCoverageClasses strips excluded classes and recomputes package/root line counters). Exclusion is driven by \ (default (^|/)vcpkg/). Adds Test-CoverageSourceExcluded predicate plus unit tests (20 total). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The Azure DevOps Code Coverage widget was fed the native Microsoft.CodeCoverage.Console merge, which counts each shared .c once per binary it is linked into (~3.6x inflation on this solution), so the widget headline (~41%) did not match the de-duplicated estimate in the coverage summary (~62%). Point PublishCodeCoverageResults@2 at the de-duplicated merged-helper.cobertura.xml so the widget headline matches the number we report. The native per-binary report and raw .coverage files remain in the Win32-OpenSSH-CodeCoverage artifact for per-binary / per-method drill-down. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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 free
to 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.
PR Summary
contrib/win32/openssh/code_coveragewith README/module/scripts, including tests to verify deduplication logic for same coverage from different test types (i.e. Bash and Pester)