Create alternate GuardClauseAssertion that supports methods marked with async.

[mniak]

As discussed in #268, the GuardClauseAssertion class does not wait the tasks to complete, making it unable to validate non fail fast methods.

This is completely understandable, since the fail fast is indeed the best option. Nevertheless, it seems impossible to implement fail fast behavior when dealing with methods marked with the async keyword.

[ploeh]

it seems impossible to implement fail fast behavior when dealing with methods marked with the async keyword.

How so? Can you provide an example?

[zvirja]

@ploeh Basically, any async method is not fail-fast. Consider the following example:

public async Task<T> GuardedOpenGenericMethodReturningTaskResult<T>(object arg)
{
    if (arg == null) throw new ArgumentNullException(nameof(arg));

    await Task.Yield();

    return default;
}

This method doesn't throw if you call it will null. Instead, the failed Task is returned. Therefore, the GuardClauseAssertion it its current form fails and complains about missing guard clause.

In real application almost all the code is async, therefore each invocation is awaited and exception is thrown immediately. So it's completely OK that "fast-fail" principle is violated in a form that you should await Task to see exception.

Of course, you can re-write code somehow to make it fail immediately (create private function with async keyword and move all the logic there), but there is no practical sense to do it.

[ploeh]

In real application almost all the code is async, therefore each invocation is awaited and exception is thrown immediately.

That's a fair point; I accept that argument.

I sometimes get sidetracked when someone asserts or implies (with little evidence) that something's impossible...

[zvirja]

I sometimes get sidetracked when someone asserts or implies (with little evidence) that something's impossible...

Yeah, it's fair enough 😉

Now let me try to think how I can re-design GuardClauseAssertion to not have boolean flag - I don't like it as well, to be fair. See you later in PR.

[ploeh]

Now let me try to think how I can re-design GuardClauseAssertion to not have boolean flag

I would think that, as @moodmosaic suggested, a new class might be a better design. I haven't looked hard into the issue, though, so I reserve the right to be wrong 😄

[mniak]

Here is the solution I have been using.

1. Remove the async keyword of the method signature
2. Create a nested method with the async keyword
3. Put the guard clauses in the outer part of the method
4. Call the nested method and return the results

Example:

public Task<int> GetNumberAsync(MyObject obj)
{
    // Fail-fast behavior
    if (request is null)
        throw new ArgumentNullException(nameof(obj));

    async Task<int> work()
    {
        var items = await obj.GetItemsAsync(); // whatever you need to call
        return items.Count();
    }

    return work();
}

It has been a good solution for me. Thanks for your help.

[Kralizek]

@mniak Interesting to have a concrete example of what @zvirja suggested here.

Personally I don't really like it because we are polluting the method with "tricks" for the benefit of the testing framework, which smells really bad to me and I wouldn't want my team to use this pattern.

[zvirja]

Keep it open to implement support in a native way. One day. In the nearby future. Or just one day.

[dklinger]

Hi. We stumpled over exactly this issue and were suprised that not good solution exists so far.
Are there reasons to not implement a new class for this?
I don't know the inner details of AutoFixture but if we would have a MethodInfo available within the class GuardClauseAssertion for the Method-to-Test we can have a look a its CustomAttributes and identify if it's an Async method or not(https://stackoverflow.com/a/20350646). So, maybe, a switch (bool vs. new class) isn't even necessary.

[Kralizek]

@zvirja @aivascu could we reconsider this? I really don't see why in 2021 async methods can't be tested properly.

[aivascu]

@Kralizek if I read the situation correctly, the request has not been rejected, and there is even a PR that fixes it.
I had a brief look at the PR and I have to say I agree with the feedback it received.
So the only thing left is to either implement the feedback or file a new PR with the feedback implemented.

[Kralizek]

I couldn't see the linked PR in the mobile app 😅

[andersborum]

I came across the issue (of GuardClauseAssertion not supporting methods marked with async) during implementation of a test suite this week and found this thread. It appear there's a PR available that provides the expected behaviour so what's the status?

What's required to move this request (and PR) forward?