Skip to content

Conversation

@paulmedynski
Copy link
Contributor

Motivation

Our PR pipelines are taking too long to run, for many reasons that we can't fix all at once. The low hanging fruit right now is that we are running all MDS tests twice - once in the Project pipeline, and once in the Package pipeline. This is unnecessary, so we're splitting things up:

  • Project runs will perform unit tests (i.e. MDS Unit and Functional suites).
  • Package runs will perform integration tests (i.e. MDS Manual suite).

Reasoning

  • The MDS Unit test suite requires Project reference builds anyway, so it makes sense to keep it in that pipeline.
  • The MDS Functional tests are (in theory) unit tests in disguise, and they don't typically take very long to run, so lumping them in with the Unit test suite seems fine.
  • The Manual tests take a long time, and they are intended to be more black-box, hence Package-based builds make sense for them.
  • The other pipelines (CI, downstream in ADO.Net, etc) will continue to run all tests multiple times).

Testing

PR pipeline runs will confirm that tests are still passing. I will manually verify that the expected types of tests are running in the designated pipelines.

  - Project runs will perform unit tests (i.e. MDS Unit and Functional suites).
  - Package runs will perform integration tests (i.e. MDS Manual suite).
@paulmedynski paulmedynski requested a review from a team as a code owner January 21, 2026 15:58
Copilot AI review requested due to automatic review settings January 21, 2026 15:58
@paulmedynski paulmedynski added Area\Tests Issues that are targeted to tests or test projects Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. labels Jan 21, 2026
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 pull request optimizes CI/CD pipeline execution by separating unit and integration tests to reduce redundant test execution. The PR introduces a testType parameter that controls which test suites run in different pipeline contexts.

Changes:

  • Added testType parameter ('All', 'Unit', or 'Integration') to pipeline templates
  • Modified PR pipelines to run only relevant test suites (Unit tests in Project builds, Integration tests in Package builds)
  • Updated test execution conditionals to respect the new testType parameter

Reviewed changes

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

Show a summary per file
File Description
eng/pipelines/sqlclient-pr-project-ref-pipeline.yml Sets testType to 'Unit' for project reference builds, restricting to unit and functional tests
eng/pipelines/sqlclient-pr-package-ref-pipeline.yml Sets testType to 'Integration' for package reference builds, restricting to manual tests
eng/pipelines/dotnet-sqlclient-ci-core.yml Defines testType parameter with default 'All' for CI pipelines to continue running all tests
eng/pipelines/common/templates/steps/run-all-tests-step.yml Implements conditional test execution based on testType parameter
eng/pipelines/common/templates/stages/ci-run-tests-stage.yml Propagates testType parameter through stage template
eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Propagates testType parameter through job template

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.84%. Comparing base (77f79e0) to head (190fcb7).
⚠️ Report is 2 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (77f79e0) and HEAD (190fcb7). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (77f79e0) HEAD (190fcb7)
netfx 2 1
netcore 2 1
addons 2 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3907       +/-   ##
===========================================
- Coverage   76.90%   28.84%   -48.07%     
===========================================
  Files         269      263        -6     
  Lines       43246    66170    +22924     
===========================================
- Hits        33260    19089    -14171     
- Misses       9986    47081    +37095     
Flag Coverage Δ
addons ?
netcore 28.02% <ø> (-48.92%) ⬇️
netfx 29.97% <ø> (-46.40%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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
benrr101 previously approved these changes Jan 21, 2026
Copy link
Contributor

@benrr101 benrr101 left a comment

Choose a reason for hiding this comment

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

Overall, probably a good change. The main issue I have right now is the lumping of functional tests with unit tests. My understanding of them were that they were not unit tests in theory, they were just the simple integration tests that validate they do the most basic stuff. As it turns out a lot of those are unit tests, but not the majority. I was working on the side a bit to move the unit tests from the functional tests and into the unit test project. Ultimately I think we're aligned that we want: unit test, local integration test, remote integration test, stress test, and fuzz test projects. And this change is kinda orthogonal to those goals, without getting in the way of them. So go for it 😆

I think another low-ish hanging fruit is to separate the unit tests and functional tests as separate jobs, like the test sets. That way we don't need to repeat unit and functional tests four times for each platform. They're fast, yes, but not instantaneous, and I imagine we could shave a decent amount of time off a build doing it.

@paulmedynski
Copy link
Contributor Author

You're correct on all counts. I chose to combine the running of Unit and Functional tests because they're mostly unit with some local integration sprinkled in, but mainly because their combined run time is close to the Manual ones. This gives us the most bang-for-the-buck in terms of speeding up the PR runs right now.

We also shouldn't be running Unit and Functional tests in any of the Azure SQL configurations - another future optimization.


# Include the code coverage job if the build type is Project.
${{ if eq(parameters.buildType, 'Project') }}:
# Include the code coverage job under certain circumstances:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will continue to publish code coverage the same as we did before the split... for now. This is fragile and we should re-think how/when/why to publish coverage as we re-write these pipelines.

Copilot AI review requested due to automatic review settings January 22, 2026 00:29
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 7 out of 7 changed files in this pull request and generated 2 comments.

@priyankatiwari08 priyankatiwari08 self-assigned this Jan 22, 2026
Copy link
Contributor

@priyankatiwari08 priyankatiwari08 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Manual tests in itself takes quite significant amount of time to run. So, this segregation should help us in longer run.

Copilot AI review requested due to automatic review settings January 22, 2026 18:46
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 7 out of 7 changed files in this pull request and generated 2 comments.

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

Labels

Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. Area\Tests Issues that are targeted to tests or test projects

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants