nunit
Allow specifying max test duration as a string-formatted TimeSpan
So our company has a DatabaseTransactionBaseTestFixture which, as it suggests, operates all interactions with the database with an open enlisted TransactionScope that is aborted.
I would like to propose a few features based on this:
1)a FailAfterAttribute, a way to Assert.Fail after a test takes too long. You have CancelAfter, which produces a CancellationToken but it still is up to the implementor to test both their assertion AND that their assertion completes in a timely manner.
1b) a way to interact with FailAfter programmatically, using TestContext or the like. I would like to be able to set a protected Virtual Timeout on the TransactionScope and have it be able to programmatically used to set FailAfter.
Coincidentally this acts in the same way as your Timeout Attribute which you seem to have deprecated according to the Analyzers. But instead of aborting the thread, which is no longer supported(?) It would simply initiate an AssertionFailure and then Cancel the CancellationTokenSource.
I was able to mimic this using a Task with my own custom CancellationTokenSource, and Assert.Fail in the Task, which adds it to the failure reason but I unfortunately can't test that my mimicry works because I need...
- A way to have the final say in the outcome of the test. After some searching I found this issue: https://github.com/nunit/nunit/issues/49
That looks wonderful! But it seems it may be difficult to add to the code, hence why it was closed as an abandoned issue? What about if on TestContext.Current.Result you had a method "SetFinalOutcome" that overrides the final result?
I'm fine with either option. Would either of these be possible?
Hi @EdLichtman Thanks for your proposal, and sorry it's taken us a bit to get back to you.
I'm hoping we can take a step back to confirm the desired behavior before we jump to a proposal. Are you looking to have the test marked as "Failed" and the associated transaction rolled back after a specified time elapses?
If that's the case and the transaction side is already working for you then I am wondering if perhaps the existing MaxTime attribute could handle marking the test as failed, provided they both have the same timeout value.
If that's not the case, please let us know the behavior you're trying for and we can try to help
CancelAfter should also likely be an option here if you were to register a callback on the context's cancellation token from which you could do things like abort the transaction. This should also allow the test to automatically be marked as failed I think, along with allowing for your tests to cooperatively cancel background tasks (if relevant). This would allow a single source of truth for the timeout value, but would shift it from the transaction to nunit's CancelAfter attribute itself. You may still want to handle rolling back the transaction yourself if the test succeeded, of course.
https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken.register?view=net-9.0
MaxTime claims to not actually complete the test, but rather wait until the test completes and then fails the test. Therefore, could the test end up running for 1 minute or longer if MaxTime is set only for 30 seconds?
https://docs.nunit.org/articles/nunit/writing-tests/attributes/maxtime.html#:~:text=The%20MaxTimeAttribute%20is%20used%20on,is%20reported%20as%20a%20failure.
I think the big thing is I wanted to be able to use TimeSpan rather than "magic number that represents Milliseconds" to control the flow.
I suppose a bit of an assumption was baked into my suggesting of MaxTime.
You're quite right that it would only mark the test as failed; the test itself would continue to run. I'd assumed that the transaction would wrap the whole test and that a timeout within the transaction would throw and an exception stop the test itself... but that was an assumption on my part. If that's incorrect then TimeOut and its newer cooperative-cancellation equivalent CancelAfter exist to also have NUnit handle stopping the test itself too. Note that TimeOut will only work on .NET Framework due to Thread.Abort not being supported on newer runtimes.
I think the big thing is I wanted to be able to use TimeSpan rather than "magic number that represents Milliseconds" to control the flow.
This is a very neat idea! We're not able to use TimeSpan directly in attribute properties as they're not supported by .NET itself, but we could potentially allow a string to be passed in to represent the timespan. This could then be supplemented on newer runtimes by NUnit also specifying StringSyntaxAttribute.TimeSpanFormat to help ensure the passed string is correctly formatted. This could then be added for each of MaxTime, CancelAfter and (for API consistency) on TimeOut as well.
EDIT: For example:
[MaxTime("00:01:00")]
public static void MyLongRunningTest() {
}
How could I use that as a timespan property too? If I had a virtual property would I then have a const that is created using Timespan.Parse?
What about an additional contract with a class and property like with TestCaseSource?
Like: [CancelAfter(nameof(MyTestFixture), nameof(MyProperty)]
?
Is there a reason you need to have the attribute load the timeout interval externally rather then to have the transaction derive it's timeout value from the attribute?
The Timeout and CancelAfter would both store their timeout value as a property of the test which should be readable from the test context: https://github.com/nunit/nunit/blob/main/src/NUnitFramework/framework/Attributes/CancelAfterAttribute.cs#L26. It's not stored as a timespan but you could use TimeSpan.FromMilliseconds to convert it for your needs. Changing the format in which NUnit stores that would, unfortunately, likely be a breaking change.
To expand a bit more: I'm hesitant about your proposed change.
Dynamically loading the timeout here, especially from a non-static method, adds considerable complexity. For example, instance methods for "Source" attributes have the potential that class-level variables could share state between multiple "source"s without a guaranteed order in which they were run, leading to surprising results. This is why we've kept the other "source"-type attributes like TestCaseSource, ValueSource, and TestFixtureSource to all primarily require static methods.
EDIT: You could, of course, create one yourself in your project by extending CancelAfterAttribute to support this :)
I see. Well I'm glad I brought it up. I remember a weird instance in which I had a static constructor below a static field and spent hours figuring out why the field which depended on another field set in the constructor, caused an error. Thanks for enlightening me as to your reason.
I'll check back in on if there was a reason for the property to be dependent but I think so long as I can check for the FailAfterAttribute timespan then I'm good with that.
That said, each of CancelAfter and MaxTime still don't fail the test right away, right? If so we probably want that FailAfterAttribute? I'll double check this tomorrow.
Your FailAfterAttribute is the same as the TimeoutAttribute.
It is deprecated because it never worked on .NET Core because there is no Thread.Abort implementation.
When ignore the deprecated warning and use it, the test will continue running in another thread even though it reports as completed with a timeout error.
Even on .NET Framework aborting a Thread is dangerous. Finalizer will not be executed. If you test has any using statements, the Dispose actions will not run.
using (CreateTransaction())
{
// Several statements
}
If the Thread is aborted, the Disposal of the transaction is not executed and no roll-back performed.
Any other code in finalizers to clean up any left-over files etc. will also not be executed.
You have CancelAfter, which produces a CancellationToken but it still is up to the implementor to test both their assertion AND that their assertion completes in a timely manner.
I see. The test below takes longer than the Cancellation time, but is green. Not sure if it is a valid test where the token is not really used.
[Test]
[CancelAfter(1000)]
public void RunningTestTakesTooLong(CancellationToken token)
{
Assert.That(token, Is.Not.Default);
Thread.Sleep(2000);
Assert.That(token.IsCancellationRequested, Is.True);
}
If the CancellationToken is passed to deeper routines, the test is marked as failed because something obeys the token.
[Test]
[CancelAfter(1000)]
public async Task RunningTestTakesTooLongWithForwaredCancellationToken(CancellationToken token)
{
Assert.That(token, Is.Not.Default);
await Task.Delay(2000, token);
Assert.That(token.IsCancellationRequested, Is.True);
}
Does your transaction scope not have a CancellationToken parameter?
Sorry it took me a while to get back to this.
I'm using the TestContext.Current cancellation token for my transaction scope. However when the transaction scope takes too long, The test fails in a way that's very hard identify due to the call stack.
What I ended up doing was creating a cancellation token source that adds an extra half a second and then having a background timer that runs a cert.fail when the cancellation token has failed.
It seems to work in the order of events where the asserta fail in a background task kills the test before the transaction is able to be the reporting failure, but the cancellation token still ends up failing.
The premise of this was to provide an easier way to do this since I'm having to work around "Vendor Code" to make my goals happen, I was thinking perhaps something like that may be inherently useful.