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

AsyncFunctionAssertions methods contain redundant multithreading #1020

Closed
davidomid opened this issue Mar 31, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@davidomid
Copy link
Contributor

commented Mar 31, 2019

Fixed in PR #1027

Description

The AsyncFunctionAssertions class contains methods with redundant multithreading.

These methods are:

  • public void NotThrow(string because = "", params object[] becauseArgs)
  • public async Task NotThrowAsync(string because = "", params object[] becauseArgs)
  • public void NotThrow(string because = "", params object[] becauseArgs)
    where TException : Exception
  • public async Task NotThrowAsync(string because = "", params object[] becauseArgs)
    where TException : Exception
  • private Exception InvokeSubjectWithInterception()
  • private async Task InvokeSubjectWithInterceptionAsync()

Complete minimal example reproducing the issue

E.g. this is how NotThrowAsync currently looks:

public async Task NotThrowAsync(string because = "", params object[] becauseArgs)
{
     try
     {
          await Task.Run(Subject);
     }
     catch (Exception exception)
     {
          NotThrow(exception, because, becauseArgs);
     }
 }

This is how I would expect it to look:

public async Task NotThrowAsync(string because = "", params object[] becauseArgs)
{
     try
     {
          await Subject();
     }
     catch (Exception exception)
     {
          NotThrow(exception, because, becauseArgs);
     }
 }

Expected behavior:

The method should await the task returned by the Subject func.

In the other async methods listed above, I expect similar behaviour. In the non-async methods, I expect the task to be awaited in a blocking way (i.e. using .Wait() instead of await).

Actual behavior:

The method awaits a new task returned by Task.Run, which is sent the Subject func.
This schedules the work to be done on the thread pool, potentially using a different thread.
This adds overhead and is otherwise redundant.

This behaviour is similar for other methods listed above.

Versions

Any version

@jnyrup

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

Fixed in #1027

@jnyrup jnyrup closed this May 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.