Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 0 additions & 11 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,6 @@
<TestRunnerName>Microsoft.Testing.Platform</TestRunnerName>
</PropertyGroup>

<!-- Polyfill config -->
<PropertyGroup>
<PolyEnsure>true</PolyEnsure>
<!-- PolyGuard is temporary until the binary compat with VSTestBridge
https://github.com/microsoft/testfx/pull/7064#issuecomment-3623472023
https://github.com/microsoft/testfx/pull/6977 -->
<PolyGuard>true</PolyGuard>
<PolyStringInterpolation>true</PolyStringInterpolation>
<PolyUseEmbeddedAttribute>true</PolyUseEmbeddedAttribute>
</PropertyGroup>

<ItemGroup>
<Using Include="System.Collections" />
<Using Include="System.Collections.Concurrent" />
Expand Down
1 change: 0 additions & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
<PackageVersion Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageVersion Include="Microsoft.WindowsAppSDK" Version="1.8.251003001" />
<PackageVersion Include="OpenTelemetry" Version="1.14.0" />
<PackageVersion Include="Polyfill" Version="9.7.3" />
<PackageVersion Include="System.Threading.Tasks.Extensions" Version="$(SystemThreadingTasksExtensionsVersion)" />
</ItemGroup>
<ItemGroup Label="Test dependencies">
Expand Down
6 changes: 2 additions & 4 deletions src/Adapter/MSTest.Engine/Engine/TestArgumentsManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ public void RegisterTestArgumentsEntryProvider<TArguments>(
throw new InvalidOperationException("Cannot register TestArgumentsEntry provider after registration is frozen.");
}

if (!_testArgumentsEntryProviders.TryAdd(testNodeStableUid, argumentPropertiesProviderCallback))
{
throw new InvalidOperationException($"TestArgumentsEntry provider is already registered for test node with UID '{testNodeStableUid}'.");
}
// Add will throw an exception if the key already exists, which is intended.
_testArgumentsEntryProviders.Add(testNodeStableUid, argumentPropertiesProviderCallback);
Comment thread
Youssef1313 marked this conversation as resolved.
}

internal void FreezeRegistration() => _isRegistrationFrozen = true;
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTest.Engine/MSTest.Engine.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ This package provides a new experimental engine for MSTest test framework.]]>
</ItemGroup>

<ItemGroup>
<PackageReference Include="Polyfill" PrivateAssets="all" />
<Compile Include="$(RepoRoot)src/Polyfills/**/*.cs" Link="Polyfills\%(RecursiveDir)%(Filename)%(Extension)" />
</ItemGroup>

<!-- NuGet package layout -->
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Polyfill" PrivateAssets="all" />
<Compile Include="$(RepoRoot)src/Polyfills/**/*.cs" Link="Polyfills\%(RecursiveDir)%(Filename)%(Extension)" />
</ItemGroup>

<ItemGroup Label="NuGet">
Expand Down
16 changes: 14 additions & 2 deletions src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,13 @@ internal async Task RunTestsAsync(IEnumerable<TestCase>? tests, IRunContext? run
throw new ArgumentNullException(nameof(frameworkHandle));
}

Ensure.NotNullOrEmpty(tests);
// TODO: Verify why VSTest annotates the IEnumerable as nullable.
if (tests is null)
{
throw new ArgumentNullException(nameof(tests));
}

Ensure.NotEmpty(tests);

