It hang again.

Maybe we try using what I've done in Chill: https://github.com/ChillBDD/Chill/pull/78/files

Maybe we try using what I've done in Chill: https://github.com/ChillBDD/Chill/pull/78/files

Thanks very much for this link. Interesting solution! I'll see what I can do :)

@dennisdoomen the build is now passing. Please review this pull request and let me know your thoughts.

Thanks very much again for that ChillBDD suggestion.

If I understand the purpose of NoSynchronizationContextScope, it is meant to be called at the boundaries and then avoiding having to call ConfigureAwait(false) everywhere deeper in the stack.

If that is correct, shouldn't it be enough to only call NoSynchronizationContextScope in:

• public static ExecutionTime ExecutionTime(this Func<Task> action)
• public static AsyncFunctionAssertions Should(this Func<Task> action)
• public static AsyncFunctionAssertions Should<T>(this Func<Task<T>> action)

as they are, as far as I could see, exactly the boundaries of calling Task and Task<T>?

If I understand the purpose of NoSynchronizationContextScope, it is meant to be called at the boundaries and then avoiding having to call ConfigureAwait(false) everywhere deeper in the stack.

If that is correct, shouldn't it be enough to only call NoSynchronizationContextScope in:

• public static ExecutionTime ExecutionTime(this Func<Task> action)
• public static AsyncFunctionAssertions Should(this Func<Task> action)
• public static AsyncFunctionAssertions Should<T>(this Func<Task<T>> action)

as they are, as far as I could see, exactly the boundaries of calling Task and Task<T>?

Thanks for the feedback, you've raised an excellent point. I'll get right on it :)

If I understand the purpose of NoSynchronizationContextScope, it is meant to be called at the boundaries and then avoiding having to call ConfigureAwait(false) everywhere deeper in the stack.

If that is correct, shouldn't it be enough to only call NoSynchronizationContextScope in:

• public static ExecutionTime ExecutionTime(this Func<Task> action)
• public static AsyncFunctionAssertions Should(this Func<Task> action)
• public static AsyncFunctionAssertions Should<T>(this Func<Task<T>> action)

as they are, as far as I could see, exactly the boundaries of calling Task and Task<T>?

@jnyrup I've now pushed these changes. Please let me know if this is what you had in mind, or if you have other suggestions.

Thanks very much

Looks good!
(minor note: There are now a few superfluous namespaces and whitespace additions)

What happens if I execute one the following sync-over-async snippets, that calls into ActionAssertions and FunctionAssertions, respectively?

Action act = () => Task.Delay(0).Wait(0);
act.Should().NotThrow();

Func<bool> func = () => Task.Delay(0).Wait(0);
func.Should().NotThrow();

Looks good!
(minor note: There are now a few superfluous namespaces and whitespace additions)

My bad, I'll resolve this.

What happens if I execute one the following sync-over-async snippets, that calls into ActionAssertions and FunctionAssertions, respectively?

Action act = () => Task.Delay(0).Wait(0);
act.Should().NotThrow();

Func<bool> func = () => Task.Delay(0).Wait(0);
func.Should().NotThrow();

You've raised an excellent point. I could be mistaken but I don't think we have any test cases like this already. I'll create some and see if we have any potential issues.

Thanks very much!

@jnyrup I've made the changes to remove the superfluous namespaces and whitespace. I've also added some new tests with the scenarios you described above. Those tests seem to pass fine but it looks like the current build is failing due to one of those other flaky tests which are being investigated separately. Do we need to retry the AppVeyor build somehow?

Please let me know if you have any other issues or suggestions.

Thanks very much :)

It just looked odd to me. When I looked into it, it looked a lot like it was only there to prevent deadlocks, so I thought it would be good to investigate and try to fix the underlying issue instead.

In terms of retrying the build, I could always push a blank commit to force it to rebuild but I don't want to pollute the commit history and would rather ask if there's an alternative first :)

@davidomid or, if you haven't tried before and would like to train your git-fu, you could trigger a build by rebasing on master into atomic commits, e.g.

• Removed redundant multithreading
• Added additional test cases for ActionAssertions and FunctionAssertions

Not the case for this PR, but for large PR atomic commits greatly simplifies reviewing.

@davidomid or, if you haven't tried before and would like to train your git-fu, you could trigger a build by rebasing on master into atomic commits, e.g.

• Removed redundant multithreading
• Added additional test cases for ActionAssertions and FunctionAssertions

Not the case for this PR, but for large PR atomic commits greatly simplifies reviewing.

I'll do it for this one :)

Thanks for the advice!

@jnyrup @dennisdoomen

I've made the commits atomic and the latest build was successful :)

Please let me know if there's anything else you'd like me to resolve.

Well done @davidomid. Thanks for the hard work.