Adjust TestSqlCommandCancel condition to reflect key store config requirement

[David-Engel]

Description

Adjusts the ConditionalTheory on TestSqlCommandCancel to only run if AE is configured with an appropriate key store for Windows and non-Windows. The Kerberos test pipeline doesn't use/configure AKV and due to recent changes to this test, it now fails in that pipeline on the Linux job (no AKV configured). This change skips the test on Linux if AKV isn't configured.

[Copilot]

Pull Request Overview

This PR adjusts the conditional test execution for TestSqlCommandCancel to require proper key store configuration for Always Encrypted (AE) testing. The change addresses test failures in the Kerberos pipeline on Linux where Azure Key Vault (AKV) is not configured.

• Updates the ConditionalTheory attribute to use a more specific condition that checks for key store availability
• Prevents test execution on platforms without proper AE key store configuration

[benrr101]

We need to come up with a better strategy for how to do these conditional tests, but working within the constraints we have right now, this makes sense to me.

[David-Engel]

We need to come up with a better strategy for how to do these conditional tests, but working within the constraints we have right now, this makes sense to me.

@benrr101
Can you elaborate on what you think is wrong with the current strategy? The conditions are supposed to define test environment config requirements and skip the tests if the config is not satisfied.

In the case of this test, I could see from the history how it was "broken" by a recent change. Before the recent change, the condition excluded the test from .NET, which meant .NET Framework only, which meant Windows-only, which meant a the Windows Cert Store (any key store is the test requirement) was always available when the test ran. The recent change enabled the test for .NET, cross-platform. Most of our pipelines are set with most options configured to maximize coverage, which meant .NET and Linux jobs always had AKV configured. The Kerberos pipeline is more optimized to just those configs required for Kerberos testing, which resulted in this test failing because its condition wasn't specifically looking for a key store to be configured.

[benrr101]

@David-Engel it's more a general purpose comment not specific to this test. One big problem we have currently is that checking conditions has to happen for every test, which means for tests that eg, require a connection to not be to synapse, it has to eg, connect and execute a query to the db to validate. If that fails, the test shows as failing, but it's incredibly unclear why. And of course all that overhead of hitting the server for each test slows down the tests.

One proposal @paulmedynski and I have discussed is having different connection string settings for the the different kinds of tests we want to run (eg, a synapse connection string, an ae connection string, a regular connection string), and operate under the assumption that if the string exists, it's connecting to the right kind of server for the test.

One other things we've been discussing is that skipping and categorizing tests by platform isn't ideal. The ideal would be to just not even compile them if they're not compatible with the platform, so wrapping them in #if for the platform (supported for .net platform but will need to wait until the common project for os platform) would be better.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.64%. Comparing base (55cc333) to head (2570900).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3524      +/-   ##
==========================================
+ Coverage   59.44%   66.64%   +7.19%     
==========================================
  Files         268      274       +6     
  Lines       62160    62478     +318     
==========================================
+ Hits        36950    41637    +4687     
+ Misses      25210    20841    -4369     

Flag	Coverage Δ
addons	90.82% <ø> (?)
netcore	68.56% <ø> (+6.34%)	⬆️
netfx	69.15% <ø> (+5.86%)	⬆️

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.