-
Notifications
You must be signed in to change notification settings - Fork 311
Commit 9056f13
authored
Downgrade Xunit, Improve exception handling for Diagnonstic Tests (#2379)
**Description**: While doing some investigation into improving the diagnostic tracing tests, I noticed that test failures are logged very non-descriptly by remote executor. To resolve this issue, we have to downgrade xunit to v2.4.2. Here's the rationale for this change:
* RemoteExecutor throws `XunitException`s from xunit.assert nuget and uses the `(string, string)` constructor
* The `(string, string)` constructor was removed in >=2.5 of xunit.assert
* RemoteExecutor was fixed to inherit from Exception in newer versions, but for us to take this requires upgrading to .net 7 or 8 (which I imagine is a much larger lift)
* In order to downgrade xunit.assert, we need to downgrade xunit
* In order to downgrade xunit, we also need to downgrade Microsoft.DotNet.XunitExtensions
* Without this change, exceptions from RemoteExecutor will fail to construct, masking all errors with "MethodNotFound"
Although normally downgrading is bad for security reasons, I believe this to be a safe change since it only affects tests.
This PR also makes a bunch of improvements to the `DiagnosticTest`s:
* Remove conditional facts when test TDS server is used
* Remove try/catch with empty catch block, replace with assert.throws
* Ensure that connections are opened before trying test conditions
* Document scenarios where test TDS server doesn't obey actual behavior, or where test TDS server cannot be used
* Formatting improvements
**Testing**:
* All tests (that are expected to pass) continue to pass
* In a synthetic failure of diagnostic tests, here's the before and after of the error messages:
* Before:
```
System.MissingMethodException: Method not found: 'Void Xunit.Sdk.XunitException..ctor(System.String, System.String)'.
System.MissingMethodException
Method not found: 'Void Xunit.Sdk.XunitException..ctor(System.String, System.String)'.
```
* After:
```
Microsoft.DotNet.RemoteExecutor.RemoteExecutionException: Remote process failed with an unhandled exception.
Microsoft.DotNet.RemoteExecutor.RemoteExecutionException
Remote process failed with an unhandled exception.
Child exception:
Xunit.Sdk.FailException: Assert.Fail(): Foo
at Microsoft.Data.SqlClient.ManualTesting.Tests.DiagnosticTest.<>c.<SyntheticFailure>b__1_0() in
C:\Projects\benrr101\sqlclient\src\Microsoft.Data.SqlClient\tests\ManualTests\TracingTests\DiagnosticTest.cs:line 34
```
## Commits
* Downgrade xunit for better error logging from remote executor
* All but one test improved ... ?
* Understanding why test was failing
* Add arcade and xunit nugets to .net8+ special versions
* Remove xunitextensions version boost (which I spelled wrong anyhow)1 parent 769b982 commit 9056f13Copy full SHA for 9056f13
File tree
Expand file treeCollapse file tree
3 files changed
+115
-134
lines changedFilter options
- src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests
- tools/props
Expand file treeCollapse file tree
3 files changed
+115
-134
lines changed
0 commit comments