Skip to content

Split tests by method rather than class #45520

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Split tests by method rather than class #45520

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5e6778
Switch to spliting our test work items by method instead of class. Le…
marcpopMSFT Nov 15, 2024
26963c0
Fix the cast exception
marcpopMSFT Dec 17, 2024
b5b8a6c
Fix merge issues
marcpopMSFT Dec 18, 2024
f68de14
Fix typo
marcpopMSFT Dec 19, 2024
16e2eb8
Add correct dotnet test splitter
marcpopMSFT Jan 11, 2025
50baef4
Fix the separator logic
marcpopMSFT Jan 13, 2025
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
145 changes: 61 additions & 84 deletions test/HelixTasks/AssemblyScheduler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
using System.Text;

namespace Microsoft.DotNet.SdkCustomHelix.Sdk
{
Expand Down Expand Up @@ -78,7 +79,7 @@ private AssemblyInfoBuilder(string assemblyPath, int methodLimit)
_methodLimit = methodLimit;
}

internal static void Build(string assemblyPath, int methodLimit, List<TypeInfo> typeInfoList, out List<Partition> partitionList, out List<AssemblyPartitionInfo> assemblyInfoList, bool netFramework = false)
internal static void Build(string assemblyPath, int methodLimit, List<TypeInfo> typeInfoList, out List<Partition> partitionList, out List<AssemblyPartitionInfo> assemblyInfoList)
{
var builder = new AssemblyInfoBuilder(assemblyPath, methodLimit);
builder.Build(typeInfoList);
Expand All @@ -93,13 +94,12 @@ private void Build(List<TypeInfo> typeInfoList)
foreach (var typeInfo in typeInfoList)
{
_currentTypeInfoList.Add(typeInfo);

if (_builder.Length > 0)
var separator = "";
if(_builder.Length > 0)
{
_builder.Append("|");
separator = "|";
}
_builder.Append($@"{typeInfo.FullName}");

_builder.Append($@"{separator}{typeInfo.FullName}");
CheckForPartitionLimit(done: false);
}

Expand Down Expand Up @@ -175,12 +175,12 @@ internal IEnumerable<AssemblyPartitionInfo> Schedule(IEnumerable<string> assembl
return list;
}

public IEnumerable<AssemblyPartitionInfo> Schedule(string assemblyPath, bool force = false, bool netFramework = false)
public IEnumerable<AssemblyPartitionInfo> Schedule(string assemblyPath, bool force = false)
{
var typeInfoList = GetTypeInfoList(assemblyPath);
var assemblyInfoList = new List<AssemblyPartitionInfo>();
var partitionList = new List<Partition>();
AssemblyInfoBuilder.Build(assemblyPath, _methodLimit, typeInfoList, out partitionList, out assemblyInfoList, netFramework);
AssemblyInfoBuilder.Build(assemblyPath, _methodLimit, typeInfoList, out partitionList, out assemblyInfoList);

// If the scheduling didn't actually produce multiple partition then send back an unpartitioned
// representation.
Expand Down Expand Up @@ -211,22 +211,25 @@ private static List<TypeInfo> GetTypeInfoList(string assemblyPath)
private static List<TypeInfo> GetTypeInfoList(MetadataReader reader)
{
var list = new List<TypeInfo>();
foreach (var handle in reader.TypeDefinitions)
foreach (var handle in reader.MethodDefinitions)
{
var type = reader.GetTypeDefinition(handle);
if (!IsValidIdentifier(reader, type.Name))
var method = reader.GetMethodDefinition(handle);

var name = reader.GetString(method.Name);
if (!IsValidIdentifier(reader, name))
{
continue;
}

var methodCount = GetMethodCount(reader, type);
if (!ShouldIncludeType(reader, type, methodCount))
if (!ShouldIncludeMethod(reader, method))
{
continue;
}

var fullName = GetFullName(reader, type);
list.Add(new TypeInfo(fullName, methodCount));
var type = reader.GetTypeDefinition(method.GetDeclaringType());
var fullName = GetFullName(reader, type) + "." + name;
list.Add(new TypeInfo(fullName, 1));

}

// Ensure we get classes back in a deterministic order.
Expand All @@ -235,69 +238,56 @@ private static List<TypeInfo> GetTypeInfoList(MetadataReader reader)
}

/// <summary>
/// Determine if this type should be one of the <c>class</c> values passed to xunit. This
/// code doesn't actually resolve base types or trace through inherrited Fact attributes
/// hence we have to error on the side of including types with no tests vs. excluding them.
/// Determine if this method method values passed to xunit. This doesn't check for skipping
/// Include any tests with the known theory and fact attributes
/// </summary>
private static bool ShouldIncludeType(MetadataReader reader, TypeDefinition type, int testMethodCount)
{
// xunit only handles public, non-abstract, non-generic classes
var isPublic =
TypeAttributes.Public == (type.Attributes & TypeAttributes.VisibilityMask) ||
TypeAttributes.NestedPublic == (type.Attributes & TypeAttributes.VisibilityMask);
if (!isPublic ||
TypeAttributes.Abstract == (type.Attributes & TypeAttributes.Abstract) ||
type.GetGenericParameters().Count != 0 ||
TypeAttributes.Class != (type.Attributes & TypeAttributes.ClassSemanticsMask))
{
return false;
}

// Compiler generated types / methods have the shape of the heuristic that we are looking
// at here. Filter them out as well.
if (!IsValidIdentifier(reader, type.Name))
{
return false;
}

if (testMethodCount > 0)
{
return true;
}

// The case we still have to consider at this point is a class with 0 defined methods,
// inheritting from a class with > 0 defined test methods. That is a completely valid
// xunit scenario. For now we're just going to exclude types that inherit from object
// because they clearly don't fit that category.
return !(InheritsFromObject(reader, type) ?? false);
}

private static int GetMethodCount(MetadataReader reader, TypeDefinition type)
private static bool ShouldIncludeMethod(MetadataReader reader, MethodDefinition method)
{
var count = 0;
foreach (var handle in type.GetMethods())
{
var methodDefinition = reader.GetMethodDefinition(handle);
if (methodDefinition.GetCustomAttributes().Count == 0 ||
!IsValidIdentifier(reader, methodDefinition.Name))
{
continue;
}
var methodAttributes = method.GetCustomAttributes();
bool isTestMethod = false;

if (MethodAttributes.Public != (methodDefinition.Attributes & MethodAttributes.Public))
foreach (var attributeHandle in methodAttributes)
{
continue;
var attribute = reader.GetCustomAttribute(attributeHandle);
var attributeConstructorHandle = attribute.Constructor;
MemberReference attributeConstructor;
if (attributeConstructorHandle.Kind == HandleKind.MemberReference)
{
attributeConstructor = reader.GetMemberReference((MemberReferenceHandle)attributeConstructorHandle);
}
else
{
continue;
}
var attributeType = reader.GetTypeReference((TypeReferenceHandle)attributeConstructor.Parent);
var attributeTypeName = reader.GetString(attributeType.Name);

if (attributeTypeName == "FactAttribute" ||
attributeTypeName == "TheoryAttribute" ||
attributeTypeName == "CoreMSBuildAndWindowsOnlyFactAttribute" ||
attributeTypeName == "CoreMSBuildAndWindowsOnlyTheoryAttribute" ||
attributeTypeName == "CoreMSBuildOnlyFactAttribute" ||
attributeTypeName == "CoreMSBuildOnlyTheoryAttribute" ||
attributeTypeName == "FullMSBuildOnlyFactAttribute" ||
attributeTypeName == "FullMSBuildOnlyTheoryAttribute" ||
attributeTypeName == "PlatformSpecificFact" ||
attributeTypeName == "PlatformSpecificTheory" ||
attributeTypeName == "RequiresMSBuildVersionFactAttribute" ||
attributeTypeName == "RequiresMSBuildVersionTheoryAttribute" ||
attributeTypeName == "RequiresSpecificFrameworkFactAttribute" ||
attributeTypeName == "RequiresSpecificFrameworkTheoryAttribute" ||
attributeTypeName == "WindowsOnlyRequiresMSBuildVersionFactAttribute" ||
attributeTypeName == "WindowsOnlyRequiresMSBuildVersionTheoryAttribute")
{
isTestMethod = true;
break;
}
}

count++;
}

return count;
return isTestMethod;
}

private static bool IsValidIdentifier(MetadataReader reader, StringHandle handle)
private static bool IsValidIdentifier(MetadataReader reader, String name)
{
var name = reader.GetString(handle);
for (int i = 0; i < name.Length; i++)
{
switch (name[i])
Expand All @@ -312,19 +302,6 @@ private static bool IsValidIdentifier(MetadataReader reader, StringHandle handle
return true;
}

private static bool? InheritsFromObject(MetadataReader reader, TypeDefinition type)
{
if (type.BaseType.Kind != HandleKind.TypeReference)
{
return null;
}

var typeRef = reader.GetTypeReference((TypeReferenceHandle)type.BaseType);
return
reader.GetString(typeRef.Namespace) == "System" &&
reader.GetString(typeRef.Name) == "Object";
}

private static string GetFullName(MetadataReader reader, TypeDefinition type)
{
var typeName = reader.GetString(type.Name);
Expand Down
Loading