xunit / visualstudio.xunit

VSTest runner for xUnit.net (for Visual Studio Test Explorer and dotnet test)
https://xunit.net/
Other
145 stars 81 forks source link

DisableAppDomain switch is not fully respected for Discovery #331

Closed nohwnd closed 1 year ago

nohwnd commented 2 years ago

I am testing with Roslyn like solution, to see how much gain we would get by passing DisableAppDomain switch by default in discovery to avoid the slowness of RealProxy xunit/xunit#2323, because we run in parallel by default now, and isolate on process level.

DisableAppDomain vstest option switch was implemented in xunit/xunit#944, but I don't think it is fully propagated for discovery. When I debug the testhost process I can see there are still 4 appdomains. And I can see a lot of time is spent remoting between them.

I am also unsure if CollectSourceInformation is propagated fully, because that I see dia session being created, in a separate appdomain, but maybe it is created but later not used (honoring the option is implemented in xunit/xunit#1352).

When I crudely patch v2 xunit and xunit.runner.visualstudio to always use AppDomainManager_NoAppDomain, and NullSourceInformationProvider, the discovery time goes from ~8 seconds down to ~2 for the biggest projects with ~25k tests each:

Before, ~8s per project:

[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.3+1b45f5407b (64-bit Desktop .NET 4.0.30319.42000)
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.3+1b45f5407b (64-bit Desktop .NET 4.0.30319.42000)
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.37]   Discovering: Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.37]   Discovering: Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:07.47]   Discovered:  Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:08.01]   Discovered:  Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472
[DISCOVERY.COMPLETE] aborted? False, tests count: 54628, discovered count: 54628
Last chunk:
Fully discovered:
        S:\t\fake-roslyn\FakeRoslynXunit\Generated\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472\bin\Debug\net472\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472.dll
        S:\t\fake-roslyn\FakeRoslynXunit\Generated\Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472\bin\Debug\net472\Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472.dll
Partially discovered:
        <empty>
Skipped discovery:
        <empty>
Not discovered:
        <empty>
Discovery done in 14803 ms
Discovery: 14803 ms, Run: 0 ms, Total: 14803 ms

After ~2s per project:

[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.6-preview.4+b0b4704a7c (64-bit Desktop .NET 4.0.30319.42000)
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.6-preview.4+b0b4704a7c (64-bit Desktop .NET 4.0.30319.42000)
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.25]   Discovering: Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:00.24]   Discovering: Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:01.85]   Discovered:  Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472
[DISCOVERY.INFORMATIONAL] [xUnit.net 00:00:01.96]   Discovered:  Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472
[DISCOVERY.COMPLETE] aborted? False, tests count: 54628, discovered count: 54628
Last chunk:
Fully discovered:
        S:\t\fake-roslyn\FakeRoslynXunit\Generated\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472\bin\Debug\net472\Microsoft.CodeAnalysis.CSharp.EditorFeatures.UnitTests-net472.dll
        S:\t\fake-roslyn\FakeRoslynXunit\Generated\Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472\bin\Debug\net472\Microsoft.CodeAnalysis.CSharp.EditorFeatures2.UnitTests-net472.dll
Partially discovered:
        <empty>
Skipped discovery:
        <empty>
Not discovered:
        <empty>
Discovery done in 8681 ms
Discovery: 8681 ms, Run: 0 ms, Total: 8681 ms

I don't have a simple repro for the fix, but .NET does not use appdomains (and is overall faster), so creating a solution with 25k tests, and switching between net48, and net6.0 target frameworks shows significantly different times it takes to discover the project under VS.

<!-- file combi.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net472</TargetFramework> <!-- switch to net6.0 and rebuild  -->
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <LangVersion>Latest</LangVersion>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
    <PackageReference Include="Xunit.Combinatorial" Version="1.5.7-beta" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="3.1.2">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

</Project>
// file UnitTest1.cs
namespace combi;

public class UnitTest1
{
    [Theory, CombinatorialData]
    public void CombinatorialRandomValuesCount([CombinatorialRange(0, 25000)] int p1)
    {
        Assert.InRange(p1, 0, int.MaxValue);
    }
}
// file Usings.cs
global using Xunit;

Would you take a fix for this for v2?

nohwnd commented 2 years ago

@bradwilson not trying to be pushy, but this would really help us speed up things under VS. Is it okay if I PR fixes for it?

bradwilson commented 2 years ago

It's up to @clairernovotny to decide which fixes to take and when releases are made.

nohwnd commented 2 years ago

@clairernovotny polite bump on this one.

bradwilson commented 1 year ago

@nohwnd FYI @clairernovotny has stepped away from this project, but if you are able to provide a PR, I can put it into the 2.5.0 release (I anticipate a final build happening in the next few weeks).

nohwnd commented 1 year ago

I am not sure if I will be able to produce and test it in the next 2 weeks, but I take this as a GO and will look into it when I have some time.

bradwilson commented 1 year ago

Coming to a 2.5.0 near you, soon...