Timeout should apply to test method including its initialize and cleanup

[Youssef1313]

The suggestion in #522 which was implemented in #3334 doesn't sound reasonable to me and adds unnecessary complications.

The timeout applies to the whole test method including initialize and cleanup. If it timed out, we shouldn't attempt to run cleanup. Today, we will run the class cleanup and wait for it. If it took forever, we will be waiting and will have exceeded the test timeout. Try to run the following today:

[TestClass]
public class TestCleanup
{
    [TestInitialize]
    public void MyTestInitialize()
    {
        Console.WriteLine("Calling initialize");
    }

    [TestCleanup]
    public void MyTestCleanup()
    {
        Thread.Sleep(1000000);
    }

    [Timeout(5000)]
    [TestMethod]
    public void TestMethod1()
    {
        Console.WriteLine("Sleeping for 10 seconds");
        System.Threading.Thread.Sleep(10000);
        Console.WriteLine("Waking up");
    }

}

I would expect the test to timeout after ~5 seconds. But because we run test cleanup, we are waiting on the large sleep in it.

[nohwnd]

I don't think skipping cleanup is the right behavior.

Cleanup is there specifically to guarantee that the cleanup will run (as much as possible, we are not talking about turning the power off). It is not primarily about code re-use (as test init), but about being able to clean up.

We also need to think how this will effectively happen, .NET core does not allow the threads to abort, so rather than time out and abort, we are discussing the correct point when to abandon the work to the background, and report that it did not finish on time.

If this was the last test in the assembly we might tear down the process and not run the cleanup, this makes it unpredictable.

So unless the desire is to abort the process right away (which did happen in the past in MSTest and was constant source of confusion and complaints), we are not gaining additional correctness of not corrupting the state of the process by not running the cleanup.

Personally I think that the timeout should apply to the whole test init, test body and test cleanup, and report if they take longer than the sum or their runtimes. When init times out, we should progress to cleanup. When body times out we should progress to cleanup. When cleanup takes too long, it should report in other way to keep the guarantee of running the cleanup.

If the user (or IDE) does not desire to await that cleanup, and subsequents class and assembly cleanups, they should use an 'abort' (e.g. a session time out or hang dump) which guarantees that the work stops right now, no matter the mess it leaves behind.

[Youssef1313]

@nohwnd So for the code sample above, what is your expectation?

• Cleanup taking its full sleep amount?
• Timeout counter resetting when cleanup starts and times out after 5 seconds? i.e, test method times out after 5 seconds, then cleanup times out after 5 seconds, with total duration of the test to be 10 seconds?