Improvement: ClassCleanup method to trigger after worker has executed all test methods not after all workers are complete

[mills-andrew]

When running multiple [TestClass]es at the same time, why does the [ClassCleanup()] methods only get called at the very end?

File: UnitTest1.cs

    [TestClass]
    public class UnitTest1
    {
        [ClassInitialize()]
        public static void ClassInit(TestContext context) { }
    
        [ClassCleanup()]
        public static void ClassCleanup() { }
    
        [TestMethod]
        public void TestMethod() { }
    }

File: UnitTest2.cs

    [TestClass]
    public class UnitTest2
    {
        [ClassInitialize()]
        public static void ClassInit(TestContext context) { }
    
        [ClassCleanup()]
        public static void ClassCleanup() { }
    
        [TestMethod]
        public void TestMethod() { }
    }

The execution order is the following:

1. UnitTest2.ClassInit()
2. UnitTest2.TestMethod()
3. UnitTest1.ClassInit()
4. UnitTest1.TestMethod()
5. UnitTest2.ClassCleanup()
6. UnitTest1.ClassCleanup()

Notice that both [ClassCleanup] methods are executed at the end of the set; not after each TestClass is "done".

But I expected a different behavior:

1. UnitTest2.ClassInit()
2. UnitTest2.TestMethod()
3. UnitTest2.ClassCleanup() - Expected
4. UnitTest1.ClassInit()
5. UnitTest1.TestMethod()
6. UnitTest1.ClassCleanup() - Expected

Microsoft's Documentation - ClassCleanupAttribute Class says, "Identifies a method that contains code to be used after ALL the tests in the test class have run and to free resources obtained by the test class."

But it appears to be run/executed late, after ALL tests have run. This seems wrong, and it is blocking my development.

[cltshivash]

Assembly and class cleanup today run at the end. Could you please elaborate on why this would block your development ?

[ShreyasRmsft]

@KitoCoding any updates here?

[mills-andrew]

once a class has run all of its test cases, there is no reason to not run the test class clean up method right away. the test class is complete, so run the method... it may just be the way I imagining test cases, but it will make parallel tests much easier.

[mills-andrew]

For example, lets say i have 50 test classes, and since msTest doesn't run test classes or methods in order, I have my first test method run and its the only method in the class. I have to wait for 50 test classes to complete running before I am able to run the test class clean up method. Thats just silly

[cltshivash]

@KitoCoding Are you dependent on the cleanup done in one test class in the cleanup/initialize of another test class ? What does your cleanup do ?

[cfletcher]

@cltshivash In my use case we're running acceptance tests and our ClassCleanup is cleaning up test mocks. Cleaning them up earlier would reduce our chance of mock collisions and make debugging test issues easier (less concurrent mocks).
I wouldn't say it's blocking development for me, but it would be a valuable improvement. I realise we could just run that code outside of the cleanup segment, say in the ClassInitialize but its not init code its cleanup code

[vlaskal]

Hi @cltshivash,
I have to join to this request since I run into trouble when ClassInitialize method allocate limited resources. In my case it is Selenium WebDriver, which we have around 4 nodes with 5 workers. All tests are parallelized and execution scope is ClassLevel. When you run test then first 20 test classes are executed and the rest is not because the resources is not released and new cannot be taken. Test class holds instance and wait until all other test fails.
I think is good practice to release resources as soon as possible therefore ClassCleanup should be executed when all tests from test class are done.
And also ClassCleanup should be executed in same thread as ClassInitialize and all test methods of the class. It is executed in different thread than tests right now.

[cltshivash]

@vlaskal @cfletcher Thanks for the details. It helps provide the context. If one of you can contribute this change that would be great.

[vlaskal]

@cltshivash I can try but also some advice would be good. I already look into the code and found a trouble with DoNoTParallelize attribute. I think even this test should be done between same ClassInitialize and ClassCleanup and maybe also on same thread (right now it is on different thread). Right now it is executed after all parallel tests in project.
So are we able to describe somehow how it should work with DoNotParallelize attribute?
Then it is possible to start implement this.

[pleonex]

In my case this a blocking point for using MSTest. Our test classes create an HTTPListener for a specific port. Since ClassCleanup is not called after the tests of the class run, we cannot reuse the port between test classes (you cannot bind twice to the same port). We cannot use use different ports either because of Windows, it requires to allow binding to the port with netsh.

netsh http add urlacl url=http://+:<theport>/<url>/ user=DOMAIN\USER

This mean, that for every test class that we add, we need to use a new port so every developer needs to run again the netsh command for the new port.

In other project we have a similar issue because we use a third-party that it's binding in UDP/TCP ports, if it cannot bind in a port it tries within a range, but that range is no longer large enough because of the number of test classes we have.

The workaround is to move the initialize and clean-up code per class to per test, making the tests slower.

I think that any project using networking will find this as a real issue, because we cannot release the resources between test classes.

[mills-andrew]

I would just like to know the reasons as to why class clean up is triggered after all workers are complete. i understand if you are running parralelle runs at a method level. Sure, with no way for MSTest v2 to order the test cases, then it makes sense to just run after all workers are complete. But you supply the option to run at a test class level. One worker per test class. This is what we use, A worker should run the test init, all its methods then its clean up.

[mills-andrew]

Implementing with DoNoTParallelize attribute might be tricky, if thats the case. A warning should go out to the user. "TestCleanUp cannot be triggered until all Workers are complete due to the DoNoTParallelize attribute found [insert location]."

If a worker is scheduled to run at a test class level and all its methods are good to go (does not have the DoNoTParallelize attribute) then trigger the class clean up.

If a worker encounters a DoNoTParallelize attribute, then that worker cannot trigger test clean up until the very end.

But my methods assume workers understand the order in which they are going to work, and they also are aware of other workers and their jobs.

Essentially:

If a worker encounters a TestMethod with the DoNoTParallelize attribute, then do not trigger its TestClassClean up method until all workers are complete. Make sure to log warning so the user knows why its clean ups are not triggering

[kfrajtak]

What?! First, why is this not documented and second, I don't fully understand the decision to implement it this way.

[mills-andrew]

It is not documented because it is a bug. They have admitted that this isn't as designed but its a low priority.

[kfrajtak]

Well, if there is a bug that completely changes the test life cycle I would expect it to be well documented.

[vlaskal]

I already look at source code and found that it is by code design. Of course it is a bug. But code is written this way.

1. Collect all test cases
2. Split collection into two based on DoNotParallel flag.
3. Group parallel collection based on ExecutionScope flag.
4. Create queue of groups
5. Create threads where each
   a. Take first from queue
   b. Check assembly initialize execution and run in case is not executed yet
   c. Check class initialize execution and run in case is not executed yet
   d. Execute test method
6. Wait for all threads
7. Run all not parallel tests
8. Run all class cleanups
9. Run assembly cleanup

It is simple and straight forward design. And I think there will be lots of issues when you try to change it into proper flow.

[vlaskal]

I would like to know what is expected behavior in following scenario.
Let's imagine:

• 10 test classes each with several test methods and some of them are flagged by DoNotParallel attribute. At least one method in each class.
• Execution level is set to MethodLevel
• Number of workers/threads is set to 5

[kfrajtak]

I have also found that the test class constructor is invoked before each test.

[ScottMichaud]

Assembly and class cleanup today run at the end. Could you please elaborate on why this would block your development ?

It looks like the thread has already made some valid use cases, but here's another example: Using Appium.WebDriver to run integration tests on a single-instance application with non-trivial load and close.

I would expect that each TestClass could be used to launch the application into a specific state, run the unit tests on that state, then close the application. Because the ClassCleanup doesn't run before the next class attempts to start testing, the driver attempts to launch the application a second time in a different mode while the first instance is still open.

We're currently using TestInitialize and TestCleanup to allow a "Run All" to succeed, but this significantly limits our ability to break apart our integration tests into separate, well defined TestMethods due to how much execution time it adds.

Of course, initialization and clean-up at arbitrary levels of granularity (ex: groups) could help minimize execution time further, by batching tests to minimize state changes within an application run, but TestClasses would cover most of that need (by cutting out the load and shutdown cycle).

[RyanClementsHax]

Assembly and class cleanup today run at the end. Could you please elaborate on why this would block your development ?

My team is also experiencing an issue with this design of when ClassCleanup is called. I figured I would contribute our use case for this thread.

For us, we are developing database integration tests where each test class tests a partition of our database access code. We could force all of these tests to run sequentially, resetting the database after each test run, but this is extremely expensive. To allow for more parallelism and avoid having to reset so often, we want to create a test database for each one of these test classes. Creating one database per test class (thus per partition of our entire app) is too much. This lead us to a solution where we create a fixed set of databases these classes can use, reset after they are done, and release the database connection back to the pool for other test classes to use. With ClassCleanup happening after all other tests classes run, these test classes will never have the chance to release their resources for other test classes to use. As a result, the lack of this feature is keeping us from using this library even though we really love it and is familiar with everything else we do. Hope this helps.

[mills-andrew]

Been open for a while, any update?

[BoMarley]

any updates?

[ptrace]

this would be very useful for me as well

[dibyamani]

This is very helpful while working with database tests. Each ClassInitialize creates necessary tables, storedprocedures, feeding data and other necessary objects in sql server database and it should have provision to drop them withing the same classes's ClassCleanup. If we are not able to do so, another test class might be using the same object and it is failed.

[xuxiaotun]

Is it the same reason why we can't map the classcleanup log to the last testmethod?

[Haplois]

This is fixed in #968 and released with 2.2.8. To enable it you can set ClassCleanupBehavior to ClassCleanupBehavior.EndOfClass in your ClassCleanup attribute.

This can also be set on assembly level by ClassCleanupExecution assembly level attribute.

[nir-boger]

Thank you!

[sergiuciudin]

Seems it's not working for me, anyone tried it?

[ailn]

Didn't work for me as well

[nohwnd]

Tried it again. Works fine for me. Did you specify the correct behavior to the attribute?

C:\t\mstest35> dotnet test; cat bin\Debug\net6.0\log.txt
  Determining projects to restore...
  Restored C:\t\mstest35\mstest35.csproj (in 481 ms).
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
  mstest35 -> C:\t\mstest35\bin\Debug\net6.0\mstest35.dll
Test run for C:\t\mstest35\bin\Debug\net6.0\mstest35.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.2.0-preview-20211210-01
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 1 s - mstest35.dll (net6.0)
UnitTest1 start.
UnitTest1 done.
UnitTest2 start.
UnitTest2 done.

<!-- file mstest35.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
        <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.8" />
    <PackageReference Include="coverlet.collector" Version="3.1.0" />
  </ItemGroup>

</Project>

// file UnitTest1.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.IO;
using System.Threading;

[assembly: ClassCleanupExecution(ClassCleanupBehavior.EndOfClass)]

namespace mstest35;

public static class LogHelper
{
    public static void AppendLine(string text) => File.AppendAllText(
            Path.Combine(AppContext.BaseDirectory, "log.txt"),
            text + "\n");
}

[TestClass]
public class UnitTest1
{
    [ClassCleanup]
    public static void Cleanup()
    {
        LogHelper.AppendLine($"{nameof(UnitTest1)} done.");
    }

    [TestMethod]
    public void TestMethod1()
    {
        LogHelper.AppendLine($"{nameof(UnitTest1)} start.");
    }
}

[TestClass]
public class UnitTest2
{
    [ClassCleanup]
    public static void Cleanup()
    {
        LogHelper.AppendLine($"{nameof(UnitTest2)} done.");

    }

    [TestMethod]
    public void TestMethod1()
    {
        LogHelper.AppendLine($"{nameof(UnitTest2)} start.");
        Thread.Sleep(1000);
    }
}