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

Improve FakeTimeProvider guidance #5500

Open
slang25 opened this issue Oct 10, 2024 · 3 comments
Open

Improve FakeTimeProvider guidance #5500

slang25 opened this issue Oct 10, 2024 · 3 comments
Assignees

Comments

@slang25
Copy link

slang25 commented Oct 10, 2024

I think we should provide some better guidance for dealing with await continuations that are trigger by FakeTimeProvider.Advance, currently the guidance is this, which says

it's important to use ConfigureAwait(true)

While this may fix an issue the author faced, this is not generally true, does not explain the context of the problem, and in my opinion add to the confusion that already exists in this space.

What is really happening the in the provided example is that xUnit v2 provides its own SynchronizationContext by default (AsyncTestSyncContext), and this is changing how continuations are run when capturing context is suppressed.

Looking at how TimeProvider continations work, using Task.Delay(...) as an example, it's designed to run the continuations synchronously (see here), however when a synchronization context is present, .ConfigureAwait(false) changes this behaviour and the continuation gets queued onto the thread-pool (which I find surprising).

A reason I find this surprising is that when there is no synchronization context, like in NUnit, MSTest, TUnit or xUnit v3, .ConfigureAwait() has no effect. It makes sense to me that .ConfigureAwait(false) is effectively a no-op when there is no sync context, but the fact that it prevents inline continuations when there is a sync context I wasn't expecting.

I would prefer the fix to suggest changing the test code to fix run SynchronizationContext.SetSynchronizationContext(null) before starting to use FakeTimeProvider, and mentioning this is only for xUnit v2. cc @tarekgh @stephentoub for their opinions.

@RussKie
Copy link
Member

RussKie commented Nov 5, 2024

@dotnet/dotnet-extensions-fundamentals please triage

@amadeuszl
Copy link
Contributor

I will take a look onto this issue this week.

@amadeuszl
Copy link
Contributor

Agreed, current documentation was based on a bug related to running tests with Polly. SynchronizationContext.SetSynchronizationContext(null) seems like more universal solution to the general problem with xUnit Context. I will create PR to update documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants