Skip to content

Commit 6ededf0

Browse files
authored
Remove ClassCleanupBehavior.EndOfAssembly (#1570)
Fixes #1574
1 parent 2967e84 commit 6ededf0

File tree

34 files changed

+75
-1662
lines changed

34 files changed

+75
-1662
lines changed

src/Adapter/MSTest.TestAdapter/Execution/TestAssemblySettingsProvider.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,6 @@ internal static TestAssemblySettings GetSettings(string source)
4747

4848
testAssemblySettings.CanParallelizeAssembly = !ReflectHelper.IsDoNotParallelizeSet(testAssembly);
4949

50-
var classCleanupSequencingAttribute = ReflectHelper.GetClassCleanupAttribute(testAssembly);
51-
if (classCleanupSequencingAttribute != null)
52-
{
53-
testAssemblySettings.ClassCleanupLifecycle = classCleanupSequencingAttribute.CleanupBehavior;
54-
}
55-
5650
return testAssemblySettings;
5751
}
5852
}

src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -323,93 +323,6 @@ public void RunClassInitialize(TestContext testContext)
323323
throw testFailedException;
324324
}
325325

326-
/// <summary>
327-
/// Run class cleanup methods.
328-
/// </summary>
329-
/// <param name="classCleanupLifecycle">The current lifecycle position that ClassCleanup is executing from.</param>
330-
/// <returns>
331-
/// Any exception that can be thrown as part of a class cleanup as warning messages.
332-
/// </returns>
333-
public string? RunClassCleanup(ClassCleanupBehavior classCleanupLifecycle = ClassCleanupBehavior.EndOfClass)
334-
{
335-
if (ClassCleanupMethod is null && BaseClassInitAndCleanupMethods.All(p => p.Item2 == null))
336-
{
337-
return null;
338-
}
339-
340-
if (IsClassCleanupExecuted)
341-
{
342-
return null;
343-
}
344-
345-
lock (_testClassExecuteSyncObject)
346-
{
347-
if (IsClassCleanupExecuted)
348-
{
349-
return null;
350-
}
351-
352-
if (IsClassInitializeExecuted || ClassInitializeMethod is null)
353-
{
354-
MethodInfo? classCleanupMethod = null;
355-
356-
try
357-
{
358-
classCleanupMethod = ClassCleanupMethod;
359-
classCleanupMethod?.InvokeAsSynchronousTask(null);
360-
var baseClassCleanupQueue = new Queue<MethodInfo>(BaseClassCleanupMethodsStack);
361-
while (baseClassCleanupQueue.Count > 0)
362-
{
363-
classCleanupMethod = baseClassCleanupQueue.Dequeue();
364-
classCleanupMethod?.InvokeAsSynchronousTask(null);
365-
}
366-
367-
IsClassCleanupExecuted = true;
368-
369-
return null;
370-
}
371-
catch (Exception exception)
372-
{
373-
var realException = exception.InnerException ?? exception;
374-
ClassCleanupException = realException;
375-
376-
string errorMessage;
377-
378-
// special case AssertFailedException to trim off part of the stack trace
379-
if (realException is AssertFailedException or AssertInconclusiveException)
380-
{
381-
errorMessage = realException.Message;
382-
}
383-
else
384-
{
385-
errorMessage = StackTraceHelper.GetExceptionMessage(realException);
386-
}
387-
388-
var exceptionStackTraceInfo = realException.TryGetStackTraceInformation();
389-
390-
errorMessage = string.Format(
391-
CultureInfo.CurrentCulture,
392-
Resource.UTA_ClassCleanupMethodWasUnsuccesful,
393-
classCleanupMethod!.DeclaringType!.Name,
394-
classCleanupMethod.Name,
395-
errorMessage,
396-
exceptionStackTraceInfo?.ErrorStackTrace);
397-
398-
if (classCleanupLifecycle == ClassCleanupBehavior.EndOfClass)
399-
{
400-
var testFailedException = new TestFailedException(ObjectModelUnitTestOutcome.Failed, errorMessage, exceptionStackTraceInfo);
401-
ClassCleanupException = testFailedException;
402-
throw testFailedException;
403-
}
404-
405-
return errorMessage;
406-
}
407-
}
408-
}
409-
410-
return null;
411-
}
412-
413326
/// <summary>
414327
/// Execute current and base class cleanups.
415328
/// </summary>

