Skip to content

Commit

Permalink
Refactor some logic out of TypeCache (#4786)
Browse files Browse the repository at this point in the history 
Co-authored-by: Amaury Levé <[email protected]>
  • Loading branch information
@Youssef1313 @Evangelink
Youssef1313 and Evangelink authored Feb 19, 2025
1 parent 1d72cd0 commit d141931
Show file tree
Hide file tree
Showing 10 changed files with 485 additions and 323 deletions.
107 changes: 74 additions & 33 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Extensions;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
using Microsoft.VisualStudio.TestTools.UnitTesting;

using UTF = Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -37,22 +38,30 @@ public class TestMethodInfo : ITestMethod
internal TestMethodInfo(
MethodInfo testMethod,
TestClassInfo parent,
TestMethodOptions testMethodOptions)
ITestContext testContext)
{
DebugEx.Assert(testMethod != null, "TestMethod should not be null");
DebugEx.Assert(parent != null, "Parent should not be null");

TestMethod = testMethod;
Parent = parent;
TestMethodOptions = testMethodOptions;
TestContext = testContext;
ExpectedException = ResolveExpectedException();
RetryAttribute = GetRetryAttribute();
TimeoutInfo = GetTestTimeout();
Executor = GetTestMethodAttribute();
}

internal TimeoutInfo TimeoutInfo { get; /*For testing only*/set; }

internal TestMethodAttribute Executor { get; /*For testing only*/set; }

internal ITestContext TestContext { get; }

/// <summary>
/// Gets a value indicating whether timeout is set.
/// </summary>
public bool IsTimeoutSet => TestMethodOptions.TimeoutInfo.Timeout != TimeoutWhenNotSet;
public bool IsTimeoutSet => TimeoutInfo.Timeout != TimeoutWhenNotSet;

/// <summary>
/// Gets the reason why the test is not runnable.
Expand Down Expand Up @@ -104,11 +113,6 @@ internal TestMethodInfo(
/// </summary>
internal TestClassInfo Parent { get; }

/// <summary>
/// Gets the options for the test method in this environment.
/// </summary>
internal TestMethodOptions TestMethodOptions { get; }

internal ExpectedExceptionBaseAttribute? ExpectedException { get; set; /*set for testing only*/ }

internal RetryBaseAttribute? RetryAttribute { get; }
Expand Down Expand Up @@ -146,7 +150,7 @@ public virtual TestResult Invoke(object?[]? arguments)
// check if arguments are set for data driven tests
arguments ??= Arguments;

using LogMessageListener listener = new(TestMethodOptions.CaptureDebugTraces);
using LogMessageListener listener = new(MSTestSettings.CurrentSettings.CaptureDebugTraces);
watch.Start();
try
{
Expand All @@ -163,8 +167,8 @@ public virtual TestResult Invoke(object?[]? arguments)
result.DebugTrace = listener.GetAndClearDebugTrace();
result.LogOutput = listener.GetAndClearStandardOutput();
result.LogError = listener.GetAndClearStandardError();
result.TestContextMessages = TestMethodOptions.TestContext?.GetAndClearDiagnosticMessages();
result.ResultFiles = TestMethodOptions.TestContext?.GetResultFiles();
result.TestContextMessages = TestContext?.GetAndClearDiagnosticMessages();
result.ResultFiles = TestContext?.GetResultFiles();
}
}

Expand Down Expand Up @@ -248,6 +252,43 @@ public virtual TestResult Invoke(object?[]? arguments)
return newParameters;
}

/// <summary>
/// Gets the test timeout for the test method.
/// </summary>
/// <returns> The timeout value if defined in milliseconds. 0 if not defined. </returns>
private TimeoutInfo GetTestTimeout()
{
DebugEx.Assert(TestMethod != null, "TestMethod should be non-null");
TimeoutAttribute? timeoutAttribute = ReflectHelper.Instance.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(TestMethod, inherit: false);
if (timeoutAttribute is null)
{
return TimeoutInfo.FromTestTimeoutSettings();
}

if (!timeoutAttribute.HasCorrectTimeout)
{
string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, TestMethod.DeclaringType!.FullName, TestMethod.Name);
throw new TypeInspectionException(message);
}

return TimeoutInfo.FromTimeoutAttribute(timeoutAttribute);
}

/// <summary>
/// Provides the Test Method Extension Attribute of the TestClass.
/// </summary>
/// <returns>Test Method Attribute.</returns>
private TestMethodAttribute GetTestMethodAttribute()
{
// Get the derived TestMethod attribute from reflection.
// It should be non-null as it was already validated by IsValidTestMethod.
TestMethodAttribute testMethodAttribute = ReflectHelper.Instance.GetFirstDerivedAttributeOrDefault<TestMethodAttribute>(TestMethod, inherit: false)!;

// Get the derived TestMethod attribute from Extended TestClass Attribute
// If the extended TestClass Attribute doesn't have extended TestMethod attribute then base class returns back the original testMethod Attribute
return Parent.ClassAttribute.GetTestMethodAttribute(testMethodAttribute) ?? testMethodAttribute;
}

/// <summary>
/// Resolves the expected exception attribute. The function will try to
/// get all the expected exception attributes defined for a testMethod.
Expand Down Expand Up @@ -385,7 +426,7 @@ private TestResult ExecuteInternal(object?[]? arguments, CancellationTokenSource
// Expected Exception was thrown, so Pass the test
result.Outcome = UTF.UnitTestOutcome.Passed;
}
else if (realException.IsOperationCanceledExceptionFromToken(TestMethodOptions.TestContext!.Context.CancellationTokenSource.Token))
else if (realException.IsOperationCanceledExceptionFromToken(TestContext!.Context.CancellationTokenSource.Token))
{
result.Outcome = UTF.UnitTestOutcome.Timeout;
result.TestFailureException = new TestFailedException(
Expand All @@ -395,7 +436,7 @@ private TestResult ExecuteInternal(object?[]? arguments, CancellationTokenSource
CultureInfo.InvariantCulture,
Resource.Execution_Test_Timeout,
TestMethodName,
TestMethodOptions.TimeoutInfo.Timeout)
TimeoutInfo.Timeout)
: string.Format(
CultureInfo.InvariantCulture,
Resource.Execution_Test_Cancelled,
Expand Down Expand Up @@ -434,7 +475,7 @@ private TestResult ExecuteInternal(object?[]? arguments, CancellationTokenSource
}

// Update TestContext with outcome and exception so it can be used in the cleanup logic.
if (TestMethodOptions.TestContext is { } testContext)
if (TestContext is { } testContext)
{
testContext.SetOutcome(result.Outcome);
// Uwnrap the exception if it's a TestFailedException
Expand Down Expand Up @@ -612,12 +653,12 @@ private void RunTestCleanupMethod(TestResult result, CancellationTokenSource? ti
try
{
// Reset the cancellation token source to avoid cancellation of cleanup methods because of the init or test method cancellation.
TestMethodOptions.TestContext!.Context.CancellationTokenSource = new CancellationTokenSource();
TestContext!.Context.CancellationTokenSource = new CancellationTokenSource();

// If we are running with a method timeout, we need to cancel the cleanup when the overall timeout expires. If it already expired, nothing to do.
if (timeoutTokenSource is { IsCancellationRequested: false })
{
timeoutTokenSource?.Token.Register(TestMethodOptions.TestContext.Context.CancellationTokenSource.Cancel);
timeoutTokenSource?.Token.Register(TestContext.Context.CancellationTokenSource.Cancel);
}

// Test cleanups are called in the order of discovery
Expand Down Expand Up @@ -820,15 +861,15 @@ private bool RunTestInitializeMethod(object classInstance, TestResult result, Ca

return FixtureMethodRunner.RunWithTimeoutAndCancellation(
() => methodInfo.InvokeAsSynchronousTask(classInstance, null),
TestMethodOptions.TestContext!.Context.CancellationTokenSource,
TestContext!.Context.CancellationTokenSource,
timeout,
methodInfo,
new InstanceExecutionContextScope(classInstance, Parent.ClassType),
Resource.TestInitializeWasCancelled,
Resource.TestInitializeTimedOut,
timeoutTokenSource is null
? null
: (timeoutTokenSource, TestMethodOptions.TimeoutInfo.Timeout));
: (timeoutTokenSource, TimeoutInfo.Timeout));
}

private TestFailedException? InvokeCleanupMethod(MethodInfo methodInfo, object classInstance, int remainingCleanupCount, CancellationTokenSource? timeoutTokenSource)
Expand All @@ -841,15 +882,15 @@ timeoutTokenSource is null

return FixtureMethodRunner.RunWithTimeoutAndCancellation(
() => methodInfo.InvokeAsSynchronousTask(classInstance, null),
TestMethodOptions.TestContext!.Context.CancellationTokenSource,
TestContext!.Context.CancellationTokenSource,
timeout,
methodInfo,
new InstanceExecutionContextScope(classInstance, Parent.ClassType, remainingCleanupCount),
Resource.TestCleanupWasCancelled,
Resource.TestCleanupTimedOut,
timeoutTokenSource is null
? null
: (timeoutTokenSource, TestMethodOptions.TimeoutInfo.Timeout));
: (timeoutTokenSource, TimeoutInfo.Timeout));
}

/// <summary>
Expand All @@ -874,7 +915,7 @@ private bool SetTestContext(object classInstance, TestResult result)
{
if (Parent.TestContextProperty != null && Parent.TestContextProperty.CanWrite)
{
Parent.TestContextProperty.SetValue(classInstance, TestMethodOptions.TestContext);
Parent.TestContextProperty.SetValue(classInstance, TestContext);
}

return true;
Expand Down Expand Up @@ -912,7 +953,7 @@ private bool SetTestContext(object classInstance, TestResult result)
object? classInstance = null;
try
{
classInstance = Parent.Constructor.Invoke(Parent.IsParameterlessConstructor ? null : [TestMethodOptions.TestContext]);
classInstance = Parent.Constructor.Invoke(Parent.IsParameterlessConstructor ? null : [TestContext]);
}
catch (Exception ex)
{
Expand All @@ -935,10 +976,10 @@ private bool SetTestContext(object classInstance, TestResult result)
// It also seems that in rare cases the ex can be null.
Exception realException = ex.GetRealException();

if (realException.IsOperationCanceledExceptionFromToken(TestMethodOptions.TestContext!.Context.CancellationTokenSource.Token))
if (realException.IsOperationCanceledExceptionFromToken(TestContext!.Context.CancellationTokenSource.Token))
{
result.Outcome = UTF.UnitTestOutcome.Timeout;
result.TestFailureException = new TestFailedException(UTFUnitTestOutcome.Timeout, string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TestMethodOptions.TimeoutInfo.Timeout));
result.TestFailureException = new TestFailedException(UTFUnitTestOutcome.Timeout, string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TimeoutInfo.Timeout));
}
else
{
Expand Down Expand Up @@ -969,21 +1010,21 @@ private TestResult ExecuteInternalWithTimeout(object?[]? arguments)
{
DebugEx.Assert(IsTimeoutSet, "Timeout should be set");

if (TestMethodOptions.TimeoutInfo.CooperativeCancellation)
if (TimeoutInfo.CooperativeCancellation)
{
CancellationTokenSource? timeoutTokenSource = null;
try
{
timeoutTokenSource = new(TestMethodOptions.TimeoutInfo.Timeout);
timeoutTokenSource.Token.Register(TestMethodOptions.TestContext!.Context.CancellationTokenSource.Cancel);
timeoutTokenSource = new(TimeoutInfo.Timeout);
timeoutTokenSource.Token.Register(TestContext!.Context.CancellationTokenSource.Cancel);
if (timeoutTokenSource.Token.IsCancellationRequested)
{
return new()
{
Outcome = UTF.UnitTestOutcome.Timeout,
TestFailureException = new TestFailedException(
UTFUnitTestOutcome.Timeout,
string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TestMethodOptions.TimeoutInfo.Timeout)),
string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TimeoutInfo.Timeout)),
};
}

Expand All @@ -1001,7 +1042,7 @@ private TestResult ExecuteInternalWithTimeout(object?[]? arguments)
TestFailureException = new TestFailedException(
UTFUnitTestOutcome.Timeout,
timeoutTokenSource.Token.IsCancellationRequested
? string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TestMethodOptions.TimeoutInfo.Timeout)
? string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TimeoutInfo.Timeout)
: string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Cancelled, TestMethodName)),
};
}
Expand All @@ -1016,7 +1057,7 @@ private TestResult ExecuteInternalWithTimeout(object?[]? arguments)
TestResult? result = null;
Exception? failure = null;

if (PlatformServiceProvider.Instance.ThreadOperations.Execute(ExecuteAsyncAction, TestMethodOptions.TimeoutInfo.Timeout, TestMethodOptions.TestContext!.Context.CancellationTokenSource.Token))
if (PlatformServiceProvider.Instance.ThreadOperations.Execute(ExecuteAsyncAction, TimeoutInfo.Timeout, TestContext!.Context.CancellationTokenSource.Token))
{
if (failure != null)
{
Expand All @@ -1032,15 +1073,15 @@ private TestResult ExecuteInternalWithTimeout(object?[]? arguments)
}

// Timed out or canceled
string errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TestMethodOptions.TimeoutInfo.Timeout);
if (TestMethodOptions.TestContext.Context.CancellationTokenSource.IsCancellationRequested)
string errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TimeoutInfo.Timeout);
if (TestContext.Context.CancellationTokenSource.IsCancellationRequested)
{
errorMessage = string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Cancelled, TestMethodName);
}
else
{
// Cancel the token source as test has timed out
TestMethodOptions.TestContext.Context.CancellationTokenSource.Cancel();
TestContext.Context.CancellationTokenSource.Cancel();
}

TestResult timeoutResult = new() { Outcome = UTF.UnitTestOutcome.Timeout, TestFailureException = new TestFailedException(UTFUnitTestOutcome.Timeout, errorMessage) };
Expand Down
8 changes: 4 additions & 4 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public TestMethodRunner(TestMethodInfo testMethodInfo, TestMethod testMethod, IT
internal TestResult[] Execute(string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
{
bool isSTATestClass = AttributeComparer.IsDerived<STATestClassAttribute>(_testMethodInfo.Parent.ClassAttribute);
bool isSTATestMethod = AttributeComparer.IsDerived<STATestMethodAttribute>(_testMethodInfo.TestMethodOptions.Executor);
bool isSTATestMethod = AttributeComparer.IsDerived<STATestMethodAttribute>(_testMethodInfo.Executor);
bool isSTARequested = isSTATestClass || isSTATestMethod;
bool isWindowsOS = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
if (isSTARequested && isWindowsOS && Thread.CurrentThread.GetApartmentState() != ApartmentState.STA)
Expand Down Expand Up @@ -161,7 +161,7 @@ internal TestResult[] RunTestMethod()
DebugEx.Assert(_testMethodInfo.TestMethod != null, "Test method should not be null.");

List<TestResult> results = [];
if (_testMethodInfo.TestMethodOptions.Executor == null)
if (_testMethodInfo.Executor == null)
{
throw ApplicationStateGuard.Unreachable();
}
Expand Down Expand Up @@ -457,7 +457,7 @@ private TestResult[] ExecuteTest(TestMethodInfo testMethodInfo)
{
try
{
return _testMethodInfo.TestMethodOptions.Executor.Execute(testMethodInfo);
return _testMethodInfo.Executor.Execute(testMethodInfo);
}
catch (Exception ex)
{
Expand All @@ -469,7 +469,7 @@ private TestResult[] ExecuteTest(TestMethodInfo testMethodInfo)
string.Format(
CultureInfo.CurrentCulture,
Resource.UTA_ExecuteThrewException,
_testMethodInfo.TestMethodOptions.Executor.GetType().FullName,
_testMethodInfo.Executor.GetType().FullName,
ex.ToString()),
ex),
},
Expand Down
Loading

0 comments on commit d141931

Please sign in to comment.