[6.1] Port Flaky Test Separation by benrr101 · Pull Request #4060 · dotnet/SqlClient

benrr101 · 2026-03-17T22:30:57Z

Description

Since 6.1 branch already has the flaky test traits, this PR just updates the PR pipeline to run tests w/o flaky tests, then run only flaky tests, ignoring the results of the latter.

🤖

Codex ported these changes from the branch for #4059

Testing

These changes only apply to PR pipelines. They will be validated here.

Copilot

Pull request overview

This PR updates the shared PR pipeline test template to run additional “flaky-only” Functional and Manual test passes (and ignore failures from those passes) to help separate flaky test failures from main PR signal.

Changes:

Adds “Run Flaky Functional Tests” and “Run Flaky Manual Tests” steps for both Windows (MSBuild@1) and non-Windows (DotNetCoreCLI@2).
Adjusts test-step conditions to succeededOrFailed() so later test steps still execute even when earlier steps fail.
Marks flaky-only steps with continueOnError: true to ignore flaky test failures.

eng/pipelines/common/templates/steps/run-all-tests-step.yml

Copilot

Pull request overview

This PR aims to port the “flaky test separation” behavior into the 6.1 PR pipeline, so CI runs non-flaky tests normally and then runs flaky tests separately without failing the PR.

Changes:

Updates the shared run-all-tests-step.yml template to add dedicated “Run Flaky * Tests” steps that continue on error.
Adjusts pipeline step conditions to run subsequent test phases even if an earlier phase fails.
Updates ConnectionFailoverTests attributes/traits related to the flaky category.

Reviewed changes

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

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs	Trait/attribute adjustments for flaky categorization in this unit test class.
eng/pipelines/common/templates/steps/run-all-tests-step.yml	Adds separate flaky test executions for unit/functional/manual tests and tweaks step conditions/behavior.

eng/pipelines/common/templates/steps/run-all-tests-step.yml

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs

eng/pipelines/common/templates/steps/run-all-tests-step.yml

paulmedynski · 2026-04-02T19:04:19Z

@benrr101 - Code freeze for 6.1.5 will be Tue Apr 7. Will this change be complete by then?

- Add <Filter> MSBuild property with default 'category!=failing&category!=flaky' so that -p:Filter=... from the pipeline is honored by dotnet test - Update all 6 test targets (Unit/Functional/Manual x Windows/Unix) to use $(Filter) instead of hard-coded filter strings - Fix typo in non-Windows flaky unit test step: -p:=Test... -> -p:Test... - Remove redundant [Trait("Category", "flaky")] on method already covered by class-level trait

Copilot

Pull request overview

Ports the “flaky test separation” approach to the 6.1 branch by wiring an MSBuild-level test filter and updating the PR pipeline to run non-flaky tests first, then flaky-only tests with failures ignored.

Changes:

Add a Filter MSBuild property in build.proj (defaulting to excluding flaky) and apply it across Unit/Functional/Manual test targets.
Update the shared pipeline test step template to run each test suite twice: non-flaky first, then flaky-only with continueOnError: true.
Mark the XEvents tracing manual test as flaky and add an xUnit fixture that attempts to clean up orphaned XEvent sessions.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs	Keeps the class categorized as flaky; minor attribute ordering change.
src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs	Adds XEvent cleanup fixture and marks the test as flaky.
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs	Adds a note about test-name parsing variability.
eng/pipelines/common/templates/steps/run-all-tests-step.yml	Splits pipeline test execution into non-flaky and flaky-only runs (flaky failures ignored).
build.proj	Introduces `$(Filter)` to control `dotnet test --filter` consistently across test targets.

Copilot · 2026-04-07T00:13:05Z

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs

+                    using SqlCommand dropCommand = new(
+                        $"DROP EVENT SESSION [{sessionName}] ON DATABASE;",
+                        connection);


sessionName is interpolated into an identifier in DROP EVENT SESSION [...] without escaping. If a session name contains ] (or other special characters), this can break the statement and can potentially allow SQL injection via crafted object names. Use a safe identifier-quoting approach (e.g., QUOTENAME in dynamic SQL, or escape ] to ]] before bracket-quoting) before executing the DROP.

