Skip to content
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

Partition helix test runs using historical execution time data #62138

Merged
merged 32 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
561d57f
Add two failing tests
dibarbet Jun 23, 2022
45d49dd
Working on partition
dibarbet Jun 24, 2022
1bdef98
more work
dibarbet Jul 21, 2022
b428661
working
dibarbet Jul 21, 2022
eec06a9
more work
dibarbet Jul 22, 2022
e0a2316
some more work on prepare tests
dibarbet Jul 23, 2022
43991a3
Cleanup unneeded code
dibarbet Jul 23, 2022
04290ae
Use test platform APIs for running tests
dibarbet Jul 23, 2022
f4df81b
Revert "Use test platform APIs for running tests"
dibarbet Jul 25, 2022
8ffd275
Use rsp files and vstestconsole to run tests
dibarbet Jul 26, 2022
a8c75f3
more testing
dibarbet Jul 26, 2022
4e120ec
Fix integration tests assembly lookup
dibarbet Jul 26, 2022
29ba5cf
cleanup
dibarbet Jul 26, 2022
6b96c54
fix correctness warning
dibarbet Jul 27, 2022
e1e6c84
Attempt to fix stack overflow on mac test runs
dibarbet Jul 28, 2022
26ee734
fix check for macos
dibarbet Jul 28, 2022
aca2010
Use different filter syntax that does not use recursion
dibarbet Jul 28, 2022
7b52db0
Merge remote-tracking branch 'upstream/main' into partition_tests
dibarbet Jul 28, 2022
f1626b7
Merge remote-tracking branch 'upstream/main' into partition_tests
dibarbet Jul 29, 2022
3cc6540
Disable tests that fail with new partition scheme
dibarbet Aug 2, 2022
a5b8007
Revert "Disable tests that fail with new partition scheme"
dibarbet Aug 3, 2022
006f3b1
review feedback
dibarbet Aug 3, 2022
0fc332b
cleanup usings
dibarbet Aug 3, 2022
40b2664
Merge remote-tracking branch 'upstream/main' into partition_tests
dibarbet Aug 3, 2022
9fc71af
Add assembly attribute to require all tests to run in a single work item
dibarbet Aug 5, 2022
1bd0e1c
Comment updates
dibarbet Aug 5, 2022
5eda6de
Review feedback
dibarbet Aug 8, 2022
c66fe12
Merge branch 'main' into partition_tests
dibarbet Aug 15, 2022
0519c06
Merge remote-tracking branch 'upstream/main' into partition_tests
dibarbet Aug 16, 2022
fb0c6fc
Unblock downloaded files on windows
dibarbet Aug 16, 2022
6b0d0b8
Merge remote-tracking branch 'upstream/main' into partition_tests
dibarbet Aug 17, 2022
ff5c993
Run WinRT tests in their own work item as they can break tests using …
dibarbet Aug 17, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<MicrosoftILVerificationVersion>7.0.0-alpha.1.22060.1</MicrosoftILVerificationVersion>
<MicrosoftServiceHubVersion>4.0.117</MicrosoftServiceHubVersion>
<MicrosoftVisualStudioThreadingPackagesVersion>17.3.1-alpha</MicrosoftVisualStudioThreadingPackagesVersion>
<MicrosoftTestPlatformVersion>17.4.0-preview-20220707-01</MicrosoftTestPlatformVersion>
</PropertyGroup>
<!--
Dependency versions
Expand Down Expand Up @@ -125,6 +126,9 @@
<MicrosoftServiceHubClientVersion>$(MicrosoftServiceHubVersion)</MicrosoftServiceHubClientVersion>
<MicrosoftServiceHubFrameworkVersion>$(MicrosoftServiceHubVersion)</MicrosoftServiceHubFrameworkVersion>
<MicrosoftSourceLinkToolsVersion>1.1.1-beta-21566-01</MicrosoftSourceLinkToolsVersion>
<MicrosoftTeamFoundationServerClientVersion>16.170.0</MicrosoftTeamFoundationServerClientVersion>
<MicrosoftTestPlatformTranslationLayerVersion>$(MicrosoftTestPlatformVersion)</MicrosoftTestPlatformTranslationLayerVersion>
<MicrosoftTestPlatformObjectModelVersion>$(MicrosoftTestPlatformVersion)</MicrosoftTestPlatformObjectModelVersion>
<MicrosoftVisualBasicVersion>10.1.0</MicrosoftVisualBasicVersion>
<MicrosoftVisualStudioCacheVersion>17.2.41-alpha</MicrosoftVisualStudioCacheVersion>
<MicrosoftVisualStudioCallHierarchyPackageDefinitionsVersion>15.8.27812-alpha</MicrosoftVisualStudioCallHierarchyPackageDefinitionsVersion>
Expand Down
2 changes: 1 addition & 1 deletion eng/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,6 @@ if [[ "$test_core_clr" == true ]]; then
if [[ "$ci" != true ]]; then
runtests_args="$runtests_args --html"
fi
dotnet exec "$scriptroot/../artifacts/bin/RunTests/${configuration}/net6.0/RunTests.dll" --tfm net6.0 --configuration ${configuration} --dotnet ${_InitializeDotNetCli}/dotnet $runtests_args
dotnet exec "$scriptroot/../artifacts/bin/RunTests/${configuration}/net6.0/RunTests.dll" --tfm net6.0 --configuration ${configuration} --logs ${log_dir} --dotnet ${_InitializeDotNetCli}/dotnet $runtests_args
fi
ExitWithExitCode 0
2 changes: 1 addition & 1 deletion eng/prepare-tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ try {
$dotnet = Ensure-DotnetSdk
# permissions issues make this a pain to do in PrepareTests itself.
Remove-Item -Recurse -Force "$RepoRoot\artifacts\testPayload" -ErrorAction SilentlyContinue
Exec-Console $dotnet "$RepoRoot\artifacts\bin\PrepareTests\$configuration\net6.0\PrepareTests.dll --source $RepoRoot --destination $RepoRoot\artifacts\testPayload"
Exec-Console $dotnet "$RepoRoot\artifacts\bin\PrepareTests\$configuration\net6.0\PrepareTests.dll --source $RepoRoot --destination $RepoRoot\artifacts\testPayload --dotnetPath `"$dotnet`""
exit 0
}
catch {
Expand Down
2 changes: 1 addition & 1 deletion eng/prepare-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ InitializeDotNetCli true
# permissions issues make this a pain to do in PrepareTests itself.
rm -rf "$repo_root/artifacts/testPayload"

dotnet "$repo_root/artifacts/bin/PrepareTests/Debug/net6.0/PrepareTests.dll" --source "$repo_root" --destination "$repo_root/artifacts/testPayload" --unix
dotnet "$repo_root/artifacts/bin/PrepareTests/Debug/net6.0/PrepareTests.dll" --source "$repo_root" --destination "$repo_root/artifacts/testPayload" --unix --dotnetPath ${_InitializeDotNetCli}/dotnet
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
<TargetFrameworks>net6.0;net472</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup Label="Project References">

<ItemGroup>
<AssemblyAttribute Include="Microsoft.CodeAnalysis.Test.Utilities.RunTestsInSinglePartitionAttribute" />
jmarolf marked this conversation as resolved.
Show resolved Hide resolved
<ProjectReference Include="..\..\..\..\Test\PdbUtilities\Roslyn.Test.PdbUtilities.csproj" />
<ProjectReference Include="..\..\..\Test\Core\Microsoft.CodeAnalysis.Test.Utilities.csproj" />
<ProjectReference Include="..\..\..\Core\Portable\Microsoft.CodeAnalysis.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<TargetFramework>net472</TargetFramework>
</PropertyGroup>
<ItemGroup>
<AssemblyAttribute Include="Microsoft.CodeAnalysis.Test.Utilities.RunTestsInSinglePartitionAttribute" />
<ProjectReference Include="..\..\..\Test\Core\Microsoft.CodeAnalysis.Test.Utilities.csproj" />
<ProjectReference Include="..\..\..\Core\Portable\Microsoft.CodeAnalysis.csproj" />
<ProjectReference Include="..\..\..\Test\Resources\Core\Microsoft.CodeAnalysis.Compiler.Test.Resources.csproj" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

namespace Microsoft.CodeAnalysis.Test.Utilities;

/// <summary>
/// This is a marker attribute used to define that all tests in this assembly
/// should run in a single partition without other assemblies due to state sharing concerns.
/// For example, Microsoft.CodeAnalysis.CSharp.EndToEnd.UnitTests
///
/// RunTests uses this attribute when partitioning tests.
/// </summary>
[AttributeUsage(AttributeTargets.Assembly)]
public sealed class RunTestsInSinglePartitionAttribute : Attribute
{
}
3 changes: 3 additions & 0 deletions src/Tools/PrepareTests/PrepareTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,8 @@
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Mono.Options" Version="$(MonoOptionsVersion)" />
<PackageReference Include="Microsoft.TestPlatform.TranslationLayer" Version="$(MicrosoftTestPlatformTranslationLayerVersion)" />
<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="$(MicrosoftTestPlatformObjectModelVersion)" />
<PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
</ItemGroup>
</Project>
14 changes: 13 additions & 1 deletion src/Tools/PrepareTests/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System;
using Mono.Options;

namespace PrepareTests;

internal static class Program
{
internal const int ExitFailure = 1;
Expand All @@ -15,12 +17,14 @@ public static int Main(string[] args)
string? source = null;
string? destination = null;
bool isUnix = false;
string? dotnetPath = null;

var options = new OptionSet()
{
{ "source=", "Path to binaries", (string s) => source = s },
{ "destination=", "Output path", (string s) => destination = s },
{ "unix", "If true, prepares tests for unix environment instead of Windows", o => isUnix = o is object }
{ "unix", "If true, prepares tests for unix environment instead of Windows", o => isUnix = o is object },
{ "dotnetPath=", "Path to the dotnet CLI", (string s) => dotnetPath = s },
};
options.Parse(args);

Expand All @@ -36,6 +40,14 @@ public static int Main(string[] args)
return ExitFailure;
}

if (dotnetPath is null)
{
Console.Error.WriteLine("--dotnetPath argument must be provided");
return ExitFailure;
}

TestDiscovery.RunDiscovery(source, dotnetPath, isUnix);

MinimizeUtil.Run(source, destination, isUnix);
return ExitSuccess;
}
Expand Down
127 changes: 127 additions & 0 deletions src/Tools/PrepareTests/TestDiscovery.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.Contracts;
using System.IO;
using System.Linq;
using System.Text.Json;
using Microsoft.TestPlatform.VsTestConsole.TranslationLayer;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;

namespace PrepareTests;
internal class TestDiscovery
{
public static void RunDiscovery(string repoRootDirectory, string dotnetPath, bool isUnix)
{
var binDirectory = Path.Combine(repoRootDirectory, "artifacts", "bin");
var assemblies = GetAssemblies(binDirectory, isUnix);

Console.WriteLine($"Found {assemblies.Count} test assemblies");

var vsTestConsole = Directory.EnumerateFiles(Path.Combine(Path.GetDirectoryName(dotnetPath)!, "sdk"), "vstest.console.dll", SearchOption.AllDirectories).OrderBy(s => s).Last();

var vstestConsoleWrapper = new VsTestConsoleWrapper(vsTestConsole, new ConsoleParameters
{
LogFilePath = Path.Combine(repoRootDirectory, "logs", "test_discovery_logs.txt"),
TraceLevel = TraceLevel.Error,
});

var discoveryHandler = new DiscoveryHandler();

var stopwatch = new Stopwatch();
stopwatch.Start();
vstestConsoleWrapper.DiscoverTests(assemblies, @"<RunSettings><RunConfiguration><MaxCpuCount>0</MaxCpuCount></RunConfiguration></RunSettings>", discoveryHandler);
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
stopwatch.Stop();

var tests = discoveryHandler.GetTests();

Console.WriteLine($"Discovered {tests.Length} tests in {stopwatch.Elapsed}");

stopwatch.Restart();
var testGroupedByAssembly = tests.GroupBy(test => test.Source);
foreach (var assemblyGroup in testGroupedByAssembly)
{
var directory = Path.GetDirectoryName(assemblyGroup.Key);

// Tests with combinatorial data are output multiple times with the same fully qualified test name.
// We only need to include it once as run all combinations under the same filter.
var testToWrite = assemblyGroup.Select(test => test.FullyQualifiedName).Distinct().ToList();

using var fileStream = File.Create(Path.Combine(directory!, "testlist.json"));
JsonSerializer.Serialize(fileStream, testToWrite);
}
stopwatch.Stop();
Console.WriteLine($"Serialized tests in {stopwatch.Elapsed}");
}

private class DiscoveryHandler : ITestDiscoveryEventsHandler
{
private readonly ConcurrentBag<TestCase> _tests = new();
private bool _isComplete = false;

public void HandleDiscoveredTests(IEnumerable<TestCase>? discoveredTestCases)
{
if (discoveredTestCases != null)
{
foreach (var test in discoveredTestCases)
{
_tests.Add(test);
}
}
}

public void HandleDiscoveryComplete(long totalTests, IEnumerable<TestCase>? lastChunk, bool isAborted)
{
if (lastChunk != null)
{
foreach (var test in lastChunk)
{
_tests.Add(test);
}
}

_isComplete = true;
}

public void HandleLogMessage(TestMessageLevel level, string? message)
{
Console.WriteLine(message);
}

public void HandleRawMessage(string rawMessage)
{
}

public ImmutableArray<TestCase> GetTests()
{
Contract.Assert(_isComplete);
return _tests.ToImmutableArray();
}
}

private static List<string> GetAssemblies(string binDirectory, bool isUnix)
{
var unitTestAssemblies = Directory.GetFiles(binDirectory, "*.UnitTests.dll", SearchOption.AllDirectories).Where(ShouldInclude);
return unitTestAssemblies.ToList();

bool ShouldInclude(string path)
{
if (isUnix)
{
// Our unix build will build net framework dlls for multi-targeted projects.
// These are not valid testing on unix and discovery will throw if we try.
return Path.GetFileName(Path.GetDirectoryName(path)) != "net472";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hard coding seems wrong. Instead I think we should be passing down the desired TFM or doing some other filtering at the calling process. Having this hard coded here can lead to hard to track down issues later.

Copy link
Member Author

@dibarbet dibarbet Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can pass this in from the script / yaml if you'd prefer, but I'll still have to either list the tfm's we want to discover (which could change if we start multi-targeting a different tfm for example) or the ones we want to exclude (which could change as well).

Aternatively, if you have a suggestion on some metadata I could look at in the assembly to determine if its compatible with the current platform, that would be preferrable, I wasn't able to immediately come up with a great way.

I realize now my comment here got lost in some of the changes - but basically the unix build builds multitargeted projects which ends up building the net472 dlls. RUnning discovery on them on unix fails.

Copy link
Member Author

@dibarbet dibarbet Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment here - but didn't end up changing it. Some notes

  1. I don't want to filter by valid tfm here because those seem more likely to change than invalid ones. And if they change we'll end up with silent failures of tests not being discovered.
  2. I could move the excluded tfm up to the yaml, but that doesn't necessarily seem better (up to you though).
  3. If the .net framework tfm changes (e.g. we change to net48) then we will get an error during discovery and we can update this, which imho is better than the silent failure.
  4. If the test tries to run a dll that doesn't have the test list, it will also fail, so we'll know if this is excluding dlls incorrectly

I could also consider using SRM to check if the TargetFramework attribute contains 'NETFramework' but not sure if that is worth doing. Happy to hear other thoughts.

}

return true;
}
}
}
30 changes: 30 additions & 0 deletions src/Tools/Source/RunTests/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.IO;

namespace RunTests;

public readonly record struct AssemblyInfo(string AssemblyPath) : IComparable<AssemblyInfo>
{
public string AssemblyName => Path.GetFileName(AssemblyPath);

public int CompareTo(AssemblyInfo other)
{
// Ensure we have a consistent ordering by ordering by assembly path.
return string.Compare(this.AssemblyPath, other.AssemblyPath, StringComparison.Ordinal);
}
}

public readonly record struct TypeInfo(string Name, string FullyQualifiedName, ImmutableArray<TestMethodInfo> Tests)
{
public override string ToString() => $"[Type]{FullyQualifiedName}";
}

public readonly record struct TestMethodInfo(string Name, string FullyQualifiedName, TimeSpan ExecutionTime)
{
public override string ToString() => $"[Method]{FullyQualifiedName}";
}
Loading