Autofixture: GuardClauseAssertion does not work with async methods

Created on 14 Mar 2014  路  10Comments  路  Source: AutoFixture/AutoFixture

GuardClauseAssertion does not detect guards on async method parameters because the exception doesn't thrown until the returned task is waited or awaited.

enhancement good first issue

All 10 comments

Consider this class:

``` C#
public class AsyncHost
{
public Task Foo(object obj)
{
if (obj == null)
throw new ArgumentNullException("obj");

    return Task.Factory.StartNew(() => obj.ToString());
}

public async Task<string> Bar(object obj)
{
    if (obj == null)
        throw new ArgumentNullException("obj");

    return await Task.Factory.StartNew(() => obj.ToString());
}

}

Both of these tests pass:

``` C#
[Fact]
public void VerifyFoo()
{
    var assertion = new GuardClauseAssertion(new Fixture());
    assertion.Verify(from m in new Methods<AsyncHost>() select m.Foo(null));
}

[Fact]
public void UseFooWithNull()
{
    var sut = new AsyncHost();
    Assert.Throws<ArgumentNullException>(() => sut.Foo(null));
}

However, both of these tests fail:

``` C#
[Fact]
public void VerifyBar()
{
var assertion = new GuardClauseAssertion(new Fixture());
assertion.Verify(from m in new Methods() select m.Bar(null));
}

[Fact]
public void UseBarWithNull()
{
var sut = new AsyncHost();
Assert.Throws(() => sut.Bar(null));
}

Invoking `Bar(null)` doesn't throw an exception until `Result` is accessed, as this test demonstrates:

``` C#
[Fact]
public void UseBarWithNullThrowsWhenResultIsAccessed()
{
    var sut = new AsyncHost();
    var ae = Assert.Throws<AggregateException>(() => sut.Bar(null).Result);
    var e = ae.InnerExceptions.SingleOrDefault();
    Assert.NotNull(e);
    Assert.IsAssignableFrom<ArgumentNullException>(e);
}

As you can see, GuardClauseAssertion behaves consistently with how 'normal' code behaves. As far as I can tell, this is an artefact of how the C# compiler works, and is equivalent to using iterator blocks. Previously, we had a similar discussion about those: #127

My opinion about this is that the C# compiler violates the Fail Fast principle.

@lawrencejohnston Does the above appropriately answer your original concern?

@ploeh Hi Mark, thanks for following up on this. You're right that fail fast dictates that the method should fail immediately when an argument is invalid. Based on the conversation about the POLA in the iterator blocks discussion should the assertion also check for methods returning a task and throw a more explicit exception explaining this?

Agreed, it should return a better exception message in this case.

@ploeh Do you have some suggestion on how could we write code that does not violate the fail fast principle?
Should we just never use the async keyword or is there a nicer way to do this?

@mniak, perhaps you can split up the code into a part that does the fail-fast check, and the part that does the actual async work... It's been a while since I last looked into issues like this.

Yeah, seems like a good and simple alternative.
Thank you very much!

Yeah, seems like a good and simple alternative.
Thank you very much!

@mniak, Just out of interest what was your solution ?
I'm interested to know how you managed to split up your function without ending up with two async methods, one doing the guard assertions and one doing the actual work, as they will both still need to be async won't they ?

I'm interested to know how you managed to split up your function without ending up with two async methods, one doing the guard assertions and one doing the actual work, as they will both still need to be async won't they ?

@dexc99 You can return the bare Task.

public Task DoSomething(string parameter)
{
    _ = parameter ?? throw new ArgumentNullException(nameof(parameter));
    async Task Do()
    {
        await Task.Delay(1000);
    }
    return Do();
}

Only just got around to looking at this again. @mniak your suggestion worked perfectly. Thanks.

Was this page helpful?
0 / 5 - 0 ratings