Update xunit.v3 dependency to 4.0.0-pre.81 - #186
Conversation
Summary Updates xunit.v3 packages from 3.2.2 to 4.0.0-pre.108 and adapts all breaking API changes. This enables consumers like dotnet/winforms (https://github.com/dotnet/winforms) whose Arcade SDK pins xunit.v3 to 4.0.0-pre.81+ to use the stafact 3.x line without MissingMethodException at test discovery time. Motivation The .NET Arcade SDK pins xunit.v3 to 4.0.0-pre.81 via CentralPackageTransitivePinningEnabled. The current stafact 3.0.x line was compiled against xunit.v3 3.x, and several APIs changed signatures between 3.x and 4.0.0-pre. This causes MissingMethodException at runtime when the old binary runs against the new assembly. This manifests as all [StaFact], [StaTheory], [WinFormsFact], [WinFormsTheory], tests failing during discovery with errors like: System.MissingMethodException: Method not found: 'System.ValueTuple`12[...] Xunit.v3.TestIntrospectionHelper.GetTestCaseDetails(...)' Breaking API changes addressed 1. TheoryDiscoverer.CreateTestCasesForDataRow — The virtual (overridable) version now has 6 parameters instead of 5, with a new string? index parameter. The old 5-parameter version is retained as a non-virtual obsolete compatibility shim. 2. XunitTestCaseRunnerBaseContext constructor — Now requires a FixtureMappingManager methodFixtureMappings parameter for method-level fixture support. 3. XunitTestRunnerBaseContext constructor — Now requires a FixtureMappingManager caseFixtureMappings parameter for case-level fixture support. 4. xunit.analyzers — Transitive dependency bumped from 1.27.0 to 2.0.0-pre.51 (required by xunit.v3 4.0.0-pre.108).
… also uses version 4.0.0-pre.81. - Directory.Packages.props: XunitV3LibraryVersion 3.2.2 → 4.0.0-pre.81, xunit.analyzers 1.27.0 → 2.0.0-pre.51 - 5 discoverer files: added string? index parameter to CreateTestCasesForDataRow override
| <_TestLogs Condition="'$(_TestLogs)'==''">$(RepoRootPath)test_logs/</_TestLogs> | ||
| </PropertyGroup> | ||
| <Exec Command=""$(_AppExePath)" -xml "$(_TestLogs)$(TargetName).xml"" /> | ||
| <Exec Command=""$(_AppExePath)" --report-trx --results-directory "$(_TestLogs)"" /> |
There was a problem hiding this comment.
This... seems strange.
You're moving from a command line option that is part of the native xUnit.net UX command line switch (-xml) to one from Microsoft Testing Platform UX (--report-trx), but I see nothing to indicate that there is an intentional change to shift the UX (which would involve setting <UseMicrosoftTestingPlatformRunner> to true.
Furthermore, this also appears to change the format of the report from the xUnit.net native XML report format to the Visual Studio Test Report format.
I'm not 100% sure this was an intentional change, or one that would be successful as-is.
There was a problem hiding this comment.
It didn't work anyway. TBH the MTP vs. xunit boundary is not clear to me at all. But something in this xunit upgrade broke -xml so I was looking for an alternative.
I'll keep looking.
There was a problem hiding this comment.
We set UseMicrosoftTestingPlatformRunner to true in a Directory.Build.props file.
There was a problem hiding this comment.
In that case, -xml should never have worked.
There was a problem hiding this comment.
If you want to try to retain our report format, then the MTP version of -xml, for the 4.0.0 packages, is --report-xunit-xml.
<Exec Command=""$(_AppExePath)" --report-xunit-xml --results-directory "$(_TestLogs)"" />```
There was a problem hiding this comment.
This is what the MTP UX looks like:
There was a problem hiding this comment.
I can verify that the Mac build, for some reason, is not picking up UseMicrosoftTestingPlatformRunner:
There was a problem hiding this comment.
Culprit located. 😄
There was a problem hiding this comment.
I'm very interested in doing this right, and I'm very willing to believe I've got it wrong, even if things 'pass'.
It appears that Xunit.StaFact.Tests.Mac has an overridden Main method, and the app delegate calls ConsoleRunner:
This is what the auto-generated Main method looks like when UseMicrosoftTestingPlatformRunner is true:
if (global::System.Linq.Enumerable.Any(args, arg => arg == "-automated" || arg == "@@"))
return global::Xunit.Runner.InProc.SystemConsole.ConsoleRunner.Run(args).GetAwaiter().GetResult();
else
return global::Xunit.MicrosoftTestingPlatform.TestPlatformTestFramework.RunAsync(args, global::{{selfRegisteredExtensionsNamespace}}SelfRegisteredExtensions.AddSelfRegisteredExtensions).GetAwaiter().GetResult();That's essentially what you'd replace line 34 linked above with.
Shyam-Gupta
changed the title
3 participants
Summary: Updates xunit.v3 packages from 3.2.2 to 4.0.0-pre.81 and adapts all breaking API changes. This enables consumers like dotnet/winforms (https://github.com/dotnet/winforms) whose Arcade SDK pins xunit.v3 to
4.0.0-pre.81+ to use the stafact 3.x line without MissingMethodException at test discovery time.
Motivation: The .NET Arcade SDK pins xunit.v3 to 4.0.0-pre.81 via CentralPackageTransitivePinningEnabled. The current stafact 3.0.x line was compiled against xunit.v3 3.x, and some APIs changed signatures between 3.x and 4.0.0-pre. This causes MissingMethodException at runtime when the old binary runs against the new assembly.
This manifests as all [StaFact], [StaTheory], [WinFormsFact], [WinFormsTheory], tests failing during discovery with errors like:
System.MissingMethodException: Method not found: 'System.ValueTuple`12[...] Xunit.v3.TestIntrospectionHelper.GetTestCaseDetails(...)'
Breaking API changes addressed