Skip to content

Tests | Manual Test "Set" Trait#4071

Open
benrr101 wants to merge 10 commits into
mainfrom
dev/russellben/cleanup/testsets-categories
Open

Tests | Manual Test "Set" Trait#4071
benrr101 wants to merge 10 commits into
mainfrom
dev/russellben/cleanup/testsets-categories

Conversation

@benrr101
Copy link
Copy Markdown
Contributor

Description

This PR looks big, but it's actually really small. This updates the manual tests project to include all files in all builds, and moves the TestSet build property to a trait. Filtering of test sets can now be done via the filter command line argument.

Why is this good?

  • Uses existing mechanics to filter test sets, instead of custom baked things.
  • No surprises because files were not included
  • We can use "compiledefaultitems" in the project - no need to update the csproj when adding/removing tests.

Build2.proj now forwards the TestSet parameter to the filter argument, combining it with other filters that were provided. Thus, no changes to the pipelines were necessary.

🤖

Codex did the batch update of the test files (though it applied changes to files that didn't have any tests, so I had to back those changes out).

Testing

Everything works locally, so it should be good in PR validation.

Copilot AI review requested due to automatic review settings March 21, 2026 00:53
@benrr101 benrr101 requested a review from a team as a code owner March 21, 2026 00:53
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Mar 21, 2026

This comment was marked as outdated.

@paulmedynski paulmedynski moved this from To triage to In review in SqlClient Board Mar 23, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview1 milestone Mar 23, 2026
@paulmedynski paulmedynski added the Area\Tests Issues that are targeted to tests or test projects label Mar 23, 2026
Copy link
Copy Markdown
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

Looks like Copilot found some bugs to address. I have a question about backwards compatibility with build.proj.

@github-project-automation github-project-automation Bot moved this from In review to In progress in SqlClient Board Mar 23, 2026
@paulmedynski paulmedynski self-assigned this Mar 23, 2026
Copilot AI review requested due to automatic review settings March 24, 2026 19:39

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

I agree with the Copilot comments.

@paulmedynski paulmedynski force-pushed the dev/russellben/cleanup/testsets-categories branch from 892b3de to 85da1f8 Compare May 5, 2026 15:25
@paulmedynski paulmedynski moved this from In progress to In review in SqlClient Board May 5, 2026
@paulmedynski paulmedynski enabled auto-merge (squash) May 5, 2026 15:28
@paulmedynski paulmedynski dismissed their stale review May 5, 2026 15:36

Clearing since my changes have been implemented. Waiting for author to reply before approving.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.73%. Comparing base (bfbdd30) to head (52b0459).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4071      +/-   ##
==========================================
- Coverage   66.69%   63.73%   -2.96%     
==========================================
  Files         284      279       -5     
  Lines       43238    66069   +22831     
==========================================
+ Hits        28836    42112   +13276     
- Misses      14402    23957    +9555     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 63.73% <ø> (?)

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.

@paulmedynski paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label May 11, 2026
Copy link
Copy Markdown
Contributor

@apoorvdeshmukh apoorvdeshmukh left a comment

Choose a reason for hiding this comment

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

@benrr101 Can you resolve the branch conflicts? I think this one is good to be merged.

Copy link
Copy Markdown
Contributor

@apoorvdeshmukh apoorvdeshmukh 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!
I should have approved this PR first before the breakup bulkcopy test one. Now we have branch conflicts.
@benrr101 Can you address it?

@apoorvdeshmukh apoorvdeshmukh added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 3, 2026
Copilot AI review requested due to automatic review settings June 3, 2026 17:31
@benrr101 benrr101 removed the Author attention needed PRs that require author to respond or make updates to PR. label Jun 3, 2026

This comment was marked as outdated.

paulmedynski
paulmedynski previously approved these changes Jun 3, 2026
apoorvdeshmukh
apoorvdeshmukh previously approved these changes Jun 3, 2026
@paulmedynski paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 4, 2026
Copilot AI review requested due to automatic review settings June 4, 2026 17:00
@benrr101 benrr101 dismissed stale reviews from apoorvdeshmukh and paulmedynski via 52b0459 June 4, 2026 17:00
@benrr101 benrr101 removed the Author attention needed PRs that require author to respond or make updates to PR. label Jun 4, 2026
Copy link
Copy Markdown
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 167 out of 177 changed files in this pull request and generated 2 comments.

paulmedynski
paulmedynski previously approved these changes Jun 4, 2026
Copy link
Copy Markdown
Contributor

@paulmedynski paulmedynski left a comment

Choose a reason for hiding this comment

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

Now that we're compiling everything regardless of "test set", I'm not too concerned which test set a test decides to belong to. We're running them all anyway. I feel like we're close to abolishing the test set concept entirely, and separating them into parallelizable chunks based on different criteria anyway.

@paulmedynski paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 5, 2026
@paulmedynski paulmedynski moved this from In review to Waiting for customer in SqlClient Board Jun 5, 2026
@benrr101 benrr101 removed the Author attention needed PRs that require author to respond or make updates to PR. label Jun 5, 2026
@benrr101 benrr101 moved this from Waiting for customer to In review in SqlClient Board Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Tests Issues that are targeted to tests or test projects

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

5 participants