Skip to content

Add inline diff coverage check to Azure Pipeline#597

Open
hdwhdw wants to merge 5 commits intosonic-net:masterfrom
hdwhdw:feature/inline-diff-coverage
Open

Add inline diff coverage check to Azure Pipeline#597
hdwhdw wants to merge 5 commits intosonic-net:masterfrom
hdwhdw:feature/inline-diff-coverage

Conversation

@hdwhdw
Copy link
Contributor

@hdwhdw hdwhdw commented Feb 27, 2026

Why I did it

The coverage.sonic-net.sonic-gnmi.build GitHub Check is posted by a black-box GitHub App ("Azure-Pipelines-Wrapper") with no code visibility. Moving diff coverage enforcement into the public azure-pipelines.yml follows infrastructure-as-code principles and makes the check transparent and maintainable.

How I did it

Added a new DiffCoverageCheck job in the Build stage that:

  1. Depends on both PureCIJob and build jobs
  2. Downloads coverage artifacts published by both upstream jobs
  3. Runs diff-cover against the combined coverage data
  4. Fails the check if new/modified lines have less than 80% coverage
  5. Only runs on PR builds (condition: eq(variables['Build.Reason'], 'PullRequest'))

Other changes:

  • Publish coverage XML as pipeline artifacts from PureCIJob and build job
  • Add fetchDepth: 0 to build job checkout for git merge-base support
  • Remove unused DIFF_COVER_* variables from MemoryLeakJob and build job
  • Add DIFF_COVER_THRESHOLD top-level variable (80%)
  • Upgrade PublishCodeCoverageResults from v1 to v2
  • Add diff-cover Makefile target for local testing
PureCIJob ──publish coverage-pure.xml──┐
                                       ├──> DiffCoverageCheck (diff-cover)
build ─────publish coverage.xml────────┘

How to verify it

Validated in build 1048370. The DiffCoverageCheck job ran and correctly reported diff coverage on the dummy test packages (which were intentionally under-tested):

Diff Coverage
Diff: origin/master...HEAD, staged and unstaged changes
-------------
testpkg/integration/helper.go (38.5%): Missing lines 19-20,22,27-30,33
testpkg/pure/math.go (30.8%): Missing lines 18,23-24,26,31-32,34-35,37
-------------
Total:   26 lines
Missing: 17 lines
Coverage: 34%

The dummy test packages have been removed in the latest commit since they served only as validation vehicles.

After merging, the Azure-Pipelines-Wrapper GitHub App can be uninstalled from the repository.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Replace black-box Azure-Pipelines-Wrapper GitHub App with inline diff coverage check in Azure Pipeline YAML.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Replace the black-box Azure-Pipelines-Wrapper GitHub App with an
inline DiffCoverageCheck job that runs diff-cover against combined
pure and integration coverage artifacts.

Changes:
- Add DiffCoverageCheck job depending on PureCIJob and build
- Publish coverage XML as pipeline artifacts from both jobs
- Add fetchDepth: 0 to build job for git merge-base support
- Remove unused DIFF_COVER_* variables from MemoryLeakJob and build
- Add DIFF_COVER_THRESHOLD top-level variable (80%)
- Upgrade PublishCodeCoverageResults to v2
- Add diff-cover Makefile target for local testing
- Add dummy test packages for PR validation

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
Copilot AI review requested due to automatic review settings February 27, 2026 20:20
@mssonicbld
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the Azure-Pipelines-Wrapper GitHub App with a first-party Azure Pipelines job that runs diff-cover against combined pure + integration coverage artifacts to enforce inline diff coverage on PRs.

Changes:

  • Adds a DiffCoverageCheck job that downloads published coverage artifacts and runs diff-cover with a configurable threshold.
  • Publishes pure and integration Cobertura XML files as pipeline artifacts and upgrades PublishCodeCoverageResults to v2.
  • Adds a local make diff-cover target and introduces dummy packages wired into pure/integration test package lists.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
azure-pipelines.yml Adds diff-cover gating job, publishes coverage artifacts, sets fetchDepth: 0, and upgrades coverage publishing task version.
pure.mk Adds testpkg/pure to the “pure” package set so it runs in pure CI/coverage.
Makefile Adds testpkg/integration to integration basic packages and introduces a diff-cover convenience target.
testpkg/pure/math.go Dummy pure functions to generate diff lines for coverage validation.
testpkg/pure/math_test.go Dummy tests with intentionally incomplete coverage.
testpkg/integration/helper.go Dummy integration-like helper functions to generate diff lines for coverage validation.
testpkg/integration/helper_test.go Dummy tests with intentionally incomplete coverage.

Comment on lines +8 to +15
// Repeat returns s repeated n times.
func Repeat(s string, n int) string {
result := ""
for i := 0; i < n; i++ {
result += s
}
return result
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeat builds the result via result += s inside a loop, which causes repeated allocations/copies (quadratic behavior as n grows). Use strings.Repeat(s, n) or a strings.Builder/[]byte preallocation approach to keep runtime and allocations predictable.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dismissed — dummy packages have been removed.

With a single checkout step, Azure Pipelines checks out directly
into the working directory without a subdirectory.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@mssonicbld
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

The DiffCoverageCheck job was validated in build 1048370. Removing
the temporary testpkg/ packages and their Makefile/pure.mk entries.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@mssonicbld
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

…re coverage in Makefile

- Remove codeCoverageTool input from PublishCodeCoverageResults@2 (v2 auto-detects)
- Add succeeded() guard to DiffCoverageCheck condition to skip on upstream failure
- Include test-results/coverage-pure.xml in Makefile diff-cover target for parity with pipeline

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@mssonicbld
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…lity

The Azure-Pipelines-Wrapper GitHub App needs these variables to
complete its check. They can be removed once the app is uninstalled.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
@hdwhdw
Copy link
Contributor Author

hdwhdw commented Feb 27, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants