Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
75 changes: 46 additions & 29 deletions src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,15 @@ internal set
}

/// <summary>
/// Gets a value indicating whether is class initialize executed.
/// Gets a value indicating whether class initialize has executed.
/// </summary>
public bool IsClassInitializeExecuted { get; internal set; }

/// <summary>
/// Gets a value indicating whether class cleanup has executed.
/// </summary>
public bool IsClassCleanupExecuted { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required?


/// <summary>
/// Gets the exception thrown during <see cref="ClassInitializeAttribute"/> method invocation.
/// </summary>
Expand Down Expand Up @@ -306,40 +311,52 @@ public string RunClassCleanup()
return null;
}

lock (this.testClassExecuteSyncObject)
if (this.IsClassInitializeExecuted)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: how about this.ClassCleanupMethod == null || this.IsClassInitializeExecuted == false?

{
try
{
this.ClassCleanupMethod.InvokeAsSynchronousTask(null);

return null;
}
catch (Exception exception)
lock (this.testClassExecuteSyncObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lock can be removed? All clean ups done in single thread. /cc @AbhitejJohn

{
var realException = exception.InnerException ?? exception;

string errorMessage;

// special case AssertFailedException to trim off part of the stack trace
if (realException is AssertFailedException ||
realException is AssertInconclusiveException)
{
errorMessage = realException.Message;
}
else
if (this.IsClassInitializeExecuted)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsClassInitializeExecuted [](start = 29, length = 25)

why another check needed inside?

{
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
}
try
{
this.ClassCleanupMethod.InvokeAsSynchronousTask(null);

return string.Format(
CultureInfo.CurrentCulture,
Resource.UTA_ClassCleanupMethodWasUnsuccesful,
this.ClassType.Name,
this.ClassCleanupMethod.Name,
errorMessage,
StackTraceHelper.GetStackTraceInformation(realException)?.ErrorStackTrace);
return null;
}
catch (Exception exception)
{
var realException = exception.InnerException ?? exception;

string errorMessage;

// special case AssertFailedException to trim off part of the stack trace
if (realException is AssertFailedException ||
realException is AssertInconclusiveException)
{
errorMessage = realException.Message;
}
else
{
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
}

return string.Format(
CultureInfo.CurrentCulture,
Resource.UTA_ClassCleanupMethodWasUnsuccesful,
this.ClassType.Name,
this.ClassCleanupMethod.Name,
errorMessage,
StackTraceHelper.GetStackTraceInformation(realException)?.ErrorStackTrace);
}
finally
{
this.IsClassCleanupExecuted = true;
}
}
}
}

return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ public void TestClassInfoClassCleanupMethodSetShouldThrowForMultipleClassCleanup
ActionUtility.ActionShouldThrowExceptionOfType(action, typeof(TypeInspectionException));
}

[TestMethod]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a negative test case. Can we add a positive test case as well?

public void TestClassInfoClassCleanupMethodShouldNotInvokeWhenNoTestClassInitializedIsCalled()
{
this.testClassInfo.ClassCleanupMethod = this.testClassType.GetMethods().First();
this.testClassInfo.ClassInitializeMethod = this.testClassType.GetMethods()[1];

var ret = this.testClassInfo.RunClassCleanup(); // call cleanup without calling init

Assert.AreEqual(null, ret);
Assert.AreEqual(false, this.testClassInfo.IsClassCleanupExecuted);
}

[TestMethod]
public void TestClassInfoHasExecutableCleanupMethodShouldReturnFalseIfClassDoesNotHaveCleanupMethod()
{
Expand Down