Copilot · 2026-04-07T00:13:05Z

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs

            _testName = DataTestUtility.CurrentTestName(outputHelper);
        }

+        [Trait("Category", "flaky")]
        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.IsNotManagedInstance))]


PR description says this change "just updates the PR pipeline", but this PR also changes test code here (adds a collection fixture and marks the test Category=flaky). Consider updating the PR description to reflect these additional behavioral changes so they aren’t overlooked during review/release notes.

Copilot · 2026-04-07T00:13:05Z

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs

+                    SELECT Sessions.name
+                    FROM sys.database_event_sessions Sessions
+                    LEFT JOIN sys.dm_xe_database_sessions Active ON Sessions.name = Active.name
+                    WHERE Active.name IS NULL;
+                    """,


The cleanup query selects all inactive/orphaned database XEvent sessions and will drop them. That can delete sessions not created by this test suite (e.g., user-created or intentionally stopped sessions) when manual tests are run against a shared Azure SQL database. Consider restricting the query to sessions created by these tests (e.g., a known name prefix/pattern).

Copilot · 2026-04-07T00:13:06Z

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/XEventsTracingTest.cs

+                    }
+                    catch (SqlException)
+                    {
+                        // Ignore exceptions, as the session may have been cleaned up by another
+                        // test run at the same time.
+                    }
+
+                    Console.WriteLine($"  Dropped orphaned XEvent session: {sessionName}");


The code always logs "Dropped orphaned XEvent session" even when dropCommand.ExecuteNonQuery() throws (the log is outside the try/catch). This produces misleading output. Only log a successful drop when no exception occurred (or emit a different message on failure).

Suggested change

}

catch (SqlException)

{

// Ignore exceptions, as the session may have been cleaned up by another

// test run at the same time.

}

Console.WriteLine($" Dropped orphaned XEvent session: {sessionName}");

Console.WriteLine($" Dropped orphaned XEvent session: {sessionName}");

}

catch (SqlException)

{

// Ignore exceptions, as the session may have been cleaned up by another

// test run at the same time.

Console.WriteLine($" Could not drop orphaned XEvent session: {sessionName}");

}

Port pipeline changes for flaky tests to 6.1

89c94a2

benrr101 added this to the 6.1.5 milestone Mar 17, 2026

benrr101 requested a review from a team as a code owner March 17, 2026 22:30

Copilot AI review requested due to automatic review settings March 17, 2026 22:30

benrr101 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 Mar 17, 2026

github-project-automation bot added this to SqlClient Board Mar 17, 2026

github-project-automation bot moved this to To triage in SqlClient Board Mar 17, 2026

Copilot started reviewing on behalf of benrr101 March 17, 2026 22:31 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

mdaigle requested changes Mar 18, 2026

View reviewed changes

eng/pipelines/common/templates/steps/run-all-tests-step.yml Show resolved Hide resolved

github-project-automation bot moved this from To triage to In progress in SqlClient Board Mar 18, 2026

Flaky unit tests

ecda665

mdaigle reviewed Mar 18, 2026

View reviewed changes

eng/pipelines/common/templates/steps/run-all-tests-step.yml Show resolved Hide resolved

Run flaky unit tests on windows too

cc60401

Copilot AI review requested due to automatic review settings March 19, 2026 16:44

Copilot started reviewing on behalf of benrr101 March 19, 2026 16:44 View session

Copilot AI reviewed Mar 19, 2026

View reviewed changes

mdaigle reviewed Mar 19, 2026

View reviewed changes

eng/pipelines/common/templates/steps/run-all-tests-step.yml Show resolved Hide resolved

paulmedynski self-assigned this Mar 19, 2026

paulmedynski requested changes Mar 19, 2026

View reviewed changes

paulmedynski previously approved these changes Apr 6, 2026

View reviewed changes

Make XEvents flaky and bring over xevents cleaner

4cd65e6

Copilot AI review requested due to automatic review settings April 7, 2026 00:07

benrr101 dismissed paulmedynski’s stale review via 4cd65e6 April 7, 2026 00:07

Copilot started reviewing on behalf of benrr101 April 7, 2026 00:08 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Conversation

benrr101 commented Mar 17, 2026

Description

🤖

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulmedynski commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants