Skip to content

Commit

Permalink
More extensive refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@Youssef1313
Youssef1313 committed Jan 10, 2025
1 parent 9f33522 commit 9e6070a
Show file tree
Hide file tree
Showing 6 changed files with 204 additions and 163 deletions.
82 changes: 40 additions & 42 deletions src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
Expand Down Expand Up @@ -31,7 +31,7 @@ public class TestClassInfo

private readonly Lock _testClassExecuteSyncObject = new();

private UnitTestResult? _classInitializeResult;
private TestResult? _classInitializeResult;

/// <summary>
/// Initializes a new instance of the <see cref="TestClassInfo"/> class.
Expand Down Expand Up @@ -346,37 +346,25 @@ public void RunClassInitialize(TestContext testContext)
throw testFailedException;
}

private UnitTestResult? TryGetClonedCachedClassInitializeResult()
{
private TestResult? TryGetClonedCachedClassInitializeResult()
// Historically, we were not caching class initialize result, and were always going through the logic in GetResultOrRunClassInitialize.
// When caching is introduced, we found out that using the cached instance can change the behavior in some cases. For example,
// if you have Console.WriteLine in class initialize, those will be present on the UnitTestResult.
// Before caching was introduced, these logs will be only in the first class initialize result (attached to the first test run in class)
// By re-using the cached instance, it's now part of all tests.
// To preserve the original behavior, we clone the cached instance so we keep only the information we are sure should be reused.
if (_classInitializeResult is null)
{
return null;
}

if (_classInitializeResult.ErrorStackTrace is not null && _classInitializeResult.ErrorMessage is not null)
{
return new(
new TestFailedException(
_classInitializeResult.Outcome,
_classInitializeResult.ErrorMessage,
new StackTraceInformation(_classInitializeResult.ErrorStackTrace, _classInitializeResult.ErrorFilePath, _classInitializeResult.ErrorLineNumber, _classInitializeResult.ErrorColumnNumber)));
}

// We are expecting this to be hit for the case of "Passed".
// It's unlikely to be hit otherwise (GetResultOrRunClassInitialize appears to only create either Passed results or a result from exception), but
// we can still create UnitTestResult from outcome and error message (which is expected to be null for Passed).
return new(_classInitializeResult.Outcome, _classInitializeResult.ErrorMessage);
}
=> _classInitializeResult is null
? null
: new()
{
Outcome = _classInitializeResult.Outcome,
IgnoreReason = _classInitializeResult.IgnoreReason,
TestFailureException = _classInitializeResult.TestFailureException,
};

internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext, string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
internal TestResult GetResultOrRunClassInitialize(ITestContext testContext, string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
{
UnitTestResult? clonedInitializeResult = TryGetClonedCachedClassInitializeResult();
TestResult? clonedInitializeResult = TryGetClonedCachedClassInitializeResult();

// Optimization: If we already ran before and know the result, return it.
if (clonedInitializeResult is not null)
Expand All @@ -393,7 +381,7 @@ internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext,
if (ClassInitializeMethod is null && BaseClassInitMethods.Count == 0)
{
IsClassInitializeExecuted = true;
return _classInitializeResult = new(ObjectModelUnitTestOutcome.Passed, null);
return _classInitializeResult = new() { Outcome = TestTools.UnitTesting.UnitTestOutcome.Passed };
}

bool isSTATestClass = AttributeComparer.IsDerived<STATestClassAttribute>(ClassAttribute);
Expand All @@ -402,7 +390,12 @@ internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext,
&& isWindowsOS
&& Thread.CurrentThread.GetApartmentState() != ApartmentState.STA)
{
UnitTestResult result = new(ObjectModelUnitTestOutcome.Error, "MSTest STATestClass ClassInitialize didn't complete");
var result = new TestResult()
{
Outcome = TestTools.UnitTesting.UnitTestOutcome.Error,
IgnoreReason = "MSTest STATestClass ClassInitialize didn't complete",
};

Thread entryPointThread = new(() => result = DoRun())
{
Name = "MSTest STATestClass ClassInitialize",
Expand All @@ -419,7 +412,10 @@ internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext,
catch (Exception ex)
{
PlatformServiceProvider.Instance.AdapterTraceLogger.LogError(ex.ToString());
return new UnitTestResult(new TestFailedException(ObjectModelUnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()));
return new TestResult()
{
TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()),
};
}
}
else
Expand All @@ -434,9 +430,12 @@ internal UnitTestResult GetResultOrRunClassInitialize(ITestContext testContext,
}

// Local functions
UnitTestResult DoRun()
TestResult DoRun()
{
UnitTestResult result = new(ObjectModelUnitTestOutcome.Passed, null);
var result = new TestResult()
{
Outcome = TestTools.UnitTesting.UnitTestOutcome.Passed,
};

try
{
Expand All @@ -456,17 +455,17 @@ UnitTestResult DoRun()
}
catch (TestFailedException ex)
{
result = new UnitTestResult(ex);
result = new TestResult() { TestFailureException = ex };
}
catch (Exception ex)
{
result = new UnitTestResult(new TestFailedException(ObjectModelUnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()));
result = new TestResult() { TestFailureException = new TestFailedException(ObjectModelUnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()) };
}
finally
{
// Assembly initialize and class initialize logs are pre-pended to the first result.
result.StandardOut = initializationLogs;
result.StandardError = initializationErrorLogs;
result.LogOutput = initializationLogs;
result.LogError = initializationErrorLogs;
result.DebugTrace = initializationTrace;
result.TestContextMessages = initializationTestContextMessages;
}
Expand Down Expand Up @@ -685,7 +684,7 @@ internal void ExecuteClassCleanup(TestContext testContext)
throw testFailedException;
}

internal void RunClassCleanup(ITestContext testContext, ClassCleanupManager classCleanupManager, TestMethodInfo testMethodInfo, TestMethod testMethod, UnitTestResult[] results)
internal void RunClassCleanup(ITestContext testContext, ClassCleanupManager classCleanupManager, TestMethodInfo testMethodInfo, TestMethod testMethod, TestResult[] results)
{
DebugEx.Assert(testMethodInfo.Parent == this, "Parent of testMethodInfo should be this TestClassInfo.");

Expand Down Expand Up @@ -765,22 +764,21 @@ void DoRun()
if (results.Length > 0)
{
#pragma warning disable IDE0056 // Use index operator
UnitTestResult lastResult = results[results.Length - 1];
TestResult lastResult = results[results.Length - 1];
#pragma warning restore IDE0056 // Use index operator
lastResult.Outcome = ObjectModelUnitTestOutcome.Error;
lastResult.ErrorMessage = ex.Message;
lastResult.ErrorStackTrace = ex.StackTrace;
lastResult.Outcome = TestTools.UnitTesting.UnitTestOutcome.Error;
lastResult.TestFailureException = ex;
}
}
finally
{
if (results.Length > 0)
{
#pragma warning disable IDE0056 // Use index operator
UnitTestResult lastResult = results[results.Length - 1];
TestResult lastResult = results[results.Length - 1];
#pragma warning restore IDE0056 // Use index operator
lastResult.StandardOut += initializationLogs;
lastResult.StandardError += initializationErrorLogs;
lastResult.LogOutput += initializationLogs;
lastResult.LogError += initializationErrorLogs;
lastResult.DebugTrace += initializationTrace;
lastResult.TestContextMessages += initializationTestContextMessages;
}
Expand Down
17 changes: 12 additions & 5 deletions src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ group test by test.Source into testGroup

internal virtual UnitTestDiscoverer GetUnitTestDiscoverer() => new();

internal void SendTestResults(TestCase test, IEnumerable<UnitTestResult> unitTestResults, DateTimeOffset startTime, DateTimeOffset endTime,
internal void SendTestResults(TestCase test, TestTools.UnitTesting.TestResult[] unitTestResults, DateTimeOffset startTime, DateTimeOffset endTime,
ITestExecutionRecorder testExecutionRecorder)
{
foreach (UnitTestResult unitTestResult in unitTestResults)
foreach (TestTools.UnitTesting.TestResult unitTestResult in unitTestResults)
{
_testRunCancellationToken?.ThrowIfCancellationRequested();

Expand Down Expand Up @@ -462,7 +462,7 @@ private void ExecuteTestsWithTestRunner(

// testRunner could be in a different AppDomain. We cannot pass the testExecutionRecorder directly.
// Instead, we pass a proxy (remoting object) that is marshallable by ref.
UnitTestResult[] unitTestResult = testRunner.RunSingleTest(unitTestElement.TestMethod, testContextProperties, remotingMessageLogger);
TestTools.UnitTesting.TestResult[] unitTestResult = testRunner.RunSingleTest(unitTestElement.TestMethod, testContextProperties, remotingMessageLogger);

PlatformServiceProvider.Instance.AdapterTraceLogger.LogInfo("Executed test {0}", unitTestElement.TestMethod.Name);

Expand All @@ -481,7 +481,11 @@ private void ExecuteTestsWithTestRunner(
// If there were only fixture tests, send an inconclusive result.
if (!hasAnyRunnableTests)
{
var result = new UnitTestResult(ObjectModel.UnitTestOutcome.Inconclusive, null);
var result = new TestTools.UnitTesting.TestResult()
{
Outcome = TestTools.UnitTesting.UnitTestOutcome.Inconclusive,
};

SendTestResults(currentTest, [result], DateTimeOffset.Now, DateTimeOffset.Now, testExecutionRecorder);
continue;
}
Expand All @@ -492,7 +496,10 @@ private void ExecuteTestsWithTestRunner(

if (fixtureTestResult.IsExecuted)
{
var result = new UnitTestResult(fixtureTestResult.Outcome, null);
var result = new TestTools.UnitTesting.TestResult()
{
Outcome = fixtureTestResult.Outcome.ToAdapterOutcome(),
};
SendTestResults(currentTest, [result], DateTimeOffset.Now, DateTimeOffset.Now, testExecutionRecorder);
}
}
Expand Down
28 changes: 14 additions & 14 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ public TestMethodRunner(TestMethodInfo testMethodInfo, TestMethod testMethod, IT
/// Executes a test.
/// </summary>
/// <returns>The test results.</returns>
internal List<TestResult> Execute(string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
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 isSTARequested = isSTATestClass || isSTATestMethod;
bool isWindowsOS = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
if (isSTARequested && isWindowsOS && Thread.CurrentThread.GetApartmentState() != ApartmentState.STA)
{
List<TestResult>? results = null;
TestResult[]? results = null;
Thread entryPointThread = new(() => results = SafeRunTestMethod(initializationLogs, initializationErrorLogs, initializationTrace, initializationTestContextMessages))
{
Name = (isSTATestClass, isSTATestMethod) switch
Expand All @@ -93,7 +93,7 @@ internal List<TestResult> Execute(string initializationLogs, string initializati
PlatformServiceProvider.Instance.AdapterTraceLogger.LogError(ex.ToString());
}

return results ?? new();
return results ?? Array.Empty<TestResult>();
}
else
{
Expand All @@ -107,9 +107,9 @@ internal List<TestResult> Execute(string initializationLogs, string initializati
}

// Local functions
List<TestResult> SafeRunTestMethod(string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
TestResult[] SafeRunTestMethod(string initializationLogs, string initializationErrorLogs, string initializationTrace, string initializationTestContextMessages)
{
List<TestResult>? result = null;
TestResult[]? result = null;

try
{
Expand All @@ -121,20 +121,20 @@ List<TestResult> SafeRunTestMethod(string initializationLogs, string initializat
}
catch (Exception ex)
{
if (result == null || result.Count == 0)
if (result == null || result.Length == 0)
{
result = [new TestResult() { Outcome = UTF.UnitTestOutcome.Error }];
}

#pragma warning disable IDE0056 // Use index operator
result[result.Count - 1] = new TestResult()
result[result.Length - 1] = new TestResult()
{
TestFailureException = new TestFailedException(UnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()),
LogOutput = result[result.Count - 1].LogOutput,
LogError = result[result.Count - 1].LogError,
DebugTrace = result[result.Count - 1].DebugTrace,
TestContextMessages = result[result.Count - 1].TestContextMessages,
Duration = result[result.Count - 1].Duration,
LogOutput = result[result.Length - 1].LogOutput,
LogError = result[result.Length - 1].LogError,
DebugTrace = result[result.Length - 1].DebugTrace,
TestContextMessages = result[result.Length - 1].TestContextMessages,
Duration = result[result.Length - 1].Duration,
};
#pragma warning restore IDE0056 // Use index operator
}
Expand All @@ -156,7 +156,7 @@ List<TestResult> SafeRunTestMethod(string initializationLogs, string initializat
/// Runs the test method.
/// </summary>
/// <returns>The test results.</returns>
internal List<TestResult> RunTestMethod()
internal TestResult[] RunTestMethod()
{
DebugEx.Assert(_test != null, "Test should not be null.");
DebugEx.Assert(_testMethodInfo.TestMethod != null, "Test method should not be null.");
Expand Down Expand Up @@ -244,7 +244,7 @@ internal List<TestResult> RunTestMethod()
results.Add(emptyResult);
}

return results;
return results.ToArray();
}

private bool TryExecuteDataSourceBasedTests(List<TestResult> results)
Expand Down
Loading

0 comments on commit 9e6070a

Please sign in to comment.