Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TimeoutAttribute makes all Assertions count as failure #3282

Closed
fbrosseau opened this issue Jun 15, 2019 · 3 comments · Fixed by #3366
Closed

TimeoutAttribute makes all Assertions count as failure #3282

fbrosseau opened this issue Jun 15, 2019 · 3 comments · Fixed by #3366

Comments

@fbrosseau
Copy link

fbrosseau commented Jun 15, 2019

Hello

TimeoutAttribute(via TimeoutCommand) wraps the tests in a Task.Wait call under netcore (because Thread.Abort is no longer supported API), here:

if (!Task.Run(() => context.CurrentResult = innerCommand.Execute(context)).Wait(_timeout))

Using any of the 'flow-control' assertions like Assert.Ignore will throw their respective special-case Exception (here IgnoreException), which Task.Wait will wrap in an AggregateException, which is not handled by ValidateAndUnwrap here

if ((ex is NUnitException || ex is TargetInvocationException) && ex.InnerException != null)

This will result in the test counting as full-blown failure due to unhandled exception.

This simple test will do it:

[Test, Timeout(1234)]
public void TimeoutBug()
{
    Assert.Ignore("a");
}

I believe it should be safe to change ValidateAndUnwrap to handle AggregateExceptions the same way it does for TargetInvocationExceptions. It may also be beneficial to dive into the exception recursively instead of only once.

mikeharder added a commit to mikeharder/azure-sdk-for-net that referenced this issue Dec 13, 2019
- Workaround for NUnit bug "TimeoutAttribute makes Assert.Inconclusive() count as failure" (nunit/nunit#3282)
mikeharder added a commit to Azure/azure-sdk-for-net that referenced this issue Dec 13, 2019
- Add LiveOnly attribute to Assert.Inconclusive() tests
  - Workaround for NUnit bug "TimeoutAttribute makes Assert.Inconclusive() count as failure" (nunit/nunit#3282)
@airbreather
Copy link

airbreather commented Dec 16, 2019

Another option:

  1. originalTask = the Task.Run we do now
  2. delayTask = a Task.Delay(_timeout)
  3. Task.WhenAny(originalTask, delayTask).GetAwaiter().GetResult() gives you the first Task to complete.
    • if it's delayTask, then we hit the timeout block
    • otherwise, call .GetAwaiter().GetResult() on it, which should do the right thing

@provegard
Copy link

Came here to report this exact issue. We want to use an assembly-wide Timeout attribute, but tests that use Assert.Ignore start to fail. Any workaround except not using Assert.Ignore?

@roji
Copy link

roji commented Apr 30, 2020

Ran into this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants