dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
671 stars 349 forks source link

Test with [SkipOnTargetFramework] and [ConditionalFact] grouped as 'No Traits' in Visual Studio Test Explorer #15207

Open weifenluo opened 3 weeks ago

weifenluo commented 3 weeks ago

I have the following test:

[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void TrimDbEntityType()
{
    ...
}

which uses RemoteExecutor to test against AppContext switch.

The test project is multi-targeting .Net Framework 462 and .Net 8.0.

I have following assembly level traits in AssemblyInfo.cs of the test project:

using Xunit;

#if AOT
[assembly: AssemblyTrait("test.aot", "")]
#else
[assembly: AssemblyTrait("test", "")]
#endif

The dependencies in Directory.Build.props if matters:

  <PropertyGroup Condition="'$(IsTestProject)' == 'true'">
    <IsPackable>false</IsPackable>
    <RestoreAdditionalProjectSources>https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json</RestoreAdditionalProjectSources>
    <xUnitVersion>2.9.2</xUnitVersion>
    <MicrosoftDotNetToolsVersion>9.0.0-beta.24421.7</MicrosoftDotNetToolsVersion>
  </PropertyGroup>

  <ItemGroup Condition="'$(IsTestProject)' == 'true'">
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.0" />
    <PackageReference Include="Moq" Version="4.10.0" />
    <PackageReference Include="xunit" Version="$(xUnitVersion)" />
    <PackageReference Include="xunit.extensibility.core" Version="$(xUnitVersion)" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
    <PackageReference Include="coverlet.collector" Version="6.0.2" />
    <PackageReference Include="Microsoft.DotNet.RemoteExecutor" Version="$(MicrosoftDotNetToolsVersion)" />
    <PackageReference Include="Microsoft.DotNet.XUnitExtensions" Version="$(MicrosoftDotNetToolsVersion)" />
  </ItemGroup>

The test runs as expected when targeting .Net 8.0. However when targeting .Net Framework 461, the test is displayed under 'No Traits' in Visual Studio Test Explorer and run as failed because RemoteExecutor is not supported by .Net Framework.

Am I doing anything wrong? I'm expecting the test is completely filtered out of Test Explorer.

weifenluo commented 3 weeks ago

I ended up with giving up using [SkipOnTargetFramework] and [ConditionalFact], by exclude source code files from .csproj:

  1. All tests with RemoteExecutor are in separated source code file *.RemoteExecutor.cs;
  2. In .csproj add the following:

    <ItemGroup>
    <Compile Remove="**\*.RemoteExecutor.cs" />
    </ItemGroup>
    
    <ItemGroup>
    <None Include="**\*.RemoteExecutor.cs" />
    </ItemGroup>
    
    <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
    <Compile Include="**\*.RemoteExecutor.cs" />
    </ItemGroup>
ViktorHofer commented 3 weeks ago

Yes, for those attributes to work, you need to filter out "failing" test attributes in your .runsettings file: https://github.com/dotnet/maintenance-packages/blob/5aba9a6630d795ce652afca2d15440b0360c50ca/eng/testing/.runsettings#L13-L14

ViktorHofer commented 3 weeks ago

See https://github.com/dotnet/arcade/issues/15195

MichelZ commented 1 week ago

Should this also work with ConditionalFact? I can't seem to be able to filter those out in Test Explorer, where the SkipXXX attributes seem to work

ViktorHofer commented 1 week ago

See https://github.com/dotnet/maintenance-packages/pull/141#issuecomment-2435723426 which lists the attributes that depend on the failing trait being filtered out.

MichelZ commented 1 week ago

Thanks. Looks like it does not work with ConditionalFact

ViktorHofer commented 1 week ago

Interesting. If you have time to set-up a small repro, I would be happy to take a closer look. We use that attribute a LOT in dotnet/runtime so it should work just fine in VS.

MichelZ commented 1 week ago

Absolutely! Here you go: https://github.com/MichelZ/ConditionalFactNotExcludedRepro As you can see, tests which are using e.g. SkipOnPlatformand ActiveIssueare not shown in test explorer, where all the others using ConditionalFactand ConditionalTheoryare

Image

ViktorHofer commented 1 week ago

OK got it. Will take a look. Are they getting executed when doing a dotnet test with the filtered out trait in the .runsettings file? (I can test myself as well)

MichelZ commented 1 week ago

No, they are not, dotnet test works fine:


PS G:\MIZE\DEV\GitHub\ConditionalFactNotExcludedRepro> dotnet test .\ConditionalFactNotExcludedRepro.sln --filter category!=failing
Restore complete (0.4s)
  JustTest succeeded (0.1s) → JustTest\bin\Debug\net8.0\JustTest.dll
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 8.0.11)
[xUnit.net 00:00:00.05]   Discovering: JustTest
[xUnit.net 00:00:00.08]   Discovered:  JustTest
[xUnit.net 00:00:00.09]   Starting:    JustTest
[xUnit.net 00:00:00.11]     JustTest.ATestThatShouldBe.IgnoredUsingConditionalTheory [SKIP]
[xUnit.net 00:00:00.11]       Condition(s) not met: "ShouldThisRun"
[xUnit.net 00:00:00.11]     JustTest.ATestThatShouldBe.IgnoredUsingConditionalFact [SKIP]
[xUnit.net 00:00:00.11]       Condition(s) not met: "ShouldThisRun"
[xUnit.net 00:00:00.11]     JustTest.ATestThatShouldBe.IgnoredUsingConditionalFactWithClassMethod [SKIP]
[xUnit.net 00:00:00.11]       Condition(s) not met: "HellNo"
[xUnit.net 00:00:00.11]     JustTest.ATestThatShouldBe.IgnoredUsingConditionalFactWithClass [SKIP]
[xUnit.net 00:00:00.11]       Condition(s) not met: "AbsolutlyNot"
[xUnit.net 00:00:00.11]     JustTest.ATestThatShouldBe.IgnoredUsingConditionalFactWithClassField [SKIP]
[xUnit.net 00:00:00.11]       Condition(s) not met: "Nope"
[xUnit.net 00:00:00.11]   Finished:    JustTest
  JustTest test succeeded (0.7s)

Test summary: total: 5, failed: 0, succeeded: 0, skipped: 5, duration: 0.7s
Build succeeded in 1.3s

What's nice here though is that the Conditional tests are clearly shown and marked as Skipped, which is very nice for the dotnet testexperience