-3

Consider this unit test of an API client with an async function Index. Index should throw an exception when an http error occurs and thats what I want to test.

public async Task TestServiceError()
{
    var client = GetMockClient("{RETURN}", System.Net.HttpStatusCode.BadRequest);

    await Should.ThrowAsync<PreorderException>(async () => await client.Index());
}

Index() is implemented like:

        public async Task<string> Index()
        {
            var response = await Client.GetAsync($"{Ingress}/");

            if (!response.IsSuccessStatusCode)
                throw new PreorderException(response.StatusCode, response.ReasonPhrase);

            var received = await response.Content.ReadAsStreamAsync();
            var readStream = new StreamReader(received, Encoding.UTF8);
            return readStream.ReadToEnd();
        }

So it throws a PreorderException on an error http statuscode after GetAsync os awaited.

I'm left wondering what the difference would be by not using async.

public void TestServiceError()
{
    var client = GetMockClient("{RETURN}", System.Net.HttpStatusCode.BadRequest);

    Should.Throw<PreorderException>(async () => await client.Index());
}

or

public Task TestServiceError()
{
    var client = GetMockClient("{RETURN}", System.Net.HttpStatusCode.BadRequest);

    await Should.ThrowAsync<PreorderException>(() => client.Index());
}

Maybe this is the best even? As there is a Task returned from Index and the await in front of Should await that Task?

The non-async version looks less bulky, and I don't see a reason why it would not work. Throw is synchronous and the await is done in the delegate argument. Plus it's just a unit test with short live code called once.

(The real code has 20 of these so then it makes more sense to remove all the awaits)

await Should.ThrowAsync<PreorderException>(async () => await client.Index());
await Should.ThrowAsync<PreorderException>(async () => await client.FindByODLTest("order", "dealer", "lab"));
await Should.ThrowAsync<PreorderException>(async () => await client.RegisterAsync(new()));
// etc.
3
  • 1
    I don't see a timeout mechanism; or a graceful shutdown; so I'm not sure what is being tested. Commented Aug 26 at 19:59
  • What library is this code using? XUnit doesn't have any Should class. I suspect Shoudly in which case Should.Throw is async-ready and blocks on the task. That's a very unusual API but probably exists to stop people from accidentally calling async void. That's a very strong hint why you shouldn't use Shoult.Throw(Func<Task>) - it's unclear what's going on. The method is there to stop you from shooting your own foot Commented Sep 2 at 7:30
  • I am testing if every function like client.Index() throws an exception on http error Commented Sep 3 at 8:37

3 Answers 3

4

From the non-async version:

Should.Throw<PreorderException>(async () => await client.Index());

TL;DR: At best this will fail to see that client.Index() failed to throw, by not waiting for it to complete. You have created a dependency on the thread pool's scheduling of task at execution.

When the lambda is executed by your unit testing library it will be see to return a Task<T> (or Task), and then Should.Throw<S> will consider the lambda has completed, evaluate there is no exception and not fail (unless the lambda's async operation completes immediately).

You need the unit testing code to wait for your code to complete with failure or a result, not just that some async operation has been scheduled.

Using the async version of the test methods will do this.

Sign up to request clarification or add additional context in comments.

6 Comments

But await Should.ThrowAsync<PreorderException>(() => client.Index()); should be fine right.
Should.Throw succeeds only if there IS an exception so it always sees here that an exception is thrown
@ServeLaurijssen it is not clear whether you have tested these approaches, you start with "Consider": also it would be good to know what testing framework you are using here.
Tried all 3, they all work thats not the issue. Im using XUnit
XUnit doesn't have Should. So what are you using? And what does the code really do? Are you accidentally using a feature meant to prevent people calling async void methods by mistake?
oh like that. Im using Shouldly nuget. Im testing that all functions throw an exception on an HTTP exception
3

First of all, async () => await client.Index() and () => client.Index() does the same thing. This might be more apparent if we rewrite them to regular methods:

public async Task WithAwait(){
    return await client.Index();
}
public Task WithoutAwait(){
    return client.Index();
}

Both return a Task representing the same operation. Since WithAwait does not do anything after awaiting they are directly equivalent.

Should.Throw vs Should.ThrowAsync. It is important to note that client.Index() can 'fail' in two ways:

  1. It could throw an exception the regular way
  2. It could return a Task that wraps an exception.

I don't know library you are using, but I would assume that Should.Throw only handles the former, while Should.ThrowAsync handles both. If you are using mocks it is perfectly possible that either happen work right now, but it is poor practice to rely on this.

So in summary, if you are testing asynchronous code, make the test asynchronous.

Comments

1

First of all method

public Task TestServiceError()
{
    var client = GetMockClient("{RETURN}", System.Net.HttpStatusCode.BadRequest);

    Should.Throw<PreorderException>(async () => await client.Index());
}

won't compile. It has return type Task and it's not async, so you must somehow return Task yourself. Not a great idea.

Secondly, await is there to wait for asynchronous operations, so if you have async method, you must await it, unless you want it to continue in parallel. In your case you want to assert effects of the method (either exception or result returned), which requires method to complete, so you MUST AWAIT it.

Finally, you are looking at it from wrong perspective, ThrowsAsync was designed to make nice assertions on async method. So this does not really depend on how it looks, but what problems it solves. And it solves problem "how to assert exception on async method". Most probably inside the assertion the passed method is run with await, making the assertion method async, that's why you must await it.

So this is rather natural way of following things, rather than approach where someone believes async is just more elegant or something.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.