Move to xunit.v3

[Youssef1313]

Fixes #

Proposed changes

• Migrates from xunit to xunit.v3

Customer Impact

• None. This is test-only changes

Regression?

• No

Risk

• Very low.

Test methodology

Microsoft Reviewers: Open in CodeFlow

[Youssef1313]

Test Form_DpiChanged_MinMaxSizeNotChanged_Default is failing consistently at least in CI. This needs some investigation.

ConvertManagedToNative_NullObject is also still failing even with the last change. I need to look again.

[Youssef1313]

For Form_DpiChanged_MinMaxSizeNotChanged_Default, it was written in #7467

The PR description says that changing DPI should affect min size and max size, but the test asserts that they actually don't change. The test as written in the PR contained a factor variable that was unused. So I think the intent is that min/max size change by that factor, at least that's what the PR description suggests. What is curious there is how this test ever passed before.

cc @RussKie

[Youssef1313]

Regarding Form_DpiChanged_MinMaxSizeNotChanged_Default, I investigated further. Before the migration to MTP, we are run under VSTest's testhost which was .NET Core 2.1:

So,

winforms/src/System.Windows.Forms.Primitives/src/System/LocalAppContextSwitches/LocalAppContextSwitches.cs

Line 117 in 86f7d0f

if (framework.Version.Major >= 8)

was false :)

[codecov]

Codecov Report

Attention: Patch coverage is 91.13924% with 14 lines in your changes missing coverage. Please review.

Project coverage is 76.61066%. Comparing base (8732cfb) to head (00f06f1).

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #13540         +/-   ##
===================================================
+ Coverage   76.59901%   76.61066%   +0.01164%     
===================================================
  Files           3230        3229          -1     
  Lines         639161      639163          +2     
  Branches       47297       47296          -1     
===================================================
+ Hits          489591      489667         +76     
+ Misses        145988      145920         -68     
+ Partials        3582        3576          -6     

Flag	Coverage Δ
Debug	76.61066% <91.13924%> (+0.01164%)	⬆️
integration	18.80522% <0.00000%> (+0.00979%)	⬆️
production	51.01743% <0.00000%> (+0.01115%)	⬆️
test	97.41604% <91.71975%> (+0.01192%)	⬆️
unit	48.39325% <0.00000%> (-0.01013%)	⬇️

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Youssef1313]

Thanks for review @JeremyKuhne. I addressed your comment about COM2PictureConverter. This is ready for another review, and if you can investigate the few remaining test failures please.

[merriemcgaw]

@LeafShi1 can you help investigate any remaining test failures related to this?

[LeafShi1]

@Youssef1313 The actual problem cannot be seen from the current failure log, and the failure case cannot be reproduced in the local environment. You can try to optimize these two failure cases so that the failure reason can be seen more clearly when it fails.

Perhaps you can try to give ToolStripItem an initial name and assert whether the ToolStripItem name is equal

1. Replace Assert.True(toolStrip1.Items[0].Selected); in the test Control_SelectNextControl_ToolStrips_CycleBackwardExpected with Assert.Equal(toolStrip1.GetSelectedItem().Name, toolStrip1.Items[0].Name);

2. Replace Assert.Equal(toolStripButton3, actual); in test ToolStrip_GetNextItem_ReturnsBackwardItem with Assert.Equal(toolStripButton3.Name, actual.Name);

[Youssef1313]

One thing to explore here is that the built assemblies in the x86 job are actually AnyCPU. Previously, they would run under an x86 dotnet.exe. But after the change in this PR, they are run as x64.

For xunit 2, it happens via https://github.com/dotnet/arcade/blob/b305863166c975997aafee78cf69942e7d2f862a/src/Microsoft.DotNet.Arcade.Sdk/tools/XUnit/XUnit.Runner.targets#L31

[Youssef1313]

Note: For ToolStrip_GetNextItem_ReturnsBackwardItem, I can reproduce it locally. It happens only when the whole test class is run, and not only the specific test. So there must be some state of a previous test affecting it.

Note that the order of tests is impacted between xunit 2 and xunit.v3.

[Youssef1313]

@JeremyKuhne @merriemcgaw @LeafShi1 This PR is ready for review, other than a small issue where building x86 actually produces a 64-bit apphost. This previously used to work with Arcade because it used x86/dotnet.exe, so it was forcing the assembly to run under x86. Now, we simply run the apphost as test projects are executables now, but there is something messed up where you don't end up producing 32-bit apphost.

EDIT: Found a fix/workaround in 8480526. Setting PlatformTarget will affect DefaultAppHostRuntimeIdentifier:

https://github.com/dotnet/sdk/blob/9e76ea7a1e6f06c90ef0d50325ac424e0834c6cd/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets#L196-L207

[Youssef1313]

Last commit is breaking analyzers. Reverting.

@JeremyKuhne @merriemcgaw @LeafShi1 Can you please help with getting things to build correctly and have the right apphost?