Welcome Guest! To enable all features please Login or Register.

Notification

Icon
Error

NCrunch hang with long running async test
jamezor
#1 Posted : Thursday, November 30, 2017 11:37:11 PM(UTC)
Rank: Member

Groups: Registered
Joined: 5/9/2012(UTC)
Posts: 10
Location: Auckland

Was thanked: 1 time(s) in 1 post(s)
With an NUnit test like so:

Code:
    [TestFixture]
    public class Class1
    {
        [Test]
        public async Task A_test_which_times_out_with_the_default_timeout()
        {
            await Task.Delay(TimeSpan.FromMinutes(2));
        }
    }


NCrunch will hang forever, while NUnit (via ReSharper) will take two minutes to complete successfully.

Using the latest NCrunch 3.12.0.15 on Windows 10 Enterprise 1709 with Visual Studio 2017 15.4.5.
Remco
#2 Posted : Friday, December 01, 2017 12:59:40 AM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 5,274

Thanks: 695 times
Was thanked: 856 time(s) in 814 post(s)
Hi, thanks for sharing this problem.

As you've deducted, this problem is being caused by NCrunch's timeout enforcement. As of now, NCrunch's enforcement under .NET Framework involves executing Thread.Abort on the thread responsible for executing the test. It looks like when NUnit runs an async test, there is no way to reliably establish which thread is being used for the test execution from outside of NUnit, so NCrunch is making its best guess and is aborting the wrong thread. This leaves the test to keep running, while the thread responsible for managing the execution disappears, resulting in a hung task runner.

I don't think there is a simple solution to this problem. We will need to rethink the way that timeouts are enforced for async tests.

To work around this, you'll need to do one of the following:
- Ensure you specify an NUnit 'Timeout' attribute with a sensible timeout for the test
- Set NCrunch's default timeout to something high enough that it won't be enforced for this test
- Avoid using NUnit async tests
jamezor
#3 Posted : Sunday, December 03, 2017 10:34:11 PM(UTC)
Rank: Member

Groups: Registered
Joined: 5/9/2012(UTC)
Posts: 10
Location: Auckland

Was thanked: 1 time(s) in 1 post(s)
Thank you for the quick response Remco.

I've worked around this issue for now by specifying a NUnit Timeout attribute at the assembly level. Since the NUnit timeout seems reliable for async tests, would it be possible for NCrunch to use that mechanism instead of the thread abort one?

Remco
#4 Posted : Sunday, December 03, 2017 10:44:03 PM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 5,274

Thanks: 695 times
Was thanked: 856 time(s) in 814 post(s)
jamezor;11600 wrote:

I've worked around this issue for now by specifying a NUnit Timeout attribute at the assembly level. Since the NUnit timeout seems reliable for async tests, would it be possible for NCrunch to use that mechanism instead of the thread abort one?


This is a very sensible workaround. Ideally having the test framework implementing the timeouts is always better, though test frameworks aren't really designed to use them in a blanket manner. The last time I checked, NUnit also wouldn't enforce timeouts under .NET Core (because Thread.Abort doesn't exist there), so there are some other considerations. Probably we'll go with enforcing all timeouts using Process.Kill instead, since this is much more reliable, even though it does have its own pitfalls.
jamezor
#5 Posted : Thursday, December 07, 2017 7:08:11 PM(UTC)
Rank: Member

Groups: Registered
Joined: 5/9/2012(UTC)
Posts: 10
Location: Auckland

Was thanked: 1 time(s) in 1 post(s)
Following up on this, I've attempted to set a NUnit Timeout attribute greater than the NCrunch timeout at the assembly level ala the following example:
Code:

[assembly: Timeout(90000)]

namespace NCrunchTest
{
    [TestFixture]
    public class Class1
    {
        [Test]
        public async Task A_test_with_the_default_timeout_which_will_get_killed_by_NCrunch_at_60_seconds_which_does_not_seem_right()
        {
            await Task.Delay(TimeSpan.FromMinutes(2));
        }

        [Test, Timeout(80000)]
        public async Task A_test_with_a_higher_than_default_timeout_which_will_get_killed_by_NUnit_at_80_seconds()
        {
            await Task.Delay(TimeSpan.FromMinutes(2));
        }
    }
}



But it appears the NUnit timeout at the assembly level is not being respected whereas at the individual test level it is.
Remco
#6 Posted : Thursday, December 07, 2017 9:59:41 PM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 5,274

Thanks: 695 times
Was thanked: 856 time(s) in 814 post(s)
jamezor;11618 wrote:
But it appears the NUnit timeout at the assembly level is not being respected whereas at the individual test level it is.


I was, admittedly, concerned about this. NCrunch interprets the timeout values as fed to it by NUnit, which in turn parses the attributes itself. For the timeout attribute to work correctly at assembly level, NUnit itself would need to support it there. Interestly, the attribute itself is marked as being applicable at assembly level, so I wonder if this has always been an intention for NUnit and maybe it was never implemented.

We're still looking at options for revising NCrunch's timeout enforcement, so it's my hope that from 3.13 onwards you won't need to do this. Specifying TimeoutAttribute on each long running async test is a messy but still effective workaround to this problem.
jamezor
#7 Posted : Friday, December 08, 2017 12:42:55 AM(UTC)
Rank: Member

Groups: Registered
Joined: 5/9/2012(UTC)
Posts: 10
Location: Auckland

Was thanked: 1 time(s) in 1 post(s)
When running in e.g. ReSharper, NUnit does apply the assembly level Timeout attribute correctly. So NUnit must support this feature to some degree. However if NUnit is not reporting the timeout values correctly to NCrunch that sounds like an issue with NUnit, you may wish to log an issue with them? Especially because even after the timeout enforcement is fixed in NCrunch, if NUnit reports incorrect timeouts then NCrunch will be operating incorrectly based on those misreported timeouts.
Remco
#8 Posted : Friday, December 08, 2017 1:40:03 AM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 5,274

Thanks: 695 times
Was thanked: 856 time(s) in 814 post(s)
Sorry, it looks like I was wrong.

NCrunch is inferring timeouts using the framework attributes itself, so it won't matter what NUnit reports. This isn't an NUnit problem.

The infrastructure here was built back in 2010, when NUnit didn't have an API and the whole integration was different. You'll need to use the Timeout attributes at either fixture or test level to work around this problem until it is fixed in NCrunch.
Users browsing this topic
Guest
Forum Jump  
You cannot post new topics in this forum.
You cannot reply to topics in this forum.
You cannot delete your posts in this forum.
You cannot edit your posts in this forum.
You cannot create polls in this forum.
You cannot vote in polls in this forum.

YAF | YAF © 2003-2011, Yet Another Forum.NET
This page was generated in 0.065 seconds.