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.

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

Refactor some logic out of TypeCache #4786

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Refactor some logic out of TypeCache #4786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 74 additions & 33 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs
View file
Open in desktop
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; }

internal TestMethodAttribute Executor { get; }

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 @@ -86,11 +95,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 All @@ -116,7 +120,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 @@ -133,8 +137,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 @@ -218,6 +222,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 @@ -354,7 +395,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 @@ -364,7 +405,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 @@ -403,7 +444,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 @@ -579,12 +620,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 @@ -786,15 +827,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 @@ -807,15 +848,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 @@ -840,7 +881,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 @@ -878,7 +919,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 @@ -901,10 +942,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 @@ -935,21 +976,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 @@ -967,7 +1008,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 @@ -982,7 +1023,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 @@ -998,15 +1039,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
Open in desktop
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 @@ -427,7 +427,7 @@ private TestResult[] ExecuteTest(TestMethodInfo testMethodInfo)
{
try
{
return _testMethodInfo.TestMethodOptions.Executor.Execute(testMethodInfo);
return _testMethodInfo.Executor.Execute(testMethodInfo);
}
catch (Exception ex)
{
Expand All @@ -439,7 +439,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
Loading