PR Pipeline - Code Coverage and Triggers by benrr101 · Pull Request #4360 · dotnet/SqlClient

benrr101 · 2026-06-11T22:17:00Z

Description

This is the last PR before operationalizing the PR pipeline.

PR triggers are added for the pipeline - copy/pasted from the existing PR pipeline
Code coverage stage is added that publishes test results and code coverage results
- This required some reworking of the code to populate platforms as parameters to the pipeline. It is recommended not to touch these if you manually run the pipeline :)

Testing

🔄 ADO Internal
https://dev.azure.com/SqlClientDrivers/ADO.Net/_build/results?buildId=156200&view=results
🔄 ADO Public
https://sqlclientdrivers.visualstudio.com/public/_build/results?buildId=156192&view=results
✅ Non-Official Pipeline
https://dev.azure.com/SqlClientDrivers/ADO.Net/_build/results?buildId=156112&view=results

# Conflicts: # eng/pipelines/pr/jobs/test-buildproj-job.yml # eng/pipelines/pr/stages/test-stages.yml

… all tests are first.

Copilot

Pull request overview

This PR operationalizes the dedicated Azure DevOps PR validation pipeline by adding GitHub PR triggers, publishing per-job test results as pipeline artifacts, and introducing a post-test stage to merge and publish code coverage (including upload to CodeCov).

Changes:

Add GitHub PR triggers + path filters to the PR pipeline YAML.
Publish TRX results (and artifacts containing coverage outputs) from each test job to a consistent pipeline artifact namespace.
Add a coverage-merge stage that downloads .coverage files, merges to Cobertura, publishes coverage to ADO, and uploads to CodeCov.

Reviewed changes

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

Show a summary per file

File	Description
eng/pipelines/pr/variables/pr-variables.yml	Adds variables for CodeCov token and the shared test-results artifact name.
eng/pipelines/pr/steps/publish-test-results-step.yml	New reusable step to publish TRX results and upload the job’s test-results folder as an artifact.
eng/pipelines/pr/stages/test-stages.yml	Threads `testResultsArtifactName` through all PR test jobs.
eng/pipelines/pr/stages/collect-coverage-stage.yml	New stage to download coverage artifacts, merge to Cobertura, publish to ADO, and upload to CodeCov.
eng/pipelines/pr/pr-pipeline.yml	Adds PR triggers/path filters, centralizes platform matrix as a parameter, and wires in the coverage-collection stage.
eng/pipelines/pr/jobs/test-sqlclientmanual-job.yml	Wires test-results publishing into SqlClient manual test jobs.
eng/pipelines/pr/jobs/test-buildproj-job.yml	Wires test-results folder routing + publishing into build.proj-driven test jobs.

paulmedynski · 2026-06-12T01:01:10Z

+  - task: PublishTestResults@2
+    displayName: 'Publish Test Results'
+    inputs:
+      buildConfiguration: ${{ parameters.buildConfiguration }}
+      testResultsFiles: |
+        ${{ parameters.testResultsPath }}/*.trx
+        ${{ parameters.testResultsPath }}/**/*.coverage
+      testRunTitle: "${{ parameters.platformDisplayName }}_${{ parameters.testDisplayName }}"
+    condition: succeededOrFailed()


Worth a try.

paulmedynski · 2026-06-12T00:51:44Z

            -t:Test${{ parameters.packageShortName }}${{ parameters.testProject }}
            -p:Configuration=${{ parameters.buildConfiguration }}
            -p:BuildNumber='$(Build.BuildNumber)'
            -p:BuildSuffix='${{ parameters.buildSuffix }}'
            -p:TestFramework=${{ parameters.platformDotnet }}
+            -p:TestResultsFolderPath=${{ variables.testResultsPath }}
+


Indeed - I suspect $(REPO_ROOT) is the culprit here, unless it is populated by compile-time values/variables too (i.e. can you use ${{ variables.REPO_ROOT }} above?).

🤔 I don't actually see the problem here. $() formatted variables are expanded at runtime, while ${{ }} formatted ones are evaluated at compile time.

So, at compile time, testResultsPath gets evaluated as eg, $(REPO_ROOT)/test_results/windows_net462_abstractions. Then at step runtime, it's further expanded to D:\a\_work\1\s/test_results/windows_net462_abstractions.

You can see it behaving correctly in any of the test runs: eg https://sqlclientdrivers.visualstudio.com/public/_build/results?buildId=156095&view=logs&j=9aaebe84-0bb8-5049-010a-f5d7cc3b7829&t=671e3b01-ce1f-5040-8f31-c4248ac036e2&l=12

benrr101 · 2026-06-12T17:10:03Z

+      - template: /eng/pipelines/pr/steps/publish-test-results-step.yml@self
+        parameters:
+          buildConfiguration: ${{ parameters.buildConfiguration }}
+          platformDisplayName: ${{ parameters.platformDisplayName }}
+          testDisplayName: ${{ parameters.testDisplayName }}
+          testResultsArtifactName: ${{ parameters.testResultsArtifactName }}
+          testResultsPath: ${{ variables.testResultsPath }}


This actually isn't an issue, if you look at the test logs https://dev.azure.com/SqlClientDrivers/ADO.Net/_build/results?buildId=155942&view=logs&j=9aaebe84-0bb8-5049-010a-f5d7cc3b7829&t=6f7d4681-2ac1-5fe1-83e3-59d9d33c8750

I'm not sure there's a concept of "runtime variables" as is being asserted here. There are values that are only available at runtime, but as long as those are referenced by $() they will remain $() until they are used at runtime, where they are further expanded to their value.

It's worth noting that if this wasn't the case, we'd never be able to have $(Password) in our connection strings and expect it to be expanded when we write it to the config file.

benrr101 · 2026-06-12T17:15:08Z

            -p:TestFramework=${{ parameters.platformDotnet }}
            -p:TestSet=${{ parameters.testSet }}

+      # Publish test results
+      - template: /eng/pipelines/pr/steps/publish-test-results-step.yml@self
+        parameters:
+          buildConfiguration: ${{ parameters.buildConfiguration }}
+          platformDisplayName: ${{ parameters.platformDisplayName }}
+          testDisplayName: "sqlclient_manual_${{ parameters.configDisplayName}}_${{ parameters.testSet }}"
+          testResultsArtifactName: ${{ parameters.testResultsArtifactName }}
+          testResultsPath: ${{ variables.testResultsPath }}


That and the test result folder path isn't being passed into the build.proj step

benrr101 · 2026-06-12T17:16:32Z

+          - task: CopyFiles@2
+            displayName: Copy nuget.config for tool installs
+            inputs:
+              contents: "$(REPO_ROOT)/nuget.config"
+              targetFolder: $(Agent.TempDirectory)


Ah yes because copying 0 files is success 😑

I wanted to avoid using a direct script here because we have tasks for these things ... but ok whatever.

benrr101 · 2026-06-12T17:22:57Z

+          - task: PublishCodeCoverageResults@2
+            displayName: Publish coverage results
+            inputs:
+              summaryFileLocation: "${{ variables.workingDir }}/merge/cobertura.xml"


Still not an issue....

benrr101 · 2026-06-12T17:23:22Z

+          - script: |
+              curl -o "${{ variables.workingDir }}/codecov" https://cli.codecov.io/latest/linux/codecov
+              chmod +x "${{ variables.workingDir }}/codecov"
+            displayName: Download CodeCov CLI


It works just fine

benrr101 · 2026-06-12T17:23:52Z

+          - script: >-
+              "${{ variables.workingDir }}/codecov"
+              --verbose
+              upload-process
+              --fail-on-error
+              --git-service github
+              --slug dotnet/SqlClient
+              -f "${{ variables.workingDir }}/merge/cobertura.xml"
+              -F $(Build.DefinitionName)
+            displayName: Upload coverage to CodeCov
+            env:
+              CODECOV_TOKEN: ${{ parameters.codeCovApiToken }}


I can't believe I'm misunderstanding how this works that badly

paulmedynski

Looking good! A few comments/suggestions.. I thumbs-upped the bot comments I agree with, or replied to them. I also closed a few that I disagreed with.

