-
Notifications
You must be signed in to change notification settings - Fork 314
Fix xUnit Theory serialization warnings #3186
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
Changes from all commits
ae09fcc
44b29be
034522f
430778e
5c5fa2e
334959b
e928eb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,21 +178,109 @@ | |
</Target> | ||
|
||
<!-- Tests --> | ||
|
||
<!-- Run all unit tests applicable to the host OS. --> | ||
<Target Name="RunTests" DependsOnTargets="RunFunctionalTests;RunManualTests"/> | ||
<Target Name="RunFunctionalTests"> | ||
<!-- Windows --> | ||
<Exec ConsoleToMsBuild="true" Command="$(DotnetPath)dotnet test "@(FunctionalTestsProj)" -p:Configuration=$(Configuration) -p:Target$(TFGroup)Version=$(TF) -p:ReferenceType=$(ReferenceType) --no-build -v n --collect "Code coverage" -p:TestSet=$(TestSet) --results-directory $(ResultsDirectory) -p:TestTargetOS=Windows$(TargetGroup) --filter "category!=non$(TargetGroup)tests&category!=failing&category!=nonwindowstests" "--logger:trx;LogFilePrefix=Functional-Windows$(TargetGroup)-$(TestSet)"" Condition="'$(IsEnabledWindows)' == 'true'"/> | ||
<!-- Unix --> | ||
<Exec ConsoleToMsBuild="true" Command="$(DotnetPath)dotnet test "@(FunctionalTestsProj)" -p:Configuration=$(Configuration) -p:TargetNetCoreVersion=$(TF) -p:ReferenceType=$(ReferenceType) --no-build -v n -p:TestSet=$(TestSet) --results-directory $(ResultsDirectory) -p:TestTargetOS=Unixnetcoreapp --collect "Code coverage" --filter "category!=nonnetcoreapptests&category!=failing&category!=nonlinuxtests&category!=nonuaptests" "--logger:trx;LogFilePrefix=Functional-Unixnetcoreapp-$(TestSet)"" Condition="'$(IsEnabledWindows)' != 'true'"/> | ||
|
||
<!-- Run all Functional tests applicable to the host OS. --> | ||
<Target Name="RunFunctionalTests" DependsOnTargets="RunFunctionalTestsWindows;RunFunctionalTestsUnix"/> | ||
|
||
<!-- Run all Functional tests applicable to Windows. --> | ||
<Target Name="RunFunctionalTestsWindows" Condition="'$(IsEnabledWindows)' == 'true'"> | ||
<PropertyGroup> | ||
<TestCommand> | ||
$(DotnetPath)dotnet test "@(FunctionalTestsProj)" | ||
--no-build | ||
-v n | ||
-p:Configuration=$(Configuration) | ||
-p:Target$(TFGroup)Version=$(TF) | ||
-p:ReferenceType=$(ReferenceType) | ||
-p:TestSet=$(TestSet) | ||
-p:TestTargetOS=Windows$(TargetGroup) | ||
--collect "Code coverage" | ||
--results-directory $(ResultsDirectory) | ||
--filter "category!=non$(TargetGroup)tests&category!=failing&category!=nonwindowstests" | ||
--logger:"trx;LogFilePrefix=Functional-Windows$(TargetGroup)-$(TestSet)" | ||
</TestCommand> | ||
<TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modern dotnet tooling supports String.ReplaceLineEndings(), but we're using the old .NET Framework msbuild.exe tooling to build, so we can't have the nice toys :( |
||
</PropertyGroup> | ||
<Message Text=">>> Running Functional test for Windows via command: $(TestCommand)"/> | ||
<Exec ConsoleToMsBuild="true" Command="$(TestCommand)"/> | ||
</Target> | ||
|
||
<!-- Run all Functional tests applicable to Unix. --> | ||
<Target Name="RunFunctionalTestsUnix" Condition="'$(IsEnabledWindows)' != 'true'"> | ||
<PropertyGroup> | ||
<TestCommand> | ||
$(DotnetPath)dotnet test "@(FunctionalTestsProj)" | ||
--no-build | ||
-v n | ||
-p:Configuration=$(Configuration) | ||
-p:TargetNetCoreVersion=$(TF) | ||
-p:ReferenceType=$(ReferenceType) | ||
-p:TestSet=$(TestSet) | ||
-p:TestTargetOS=Unixnetcoreapp | ||
--collect "Code coverage" | ||
--results-directory $(ResultsDirectory) | ||
--filter "category!=nonnetcoreapptests&category!=failing&category!=nonlinuxtests&category!=nonuaptests" | ||
--logger:"trx;LogFilePrefix=Functional-Unixnetcoreapp-$(TestSet)" | ||
</TestCommand> | ||
<TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand> | ||
</PropertyGroup> | ||
<Message Text=">>> Running Functional test for Unix via command: $(TestCommand)"/> | ||
<Exec ConsoleToMsBuild="true" Command="$(TestCommand)"/> | ||
</Target> | ||
|
||
<!-- Run all Manual tests applicable to the host OS. --> | ||
<Target Name="RunManualTests" DependsOnTargets="RunManualTestsWindows;RunManualTestsUnix"/> | ||
|
||
<!-- Run all Manual tests applicable to Windows. --> | ||
<Target Name="RunManualTestsWindows" Condition="'$(IsEnabledWindows)' == 'true'"> | ||
<PropertyGroup> | ||
<TestCommand> | ||
$(DotnetPath)dotnet test "@(ManualTestsProj)" | ||
--no-build | ||
-v n | ||
-p:Configuration=$(Configuration) | ||
-p:Target$(TFGroup)Version=$(TF) | ||
-p:ReferenceType=$(ReferenceType) | ||
-p:TestSet=$(TestSet) | ||
-p:TestTargetOS=Windows$(TargetGroup) | ||
--collect "Code coverage" | ||
--results-directory $(ResultsDirectory) | ||
--filter "category!=non$(TargetGroup)tests&category!=failing&category!=nonwindowstests" | ||
--logger:"trx;LogFilePrefix=Manual-Windows$(TargetGroup)-$(TestSet)" | ||
</TestCommand> | ||
<TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand> | ||
</PropertyGroup> | ||
<Message Text=">>> Running Manual test for Windows via command: $(TestCommand)"/> | ||
<Exec ConsoleToMsBuild="true" Command="$(TestCommand)"/> | ||
Check failure on line 257 in build.proj
|
||
</Target> | ||
|
||
<Target Name="RunManualTests"> | ||
<!-- Windows --> | ||
<Exec ConsoleToMsBuild="true" Command="$(DotnetPath)dotnet test "@(ManualTestsProj)" -p:Configuration=$(Configuration) -p:Target$(TFGroup)Version=$(TF) -p:ReferenceType=$(ReferenceType) --no-build -l "console;verbosity=normal" --collect "Code coverage" -p:TestSet=$(TestSet) --results-directory $(ResultsDirectory) -p:TestTargetOS=Windows$(TargetGroup) --filter "category!=non$(TargetGroup)tests&category!=failing&category!=nonwindowstests" "--logger:trx;LogFilePrefix=Manual-Windows$(TargetGroup)-$(TestSet)"" Condition="'$(IsEnabledWindows)' == 'true'"/> | ||
<!-- Unix --> | ||
<Exec ConsoleToMsBuild="true" Command="$(DotnetPath)dotnet test "@(ManualTestsProj)" -p:Configuration=$(Configuration) -p:TargetNetCoreVersion=$(TF) -p:ReferenceType=$(ReferenceType) --no-build -l "console;verbosity=normal" --collect "Code coverage" -p:TestSet=$(TestSet) --results-directory $(ResultsDirectory) -p:TestTargetOS=Unixnetcoreapp --filter "category!=nonnetcoreapptests&category!=failing&category!=nonlinuxtests&category!=nonuaptests" "--logger:trx;LogFilePrefix=Manual-Unixnetcoreapp-$(TestSet)"" Condition="'$(IsEnabledWindows)' != 'true'"/> | ||
<!-- Run all Manual tests applicable to Unix. --> | ||
<Target Name="RunManualTestsUnix" Condition="'$(IsEnabledWindows)' != 'true'"> | ||
<PropertyGroup> | ||
<TestCommand> | ||
$(DotnetPath)dotnet test "@(ManualTestsProj)" | ||
--no-build | ||
-v n | ||
-p:Configuration=$(Configuration) | ||
-p:TargetNetCoreVersion=$(TF) | ||
-p:ReferenceType=$(ReferenceType) | ||
-p:TestSet=$(TestSet) | ||
-p:TestTargetOS=Unixnetcoreapp | ||
--collect "Code coverage" | ||
--results-directory $(ResultsDirectory) | ||
--filter "category!=nonnetcoreapptests&category!=failing&category!=nonlinuxtests&category!=nonuaptests" | ||
--logger:"trx;LogFilePrefix=Manual-Unixnetcoreapp-$(TestSet)" | ||
</TestCommand> | ||
<TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand> | ||
</PropertyGroup> | ||
<Message Text=">>> Running Manual test for Unix via command: $(TestCommand)"/> | ||
<Exec ConsoleToMsBuild="true" Command="$(TestCommand)"/> | ||
</Target> | ||
|
||
<!-- Clean --> | ||
<Target Name="Clean"> | ||
<RemoveDir Directories='$([System.IO.Directory]::GetDirectories(".","artifacts", SearchOption.AllDirectories))' /> | ||
<RemoveDir Directories='$([System.IO.Directory]::GetDirectories(".","bin", SearchOption.AllDirectories))' /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,7 +230,16 @@ public void EncryptAndDecryptDataSuccessfully() | |
} | ||
|
||
[Theory] | ||
[CEKEncryptionReversalParameters] | ||
[MemberData( | ||
nameof(CEKEncryptionReversalData) | ||
#if NETFRAMEWORK | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .NET doesn't have a GAC, so these tests will enumerate correctly for those targets. We only need to disable for .NET Framework targets. |
||
// .NET Framework puts system enums in something called the Global | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've left a comment once in each file where we're using DisableDiscoveryEnumeration, to make it clear why we're using that flag. |
||
// Assembly Cache (GAC), and xUnit refuses to serialize enums that | ||
// live there. So for .NET Framework, we disable enumeration of the | ||
// test data to avoid warnings on the console when running tests. | ||
, DisableDiscoveryEnumeration = true | ||
#endif | ||
)] | ||
[PlatformSpecific(TestPlatforms.Windows)] | ||
public void TestCEKEncryptionReversal(StoreLocation certificateStoreLocation, String certificateStoreNameAndLocation) | ||
{ | ||
|
@@ -381,8 +390,13 @@ public void TestCustomKeyProviderListSetter() | |
} | ||
|
||
[Theory] | ||
[MemberData( | ||
nameof(ValidCertificatePathsData) | ||
#if NETFRAMEWORK | ||
, DisableDiscoveryEnumeration = true | ||
#endif | ||
)] | ||
[PlatformSpecific(TestPlatforms.Windows)] | ||
[ValidCertificatePathsParameters] | ||
public void TestValidCertificatePaths(string certificateStoreNameAndLocation, object location) | ||
{ | ||
StoreLocation certificateStoreLocation; | ||
|
@@ -505,33 +519,25 @@ public override IEnumerable<Object[]> GetData(MethodInfo testMethod) | |
} | ||
} | ||
|
||
public class CEKEncryptionReversalParameters : DataAttribute | ||
public static IEnumerable<object[]> CEKEncryptionReversalData() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since xUnit only supports DisableDiscoveryEnumeration on MemberData attributes, I have converted all of the data sources to be compatible with MemberData. You will see similar changes throughout the tests, where data source classes become methods. |
||
{ | ||
public override IEnumerable<Object[]> GetData(MethodInfo testMethod) | ||
yield return new object[2] { StoreLocation.CurrentUser, CurrentUserMyPathPrefix }; | ||
// use localmachine cert path only when current user is Admin. | ||
if (ColumnEncryptionCertificateFixture.IsAdmin) | ||
{ | ||
yield return new object[2] { StoreLocation.CurrentUser, CurrentUserMyPathPrefix }; | ||
// use localmachine cert path only when current user is Admin. | ||
if (ColumnEncryptionCertificateFixture.IsAdmin) | ||
{ | ||
yield return new object[2] { StoreLocation.LocalMachine, LocalMachineMyPathPrefix }; | ||
} | ||
yield return new object[2] { StoreLocation.LocalMachine, LocalMachineMyPathPrefix }; | ||
} | ||
} | ||
|
||
|
||
public class ValidCertificatePathsParameters : DataAttribute | ||
public static IEnumerable<object[]> ValidCertificatePathsData() | ||
{ | ||
|
||
public override IEnumerable<Object[]> GetData(MethodInfo testMethod) | ||
yield return new object[2] { CurrentUserMyPathPrefix, StoreLocation.CurrentUser }; | ||
// use localmachine cert path (or an incomplete path, which defaults to localmachine) only when current user is Admin. | ||
if (ColumnEncryptionCertificateFixture.IsAdmin) | ||
{ | ||
yield return new object[2] { CurrentUserMyPathPrefix, StoreLocation.CurrentUser }; | ||
// use localmachine cert path (or an incomplete path, which defaults to localmachine) only when current user is Admin. | ||
if (ColumnEncryptionCertificateFixture.IsAdmin) | ||
{ | ||
yield return new object[2] { MyPathPrefix, StoreLocation.LocalMachine }; | ||
yield return new object[2] { @"", StoreLocation.LocalMachine }; | ||
yield return new object[2] { LocalMachineMyPathPrefix, StoreLocation.LocalMachine }; | ||
} | ||
yield return new object[2] { MyPathPrefix, StoreLocation.LocalMachine }; | ||
yield return new object[2] { @"", StoreLocation.LocalMachine }; | ||
yield return new object[2] { LocalMachineMyPathPrefix, StoreLocation.LocalMachine }; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
using System; | ||
using System.Runtime.InteropServices; | ||
using System.Threading; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is necessary. |
||
using System.Threading.Tasks; | ||
using System.Transactions; | ||
using Xunit; | ||
|
@@ -75,7 +76,12 @@ public class AmbientTransactionFailureTest | |
}; | ||
|
||
[ConditionalTheory(nameof(s_isNotArmProcess))] // https://github.com/dotnet/corefx/issues/21598 | ||
[MemberData(nameof(ExceptionTestDataForSqlException))] | ||
[MemberData( | ||
nameof(ExceptionTestDataForSqlException), | ||
// xUnit can't consistently serialize the data for this test, so we | ||
// disable enumeration of the test data to avoid warnings on the | ||
// console. | ||
DisableDiscoveryEnumeration = true)] | ||
public void TestSqlException(Action<string> connectAction, string connectionString) | ||
{ | ||
Assert.Throws<SqlException>(() => | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,12 @@ public void InstanceTest() | |
}; | ||
|
||
[Theory] | ||
[MemberData(nameof(FactoryMethodTestData))] | ||
[MemberData( | ||
nameof(FactoryMethodTestData), | ||
// xUnit can't consistently serialize the data for this test, so we | ||
// disable enumeration of the test data to avoid warnings on the | ||
// console. | ||
DisableDiscoveryEnumeration = true)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would a custom serializer help in this case? Not to say we should do it in this PR, just curious if you experimented with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes looked into that, but didn't want to figure out serialization for all of the affected tests as part of this PR. The changes here don't affect how the tests run, and they don't change how they're visualized in IDEs. They just suppress the warnings on the console. I've opened an ADO task to track fixing the serialization itself. |
||
public void FactoryMethodTest(Func<object> factory, Type expectedType, bool singleton) | ||
{ | ||
object value1 = factory(); | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatted for readability, and then squished back into a single line since
<Exec>
treats each line in the Command as a separate command!