src/Adapter/MSTest.TestAdapter/Execution/TestExecutionManager.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ private static void InitializeClassCleanupManager(string source, UnitTestRunner
360360
try
361361
{
362362
var unitTestElements = testsToRun.Select(e => e.ToUnitTestElement(source)).ToArray();
363-
testRunner.InitializeClassCleanupManager(unitTestElements, (int)sourceSettings.ClassCleanupLifecycle);
363+
testRunner.InitializeClassCleanupManager(unitTestElements);
364364
}
365365
catch (Exception ex)
366366
{

src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -85,21 +85,10 @@ public override object InitializeLifetimeService()
8585
/// and all inputs must be serializable from host process.
8686
/// </summary>
8787
/// <param name="testsToRun">the list of tests that will be run in this execution.</param>
88-
/// <param name="classCleanupLifecycle">The assembly level class cleanup lifecycle.</param>
89-
internal void InitializeClassCleanupManager(ICollection<UnitTestElement> testsToRun, int? classCleanupLifecycle)
88+
internal void InitializeClassCleanupManager(ICollection<UnitTestElement> testsToRun)
9089
{
91-
// We can't transport the Enum across AppDomain boundaries because of backwards and forwards compatibility.
92-
// So we're converting here if we can, or falling back to the default.
93-
var lifecycle = ClassCleanupBehavior.EndOfClass;
94-
if (classCleanupLifecycle != null && Enum.IsDefined(typeof(ClassCleanupBehavior), classCleanupLifecycle))
95-
{
96-
lifecycle = (ClassCleanupBehavior)classCleanupLifecycle;
97-
}
98-
9990
_classCleanupManager = new ClassCleanupManager(
10091
testsToRun,
101-
MSTestSettings.CurrentSettings.ClassCleanupLifecycle,
102-
lifecycle,
10392
_reflectHelper);
10493
}
10594

@@ -286,24 +275,18 @@ private bool IsTestMethodRunnable(
286275

287276
private class ClassCleanupManager
288277
{
289-
private readonly ClassCleanupBehavior? _lifecycleFromMsTest;
290-
private readonly ClassCleanupBehavior _lifecycleFromAssembly;
291278
private readonly ReflectHelper _reflectHelper;
292279
private readonly ConcurrentDictionary<string, HashSet<string>> _remainingTestsByClass;
293280

294281
public ClassCleanupManager(
295282
IEnumerable<UnitTestElement> testsToRun,
296-
ClassCleanupBehavior? lifecycleFromMsTest,
297-
ClassCleanupBehavior lifecycleFromAssembly,
298283
ReflectHelper? reflectHelper = null)
299284
{
300285
_remainingTestsByClass =
301286
new(testsToRun.GroupBy(t => t.TestMethod.FullClassName)
302287
.ToDictionary(
303288
g => g.Key,
304289
g => new HashSet<string>(g.Select(t => t.TestMethod.UniqueName))));
305-
_lifecycleFromMsTest = lifecycleFromMsTest;
306-
_lifecycleFromAssembly = lifecycleFromAssembly;
307290
_reflectHelper = reflectHelper ?? new ReflectHelper();
308291
}
309292

@@ -325,11 +308,7 @@ public void MarkTestComplete(TestMethodInfo testMethodInfo, TestMethod testMetho
325308
_remainingTestsByClass.TryRemove(testMethodInfo.TestClassName, out _);
326309
if (testMethodInfo.Parent.HasExecutableCleanupMethod)
327310
{
328-
var cleanupLifecycle = _reflectHelper.GetClassCleanupBehavior(testMethodInfo.Parent)
329-
?? _lifecycleFromMsTest
330-
?? _lifecycleFromAssembly;
331-
332-
shouldRunEndOfClassCleanup = cleanupLifecycle == ClassCleanupBehavior.EndOfClass;
311+
shouldRunEndOfClassCleanup = true;
333312
}
334313
}
335314

src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,6 @@ internal static bool IsDoNotParallelizeSet(Assembly assembly)
347347
=> PlatformServiceProvider.Instance.ReflectionOperations.GetCustomAttributes(assembly, typeof(DoNotParallelizeAttribute))
348348
!.Any(); // TODO: Investigate if we rely on NRE
349349

350-
/// <summary>
351-
/// Gets the class cleanup lifecycle set on an assembly.
352-
/// </summary>
353-
/// <param name="assembly"> The test assembly. </param>
354-
/// <returns> The class cleanup lifecycle attribute if set. null otherwise. </returns>
355-
internal static ClassCleanupExecutionAttribute? GetClassCleanupAttribute(Assembly assembly)
356-
=> PlatformServiceProvider.Instance.ReflectionOperations.GetCustomAttributes(assembly, typeof(ClassCleanupExecutionAttribute))
357-
!.OfType<ClassCleanupExecutionAttribute>() // TODO: Investigate if we rely on NRE
358-
.FirstOrDefault();
359-
360350
/// <summary>
361351
/// Gets custom attributes at the class and assembly for a method.
362352
/// </summary>
@@ -477,39 +467,6 @@ internal virtual TAttribute[] GetCustomAttributes<TAttribute>(MemberInfo attribu
477467
: ignoreAttribute[0].IgnoreMessage;
478468
}
479469

480-
/// <summary>
481-
/// Gets the class cleanup lifecycle for the class, if set.
482-
/// </summary>
483-
/// <param name="classInfo">The class to inspect.</param>
484-
/// <returns>Returns <see cref="ClassCleanupBehavior"/> if provided, otherwise <c>null</c>.</returns>
485-
internal virtual ClassCleanupBehavior? GetClassCleanupBehavior(TestClassInfo classInfo)
486-
{
487-
if (!classInfo.HasExecutableCleanupMethod)
488-
{
489-
return null;
490-
}
491-
492-
var cleanupBehaviors =
493-
new HashSet<ClassCleanupBehavior?>(
494-
classInfo.BaseClassCleanupMethodsStack
495-
.Select(x => x.GetCustomAttribute<ClassCleanupAttribute>(true)?.CleanupBehavior))
496-
{
497-
classInfo.ClassCleanupMethod?.GetCustomAttribute<ClassCleanupAttribute>(true)?.CleanupBehavior,
498-
};
499-
500-
if (cleanupBehaviors.Contains(ClassCleanupBehavior.EndOfClass))
501-
{
502-
return ClassCleanupBehavior.EndOfClass;
503-
}
504-
505-
if (cleanupBehaviors.Contains(ClassCleanupBehavior.EndOfAssembly))
506-
{
507-
return ClassCleanupBehavior.EndOfAssembly;
508-
}
509-
510-
return null;
511-
}
512-
513470
/// <summary>
514471
/// KeyValue pairs that are provided by TestPropertyAttributes of the given test method.
515472
/// </summary>

src/Adapter/MSTest.TestAdapter/MSTestSettings.cs

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,6 @@ public static RunConfigurationSettings RunConfigurationSettings
118118
/// </summary>
119119
public bool EnableBaseClassTestMethodsFromOtherAssemblies { get; private set; }
120120

121-
/// <summary>
122-
/// Gets a value indicating where class cleanup should occur.
123-
/// </summary>
124-
public ClassCleanupBehavior? ClassCleanupLifecycle { get; private set; }
125-
126121
/// <summary>
127122
/// Gets the number of threads/workers to be used for parallelization.
128123
/// </summary>
@@ -169,7 +164,6 @@ public static void PopulateSettings(MSTestSettings settings)
169164
CurrentSettings.MapNotRunnableToFailed = settings.MapNotRunnableToFailed;
170165
CurrentSettings.TreatDiscoveryWarningsAsErrors = settings.TreatDiscoveryWarningsAsErrors;
171166
CurrentSettings.EnableBaseClassTestMethodsFromOtherAssemblies = settings.EnableBaseClassTestMethodsFromOtherAssemblies;
172-
CurrentSettings.ClassCleanupLifecycle = settings.ClassCleanupLifecycle;
173167
CurrentSettings.ParallelizationWorkers = settings.ParallelizationWorkers;
174168
CurrentSettings.ParallelizationScope = settings.ParallelizationScope;
175169
CurrentSettings.DisableParallelization = settings.DisableParallelization;
@@ -348,26 +342,6 @@ private static MSTestSettings ToSettings(XmlReader reader)
348342
break;
349343
}
350344

351-
case "CLASSCLEANUPLIFECYCLE":
352-
{
353-
var value = reader.ReadInnerXml();
354-
if (TryParseEnum(value, out ClassCleanupBehavior lifecycle))
355-
{
356-
settings.ClassCleanupLifecycle = lifecycle;
357-
}
358-
else
359-
{
360-
throw new AdapterSettingsException(
361-
string.Format(
362-
CultureInfo.CurrentCulture,
363-
Resource.InvalidClassCleanupLifecycleValue,
364-
value,
365-
string.Join(", ", Enum.GetNames(typeof(ClassCleanupBehavior)))));
366-
}
367-
368-
break;
369-
}
370-
371345
case "FORCEDLEGACYMODE":
372346
{
373347
if (bool.TryParse(reader.ReadInnerXml(), out result))

src/Adapter/MSTest.TestAdapter/ObjectModel/TestAssemblySettings.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,4 @@ public TestAssemblySettings()
2828
/// Gets or sets a value indicating whether the assembly can be parallelized.
2929
/// </summary>
3030
internal bool CanParallelizeAssembly { get; set; }
31-
32-
/// <summary>
33-
/// Gets or sets the class cleanup lifecycle timing.
34-
/// </summary>
35-
internal ClassCleanupBehavior ClassCleanupLifecycle { get; set; }
3631
}

src/Adapter/MSTest.TestAdapter/PublicAPI/PublicAPI.Shipped.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.H
3838
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.IsClassCleanupExecuted.get -> bool
3939
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.IsClassInitializeExecuted.get -> bool
4040
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.Parent.get -> Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestAssemblyInfo!
41-
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.RunClassCleanup(Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupBehavior classCleanupLifecycle = Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupBehavior.EndOfClass) -> string?
4241
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.RunClassInitialize(Microsoft.VisualStudio.TestTools.UnitTesting.TestContext! testContext) -> void
4342
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.TestCleanupMethod.get -> System.Reflection.MethodInfo?
4443
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution.TestClassInfo.TestContextProperty.get -> System.Reflection.PropertyInfo?
@@ -73,7 +72,6 @@ Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.TestExecut
7372
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestExecutor.TestExecutionManager.set -> void
7473
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings
7574
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.CaptureDebugTraces.get -> bool
76-
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.ClassCleanupLifecycle.get -> Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupBehavior?
7775
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.DisableParallelization.get -> bool
7876
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.EnableBaseClassTestMethodsFromOtherAssemblies.get -> bool
7977
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.ForcedLegacyMode.get -> bool

src/TestFramework/TestFramework/Attributes/Lifecycle/Cleanup/ClassCleanupAttribute.cs

Lines changed: 1 addition & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public sealed class ClassCleanupAttribute : Attribute
1515
/// Initializes a new instance of the <see cref="ClassCleanupAttribute"/> class.
1616
/// </summary>
1717
public ClassCleanupAttribute()
18-
: this(InheritanceBehavior.None, null)
18+
: this(InheritanceBehavior.None)
1919
{
2020
}
2121

@@ -26,63 +26,12 @@ public ClassCleanupAttribute()
2626
/// Specifies the ClassCleanup Inheritance Behavior.
2727
/// </param>
2828
public ClassCleanupAttribute(InheritanceBehavior inheritanceBehavior)
29-
: this(inheritanceBehavior, null)
30-
{
31-
}
32-
33-
/// <summary>
34-
/// Initializes a new instance of the <see cref="ClassCleanupAttribute"/> class.
35-
/// </summary>
36-
/// <param name="cleanupBehavior">
37-
/// Specifies the class clean-up behavior.
38-
/// To capture output of class clean-up method in logs
39-
/// <paramref name="cleanupBehavior"/> must be set to <see cref="ClassCleanupBehavior.EndOfClass"/>.
40-
/// </param>
41-
public ClassCleanupAttribute(ClassCleanupBehavior cleanupBehavior)
42-
: this(InheritanceBehavior.None, cleanupBehavior)
43-
{
44-
}
45-
46-
/// <summary>
47-
/// Initializes a new instance of the <see cref="ClassCleanupAttribute"/> class.
48-
/// </summary>
49-
/// <param name="inheritanceBehavior">
50-
/// Specifies the ClassCleanup Inheritance Behavior.
51-
/// </param>
52-
/// <param name="cleanupBehavior">
53-
/// Specifies the class clean-up behavior.
54-
/// To capture output of class clean-up method in logs
55-
/// <paramref name="cleanupBehavior"/> must be set to <see cref="ClassCleanupBehavior.EndOfClass"/>.
56-
/// </param>
57-
public ClassCleanupAttribute(InheritanceBehavior inheritanceBehavior, ClassCleanupBehavior cleanupBehavior)
58-
: this(inheritanceBehavior, new ClassCleanupBehavior?(cleanupBehavior))
59-
{
60-
}
61-
62-
/// <summary>
63-
/// Initializes a new instance of the <see cref="ClassCleanupAttribute"/> class.
64-
/// </summary>
65-
/// <param name="inheritanceBehavior">
66-
/// Specifies the ClassCleanup Inheritance Behavior.
67-
/// </param>
68-
/// <param name="cleanupBehavior">
69-
/// Specifies the class clean-up behavior.
70-
/// To capture output of class clean-up method in logs
71-
/// <paramref name="cleanupBehavior"/> must be set to <see cref="ClassCleanupBehavior.EndOfClass"/>.
72-
/// </param>
73-
private ClassCleanupAttribute(InheritanceBehavior inheritanceBehavior, ClassCleanupBehavior? cleanupBehavior)
7429
{
7530
InheritanceBehavior = inheritanceBehavior;
76-
CleanupBehavior = cleanupBehavior;
7731
}
7832

7933
/// <summary>
8034
/// Gets the Inheritance Behavior.
8135
/// </summary>
8236
public InheritanceBehavior InheritanceBehavior { get; }
83-
84-
/// <summary>
85-
/// Gets when to run class cleanup methods.
86-
/// </summary>
87-
public ClassCleanupBehavior? CleanupBehavior { get; }
8837
}

0 commit comments

Comments
 (0)