if (!MSTestDiscovererHelpers.InitializeDiscovery(from test in tests select test.Source, runContext, frameworkHandle, configuration, new TestSourceHandler()))
{
Expand All @@ -129,7 +135,13 @@ internal async Task RunTestsAsync(IEnumerable<string>? sources, IRunContext? run
throw new ArgumentNullException(nameof(frameworkHandle));
}

Ensure.NotNullOrEmpty(sources);
// TODO: Verify why VSTest annotates the IEnumerable as nullable.
if (sources is null)
{
throw new ArgumentNullException(nameof(sources));
}

Ensure.NotEmpty(sources);

TestSourceHandler testSourceHandler = new();
if (!MSTestDiscovererHelpers.InitializeDiscovery(sources, runContext, frameworkHandle, configuration, testSourceHandler))
Expand Down
15 changes: 14 additions & 1 deletion src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ class AssemblyResolver :
/// <summary>
/// lock for the loaded assemblies cache.
/// </summary>
#if NET9_0_OR_GREATER
private readonly Lock _syncLock = new();
#else
private readonly object _syncLock = new();
#endif

private static List<string>? s_currentlyLoading;
private bool _disposed;
Expand All @@ -99,7 +103,16 @@ class AssemblyResolver :
/// </remarks>
public AssemblyResolver(IList<string> directories)
{
Ensure.NotNullOrEmpty(directories);
if (directories is null)
{
throw new ArgumentNullException(nameof(directories));
}

// Caller always ensures non-empty.
if (directories.Count == 0)
{
throw ApplicationStateGuard.Unreachable();
Comment thread
Youssef1313 marked this conversation as resolved.
}

_searchDirectories = [.. directories];
_directoryList = new Queue<RecursiveDirectoryPath>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,16 +547,16 @@ private async Task ExecuteTestsWithTestRunnerAsync(
// Add tcm properties.
if (tcmProperties is not null)
{
foreach ((TestProperty key, object? value) in tcmProperties)
foreach (KeyValuePair<TestProperty, object?> kvp in tcmProperties)
{
testContextProperties[key.Id] = value;
testContextProperties[kvp.Key.Id] = kvp.Value;
}
}

// Add source level parameters.
foreach ((string key, object value) in sourceLevelParameters)
foreach (KeyValuePair<string, object> kvp in sourceLevelParameters)
{
testContextProperties[key] = value;
testContextProperties[kvp.Key] = kvp.Value;
}

if (unitTestElement.Traits is { Length: > 0 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,9 @@ private async Task<TestResult> ExecuteInternalWithTimeoutAsync(object?[]? argume
else
{
// Cancel the token source as test has timed out
await TestContext.Context.CancellationTokenSource.CancelAsync().ConfigureAwait(false);
#pragma warning disable VSTHRD103 // Call async methods when in an async method - likely fine in this context. CancelAsync is .NET Core only. We prefer having the same behavior between .NET Core and .NET Framework.
TestContext.Context.CancellationTokenSource.Cancel();
#pragma warning restore VSTHRD103 // Call async methods when in an async method
}

TestResult timeoutResult = new() { Outcome = UnitTestOutcome.Timeout, TestFailureException = new TestFailedException(UnitTestOutcome.Timeout, errorMessage) };
Expand Down
191 changes: 112 additions & 79 deletions src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,26 +134,41 @@ public IEnumerable<TestAssemblyInfo> AssemblyInfoListWithExecutableCleanupMethod
DebugEx.Assert(testMethod != null, "test method is null");

string typeName = testMethod.FullClassName;

// Using GetOrAdd to ensure we calculate only once when this is called by different threads in parallel.
// Using a static lambda to ensure we don't capture.
return _classInfoCache.GetOrAdd(typeName, static (typeName, tuple) =>
#if NETCOREAPP
return _classInfoCache.GetOrAdd(typeName, CreateTestClassInfo, (this, testMethod));
#else
// On .NET Framework, we don't have the GetOrAdd overload that prevents capturing lambdas.
// So, we first try to get the value from the cache.
if (_classInfoCache.TryGetValue(typeName, out TestClassInfo? cachedClassInfo))
{
TestMethod testMethod = tuple.testMethod;
TypeCache @this = tuple.Item1;
return cachedClassInfo;
}

// Load the class type
Type? type = LoadType(typeName, testMethod.AssemblyName);
// If value doesn't already exist in the cache, we fallback to the GetOrAdd that allocates.
return _classInfoCache.GetOrAdd(typeName, typeName => CreateTestClassInfo(typeName, (this, testMethod)));
#endif
}

if (type == null)
{
// This means the class containing the test method could not be found.
// Return null so we return a not found result.
return null;
}
private static TestClassInfo? CreateTestClassInfo(string typeName, (TypeCache Cache, TestMethod Method) tuple)
{
TestMethod testMethod = tuple.Method;
TypeCache @this = tuple.Cache;

// Load the class type
Type? type = LoadType(typeName, testMethod.AssemblyName);

if (type == null)
{
// This means the class containing the test method could not be found.
// Return null so we return a not found result.
return null;
}

// Get the classInfo
return @this.CreateClassInfo(type);
}, (this, testMethod));
// Get the classInfo
return @this.CreateClassInfo(type);
}

/// <summary>
Expand Down Expand Up @@ -321,80 +336,94 @@ private TestClassInfo CreateClassInfo(Type classType)
/// <param name="assembly"> The assembly to get its info. </param>
/// <returns> The <see cref="TestAssemblyInfo"/> instance. </returns>
private TestAssemblyInfo GetAssemblyInfo(Assembly assembly)
{
#if NETCOREAPP
// Using GetOrAdd to ensure we calculate only once when this is called by different threads in parallel.
// Using a static lambda to ensure we don't capture.
=> _testAssemblyInfoCache.GetOrAdd(assembly, static (assembly, @this) =>
return _testAssemblyInfoCache.GetOrAdd(assembly, CreateTestAssemblyInfo, this);
#else
if (_testAssemblyInfoCache.TryGetValue(assembly, out TestAssemblyInfo cachedTestAssemblyInfo))
{
return cachedTestAssemblyInfo;
}

// Not cached already. Fallback to GetOrAdd call that captures "this" and allocates.
return _testAssemblyInfoCache.GetOrAdd(assembly, assembly => CreateTestAssemblyInfo(assembly, this));
#endif
}

private static TestAssemblyInfo CreateTestAssemblyInfo(Assembly assembly, TypeCache @this)
{
var assemblyInfo = new TestAssemblyInfo(assembly);

Type[] types = AssemblyEnumerator.GetTypes(assembly);

foreach (Type t in types)
{
try
{
// Only examine classes which are TestClass or derives from TestClass attribute
if (!@this._reflectionHelper.IsAttributeDefined<TestClassAttribute>(t))
{
continue;
}
}
catch (Exception ex)
{
var assemblyInfo = new TestAssemblyInfo(assembly);
// If we fail to discover type from an assembly, then do not abort. Pick the next type.
if (PlatformServiceProvider.Instance.AdapterTraceLogger.IsWarningEnabled)
{
PlatformServiceProvider.Instance.AdapterTraceLogger.Warning(
"TypeCache: Exception occurred while checking whether type {0} is a test class or not. {1}",
t.FullName,
ex);
}

Type[] types = AssemblyEnumerator.GetTypes(assembly);
continue;
}

foreach (Type t in types)
// Enumerate through all methods and identify the Assembly Init and cleanup methods.
foreach (MethodInfo methodInfo in PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethods(t))
{
if (@this.IsAssemblyOrClassInitializeMethod<AssemblyInitializeAttribute>(methodInfo))
{
try
{
// Only examine classes which are TestClass or derives from TestClass attribute
if (!@this._reflectionHelper.IsAttributeDefined<TestClassAttribute>(t))
{
continue;
}
}
catch (Exception ex)
assemblyInfo.AssemblyInitializeMethod = methodInfo;
assemblyInfo.AssemblyInitializeMethodTimeoutMilliseconds = @this.TryGetTimeoutInfo(methodInfo, FixtureKind.AssemblyInitialize);
}
else if (@this.IsAssemblyOrClassCleanupMethod<AssemblyCleanupAttribute>(methodInfo))
{
assemblyInfo.AssemblyCleanupMethod = methodInfo;
assemblyInfo.AssemblyCleanupMethodTimeoutMilliseconds = @this.TryGetTimeoutInfo(methodInfo, FixtureKind.AssemblyCleanup);
}

bool isGlobalTestInitialize = @this._reflectionHelper.IsAttributeDefined<GlobalTestInitializeAttribute>(methodInfo);
bool isGlobalTestCleanup = @this._reflectionHelper.IsAttributeDefined<GlobalTestCleanupAttribute>(methodInfo);

if (isGlobalTestInitialize || isGlobalTestCleanup)
{
// Only try to validate the method if it already has the needed attribute.
// This avoids potential type load exceptions when the return type cannot be resolved.
// NOTE: Users tend to load assemblies in AssemblyInitialize after finishing the discovery.
// We want to avoid loading types early as much as we can.
bool isValid = methodInfo is { IsSpecialName: false, IsPublic: true, IsStatic: true, IsGenericMethod: false, DeclaringType.IsGenericType: false, DeclaringType.IsPublic: true } &&
methodInfo.GetParameters() is { } parameters && parameters.Length == 1 && parameters[0].ParameterType == typeof(TestContext) &&
methodInfo.IsValidReturnType();

if (isValid && isGlobalTestInitialize)
{
// If we fail to discover type from an assembly, then do not abort. Pick the next type.
if (PlatformServiceProvider.Instance.AdapterTraceLogger.IsWarningEnabled)
{
PlatformServiceProvider.Instance.AdapterTraceLogger.Warning(
"TypeCache: Exception occurred while checking whether type {0} is a test class or not. {1}",
t.FullName,
ex);
}

continue;
assemblyInfo.GlobalTestInitializations.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestInitialize)));
}

// Enumerate through all methods and identify the Assembly Init and cleanup methods.
foreach (MethodInfo methodInfo in PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethods(t))
if (isValid && isGlobalTestCleanup)
{
if (@this.IsAssemblyOrClassInitializeMethod<AssemblyInitializeAttribute>(methodInfo))
{
assemblyInfo.AssemblyInitializeMethod = methodInfo;
assemblyInfo.AssemblyInitializeMethodTimeoutMilliseconds = @this.TryGetTimeoutInfo(methodInfo, FixtureKind.AssemblyInitialize);
}
else if (@this.IsAssemblyOrClassCleanupMethod<AssemblyCleanupAttribute>(methodInfo))
{
assemblyInfo.AssemblyCleanupMethod = methodInfo;
assemblyInfo.AssemblyCleanupMethodTimeoutMilliseconds = @this.TryGetTimeoutInfo(methodInfo, FixtureKind.AssemblyCleanup);
}

bool isGlobalTestInitialize = @this._reflectionHelper.IsAttributeDefined<GlobalTestInitializeAttribute>(methodInfo);
bool isGlobalTestCleanup = @this._reflectionHelper.IsAttributeDefined<GlobalTestCleanupAttribute>(methodInfo);

if (isGlobalTestInitialize || isGlobalTestCleanup)
{
// Only try to validate the method if it already has the needed attribute.
// This avoids potential type load exceptions when the return type cannot be resolved.
// NOTE: Users tend to load assemblies in AssemblyInitialize after finishing the discovery.
// We want to avoid loading types early as much as we can.
bool isValid = methodInfo is { IsSpecialName: false, IsPublic: true, IsStatic: true, IsGenericMethod: false, DeclaringType.IsGenericType: false, DeclaringType.IsPublic: true } &&
methodInfo.GetParameters() is { } parameters && parameters.Length == 1 && parameters[0].ParameterType == typeof(TestContext) &&
methodInfo.IsValidReturnType();

if (isValid && isGlobalTestInitialize)
{
assemblyInfo.GlobalTestInitializations.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestInitialize)));
}

if (isValid && isGlobalTestCleanup)
{
assemblyInfo.GlobalTestCleanups.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestCleanup)));
}
}
assemblyInfo.GlobalTestCleanups.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestCleanup)));
}
}
}
}

