Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix | Minor fixes to support different test environments #2045

Merged
merged 5 commits into from
Jun 2, 2023

Conversation

David-Engel
Copy link
Contributor

Mostly test fixes with one managed SNI, SSRP failure case fix.

  • Accommodate tests running in a Kerberos domain, remote server environment using named instances.
  • AE tests need to check for key store availability when they require one.
  • SPN test shouldn't assume the server is running locally.

The single, minor bug fix in SSRP was around failure cases. An AggregateException itself generally shouldn't bubble up as an SSRP failure. Since we might be trying multiple IP addresses, some might be bad and some might be good. SocketException failures should result in the same "Error Locating Server/Instance Specified" error that empty SSRP responses result in.

- Set correct condition on AE tests that require an available key store (Cert Store on Windows, else AKV)
- ServerSPN test should get FQDN of target, not local machine
- Instance name test shouldn't try to connect via IP if using integrated security in the context of a remote domain-based connection (Kerberos fails)
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.15 ⚠️

Comparison is base (f478be5) 70.97% compared to head (e9055f0) 69.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2045      +/-   ##
==========================================
- Coverage   70.97%   69.83%   -1.15%     
==========================================
  Files         305      305              
  Lines       61807    61820      +13     
==========================================
- Hits        43870    43172     -698     
- Misses      17937    18648     +711     
Flag Coverage Δ
addons 92.88% <ø> (ø)
netcore 73.29% <0.00%> (-0.32%) ⬇️
netfx 68.11% <ø> (-1.50%) ⬇️

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

Impacted Files Coverage Δ
...t/netcore/src/Microsoft/Data/SqlClient/SNI/SSRP.cs 51.51% <0.00%> (-4.78%) ⬇️
...nt/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs 39.20% <ø> (+0.05%) ⬆️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@David-Engel David-Engel added this to the 5.2.0-preview2 milestone May 30, 2023
@DavoudEshtehari DavoudEshtehari added Area\Netcore Issues that are apply only to .NET runtime or the 'netcore' project folder. Area\Tests Issues that are targeted to tests or test projects labels May 31, 2023
@David-Engel David-Engel merged commit de6390f into dotnet:main Jun 2, 2023
@David-Engel David-Engel deleted the TestFixes branch June 2, 2023 17:22
kant2002 pushed a commit to kant2002/SqlClient that referenced this pull request Jun 29, 2023
* Test Fixes:
- Set correct condition on AE tests that require an available key store (Cert Store on Windows, else AKV)
- ServerSPN test should get FQDN of target, not local machine
- Instance name test shouldn't try to connect via IP if using integrated security in the context of a remote domain-based connection (Kerberos fails)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area\Netcore Issues that are apply only to .NET runtime or the 'netcore' project folder. Area\Tests Issues that are targeted to tests or test projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants