Remove ConfigureAwait(false) in tests?

[bjornhellander]

@MartyIX asked why we have ConfigureAwait(false) in the test projects: #3719 (comment)

In later xUnit versions, there is an analyzer which warns about this due to potential parallelization issues (https://xunit.net/xunit.analyzers/rules/xUnit1030), so it seems to be a valid question.

[bjornhellander]

I found this which contained some background to xUnit1130: xunit/xunit#2628

I kind of "hoped" that running too many tests in parallel could have a negative impact on execution time etc, but after benchmarking a bit on my laptop with and without ConfigureAwait I don't see any clear differences. Will try it on github for a while too, in case it behaves differently there.

[MartyIX]

@MartyIX asked why we have ConfigureAwait(false) in the test projects: #3719 (comment)

it's not about all ConfigureAwait(false) calls but rather those that are directly in [Fact]/[Theory] methods (i.e. top-level ones).

In general, one might want to run a group of tests in a serial fashion, using ConfigureAwait(false) improperly can imho break that feature. In my mind, the we are talking here more about correctness than about some performance speedups.

Anyway, since nobody noticed this issue to this day, it might be a non-issue in practice. So maybe it can be qualified as "nice to have".

[bjornhellander]

Ok, then I misunderstood the reasoning behind it. Thanks for clarifying! I am pretty sure there is no correctness issue related to it in this repository at this moment, but it seems like a good idea to follow their analyzers. A decision will need to be taken when xunit is updated, at the latest.

[sharwell]

We will likely have to disable xUnit1030, because there are many places in Roslyn where ConfigureAwait(false) is used and we don't have control over those.