nunit: Timeout value not resetting on Retry of failed test
Not sure if this has been raised before, couldn’t find a similar issue.
I have a test with both Retry and Timeout attribute but when a test is retried the timeout isn’t being reset to 0.
using NUnit.Framework;
using System;
using System.Threading;
namespace RetryTimeoutTest
{
public class Class1
{
private static int count1 = 0;
private static int count2 = 0;
[Test]
[Retry(3)]
public void Test1()
{
count1++;
Console.WriteLine(count1);
Thread.Sleep(2000);
if (count1 < 2)
{
Console.WriteLine("In failing assert");
Assert.True(false);
}
else
{
Console.WriteLine("In passing assert");
Assert.True(true);
}
}
[Test]
[Retry(3)]
[Timeout(3000)]
public void Test2()
{
count2++;
Console.WriteLine(count2);
Thread.Sleep(2000);
if (count2 < 2)
{
Console.WriteLine("In failing assert");
Assert.True(false);
}
else
{
Console.WriteLine("In passing assert");
Assert.True(true);
}
}
}
}
Expected Result: Both tests to pass after a retry
Actual Result Test1 passes as expected but Test2 fails with message Test exceeded Timeout value of 3000ms.
It looks like the timeout isn’t being reset when the test starts the second time, is this the expected result or should a reset be performed for each retry of the tests.
ps sorry about the code sample, doesn’t look to be formatting everything
@ChrisMaddock edit: fixed formatting
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 22 (22 by maintainers)
Commits related to this issue
- #2786 - Fix for Timeout not resetting on retry and repeat. — committed to DaiMichael/nunit by DaiMichael 6 years ago
- fixes #2786 Added tests. Updated spelling mistakes. — committed to DaiMichael/nunit by DaiMichael 6 years ago
- fixes #2786 current PR put on new branch. — committed to DaiMichael/nunit by DaiMichael 6 years ago
- Merge pull request #3007 from DaiMichael/issue-2786v2 fixes #2786 Timeout value not resetting on Retry of failed test. — committed to nunit/nunit by rprouse 6 years ago
This sounds to me like an edge case we haven’t thought of. I agree I’d expect it to work as you say.
Will just wait for someone else on @nunit/framework-team to confirm!
I think we should mark as invalid (4) and do this centrally. This blocks the area off, giving us freedom to do whatever we want without fear of backwards compatibility down the road if someone ever asks for the ability to combine repeating attributes.
@jnm2 This is definitely a nit, but here goes…
Current naming pattern for this family of interfraces is IVerbObject, where Verb and Object describe what the interface does in the construction or execution of a test.
IApplyToTest applies changes to the test itself IApplyToContext applies changes to the test execution context IWrapTestMethod wraps the execution of a test method IWrapSetUpTearDown wraps the setup and teardown IRepeatTest repeats the test execution
The “-able” convention for interfaces is a good one - it’s just not the one we have been using for attributes.
IWrapTestRepeat would appear to mean that the interface wraps the repetition of a test rather than actually repeating the test.
I think you can consider this confirmed. Rationale: Timeout works at the WorkItem level. Retry works at the command level. Since the work item executes the command chain, the single timeout value applies to the aggregate of the repetitions.
Changing this would probably mean re-implementing timeout at the command level, since attributes don’t generally get to play at the WorkItem level.