paulmedynski · 2026-06-12T00:56:00Z

+              contents: "$(REPO_ROOT)/nuget.config"
+              targetFolder: $(Agent.TempDirectory)
+
+          - task: DotNetCoreCLI@2


This tool is controlled by our global tools config, so I think this and the above task can be replaced with dotnet tool install. I suspect you copied most of this file from the old PR/CI pipeline, and it's likely just stale.

You're right that I copy/pasted it (with revisions) from the old pipeline, but I also confirmed that DotNetCLI task doesn't support the tool command for whatever reason.

The above might be unrelated to your actual comment here.

I think as long as we're doing everything from the temp directory, we need to have dotnet-coverage installed globally so we can call it as a command outside the repository.
I guess alternatively, we could do everything in a folder inside the repository, but I don't know if the repository directory is cleaned up after a job runs.

paulmedynski · 2026-06-12T01:01:10Z

+  - task: PublishTestResults@2
+    displayName: 'Publish Test Results'
+    inputs:
+      buildConfiguration: ${{ parameters.buildConfiguration }}
+      testResultsFiles: |
+        ${{ parameters.testResultsPath }}/*.trx
+        ${{ parameters.testResultsPath }}/**/*.coverage
+      testRunTitle: "${{ parameters.platformDisplayName }}_${{ parameters.testDisplayName }}"
+    condition: succeededOrFailed()


Worth a try.

paulmedynski · 2026-06-12T01:04:00Z

  #
  # AzureKeyVaultTenantId
  # AzureKeyVaultUrl
+  # CodeCovApiToken


Is this a secret we shouldn't be exposing to forked PRs? If yes, then maybe we should do everything except upload to CodeCov in this pipeline, and do CodeCov stuff in the CI pipeline.

Do any of these other variables contain secrets?

:/ I'm not sure how secrety it is... I think API keys are usually considered secrets, so I'd agree with your suggestion to remove uploading to CodeCov. But I also don't know how critical uploading results to codecov are for our PR validation, so I'd be hesitant to break it as long as it's not officially classified as a secret.

None of the other variables are considered secrets, they either rely on managed identity or have the generated secret injected into them at runtime.

paulmedynski · 2026-06-12T01:07:09Z

+#
+# It maps to the "sqlclient-pr" pipeline in the Public ADO project:
+#    https://sqlclientdrivers.visualstudio.com/public/_build?definitionId=2281
+# And the "pr-pipeline" pipeline in the internal ADO project:


I think we want the pipelines to have the same name in both projects. sqlclient-pr seems correct. I would also name this file sqlclient-pr-pipeline.yml to match the pipeline name. I've been doing that for the other new pipelines.

My thought was it didn't make sense to name it sqlclient-* when you're in the sqlclient project. But in the public project we're sharing space with other products, so we'd want to differentiate.

But, ok fine, I don't care too much about the scheme as long as its systematic and reasonable.

paulmedynski · 2026-06-12T01:08:58Z

+      - .azuredevops/*
+      - .config/*
+      - doc/*
+      - eng/pipelines/*


Hmm.. is there a narrower include path we could use, and then avoid the exclusions below?

- eng/pipelines/common/* - eng/pipelines/pr/*

I guess we could just include common and pr 🤔

codecov · 2026-06-12T01:26:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.33%. Comparing base (fd8cb4b) to head (aed4424).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4360      +/-   ##
==========================================
+ Coverage   65.69%   66.33%   +0.64%     
==========================================
  Files         285      285              
  Lines       43311    66481   +23170     
==========================================
+ Hits        28453    44101   +15648     
- Misses      14858    22380    +7522

Flag	Coverage Δ
CI-SqlClient	`?`
sqlclient-pr	`66.33% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

benrr101 · 2026-06-12T18:34:55Z

@paulmedynski I disagree with most of the copilot comments you agreed with, so ... one of us is misunderstanding something 😅

Regarding the CodeCov stuff, I think I need an adult to tell me how they'd like me to address it.

Copilot

Pull request overview

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

+steps:
+  # Publish the test results (trx files) to the pipeline
+  - task: PublishTestResults@2
+    displayName: 'Publish Test Results'
+    inputs:
+      buildConfiguration: ${{ parameters.buildConfiguration }}
+      mergeTestResults: true
+      testResultsFiles: |
+        ${{ parameters.testResultsPath }}/*.trx
+        ${{ parameters.testResultsPath }}/**/*.coverage
+      testResultsFormat: VSTest
+      testRunTitle: "${{ parameters.platformDisplayName }}_${{ parameters.testDisplayName }}"
+    condition: succeededOrFailed()
+
+  # Publish the test results as artifacts for the pipeline
+  - task: PublishPipelineArtifact@1
+    displayName: 'Publish Test Artifacts'
+    inputs:
+      artifact: ${{ parameters.testResultsArtifactName }}_$(System.JobId)
+      targetPath: ${{ parameters.testResultsPath }}
+    condition: succeededOrFailed()
+