return assemblyInfo;
}, this);
return assemblyInfo;
}

/// <summary>
/// Verify if a given method is an Assembly or Class Initialize method.
Expand Down Expand Up @@ -650,10 +679,14 @@ private DiscoveryTestMethodInfo ResolveTestMethodInfoForDiscovery(TestMethod tes
/// <returns> The <see cref="MethodInfo"/>. </returns>
private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassInfo testClassInfo)
{
bool discoverInternals = _discoverInternalsCache.GetOrAdd(
testMethod.AssemblyName,
static (_, testClassInfo) => testClassInfo.Parent.Assembly.GetCustomAttribute<DiscoverInternalsAttribute>() != null,
testClassInfo);
// TODO: The cache key could be TestAssemblyInfo or Assembly which would simplify this to not need to capture.
// TODO: We might not even need a dictionary cache at all, just let it be part of TestAssemblyInfo directly.
if (!_discoverInternalsCache.TryGetValue(testMethod.AssemblyName, out bool discoverInternals))
{
discoverInternals = _discoverInternalsCache.GetOrAdd(
testMethod.AssemblyName,
_ => testClassInfo.Parent.Assembly.GetCustomAttribute<DiscoverInternalsAttribute>() is not null);
}

MethodInfo? testMethodInfo = testMethod.HasManagedMethodAndTypeProperties
? GetMethodInfoUsingManagedNameHelper(testMethod, testClassInfo, discoverInternals)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,15 @@ internal TestResult[] RunSingleTest(UnitTestElement unitTestElement, IDictionary
/// <returns> The <see cref="TestResult"/>. </returns>
internal async Task<TestResult[]> RunSingleTestAsync(UnitTestElement unitTestElement, IDictionary<string, object?> testContextProperties, IMessageLogger messageLogger)
{
Ensure.NotNull(unitTestElement);
Ensure.NotNull(testContextProperties);
if (unitTestElement is null)
{
throw new ArgumentNullException(nameof(unitTestElement));
}

if (testContextProperties is null)
{
throw new ArgumentNullException(nameof(testContextProperties));
}

TestMethod testMethod = unitTestElement.TestMethod;
ITestContext? testContextForTestExecution = null;
Expand Down
Loading