+          - script: |
+              curl -o "${{ variables.workingDir }}/codecov" https://cli.codecov.io/latest/linux/codecov
+              chmod +x "${{ variables.workingDir }}/codecov"
+            displayName: Download CodeCov CLI


+          - script: >-
+              "${{ variables.workingDir }}/codecov"
+              --verbose
+              upload-process
+              --fail-on-error
+              --git-service github
+              --slug dotnet/SqlClient
+              -f "${{ variables.workingDir }}/merge/cobertura.xml"
+              -F $(Build.DefinitionName)
+            displayName: Upload coverage to CodeCov
+            env:
+              CODECOV_TOKEN: ${{ parameters.codeCovApiToken }}


benrr101 added 15 commits June 11, 2026 14:41

First crack at adding test result publishing to non-manual test jobs.

963bccc

# Conflicts: # eng/pipelines/pr/jobs/test-buildproj-job.yml # eng/pipelines/pr/stages/test-stages.yml

If statement syntax?

1ce8fca

Ok forget the if statement.

3e28bcd

Version

824f0d2

Unique job names... could be better, but let's start here.

7bd1466

Tree in linux?

7bc3a20

Ok forget it, we don't need do emit the tree....

3388570

Wire up test result uploading to sqlclient manual test jobs

412f2e0

Give a test display name to sql manual tests

9818712

First attempt at code coverage report publishing

800a688

🤖 Code coverage takes a dynamic dependency on the platforms to ensure…

7056130

… all tests are first.

We only need *one* set of parameters

5acf9e3

We only need *one* pool name

ba57adc

Add triggers and fix code coverage verbosity thingy

f6f4726

🤖 fixes for codecov CLI

2f28f24

benrr101 added this to the 7.1.0-preview2 milestone Jun 11, 2026

benrr101 requested a review from a team as a code owner June 11, 2026 22:17

Copilot AI review requested due to automatic review settings June 11, 2026 22:17

benrr101 added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Jun 11, 2026

github-project-automation Bot added this to SqlClient Board Jun 11, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jun 11, 2026

github-actions Bot assigned apoorvdeshmukh and cheenamalhotra Jun 11, 2026

Copilot started reviewing on behalf of benrr101 June 11, 2026 22:17 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

benrr101 moved this from To triage to In review in SqlClient Board Jun 12, 2026

paulmedynski requested changes Jun 12, 2026

View reviewed changes

github-project-automation Bot moved this from In review to Waiting for customer in SqlClient Board Jun 12, 2026

👨‍🦱 and 🤖 comments

0172b62

benrr101 moved this from Waiting for customer to In review in SqlClient Board Jun 12, 2026

benrr101 requested a review from paulmedynski June 12, 2026 18:34

🤖 Use dotnet-tools.json

aed4424

Copilot AI review requested due to automatic review settings June 12, 2026 19:06

Copilot started reviewing on behalf of benrr101 June 12, 2026 19:07 View session

Copilot AI reviewed Jun 12, 2026

View reviewed changes

Conversation

benrr101 commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

benrr101 commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benrr101 commented Jun 11, 2026 •

edited

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading

benrr101 commented Jun 12, 2026 •

edited

Loading