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

Flow execution context across fixture methods when using timeout #2843

Merged
merged 6 commits into from May 16, 2024

Conversation

Evangelink
Copy link
Member

@Evangelink Evangelink commented May 10, 2024

Problem

Before the timeout PR, or when no timeout is being set (runsettings or on fixture/test methods), then all methods are being called synchronously and so the state is "shared" and can be mutated by all steps. For example, when changing the culture in AssemblyInitialize this culture will be set for the ClassInitialize, all the way down to the test method then back to the AssemblyCleanup.

When setting up timeouts, we are starting new thread so we can stop observing the execution when the timeout expires (to overcome non-cooperative cancellation). Because of this thread creation, each thread works in isolation and its state is not preserved and flown to the other methods which is unexpected by users.

Solution

To overcome this issue, we need to manually capture and restore the right ExecutionContext at the right time. The logic I decided to go with is to flow top-to-bottom meaning that the AssemblyInitialize context is flown down to the ClassInitialize... This is allowing to have a behavior similar to the one when no timeout is set.

Design decision

But there is a question happening for ClassCleanup and AssemblyCleanup methods where the state when there is no timeout is undeterministic as it depends on the last executed test (for ClassCleanup) and last executed ClassCleanup for AssemblyCleanup. Although, technically, we could mimic this behavior, I think it's better to have the ClassCleanup reusing the state of left by the corresponding ClassInitialize and have the AssemblyCleanup reusing the state left the by corresponding AssemblyInitialize as it fixes the undeterminism and it is also closer to what happens for awaited async methods.

Examples

When timeout is set:

flowchart TD

A[AssemblyInitialize\nCulture is English\nSet culture to Japanese] --> B(ClassInitialize\nCulture is Japanese\nSet culture to Italian)
B --> C(TestInitialize\nCulture is Italian\nSet culture to French)
C --> D(TestMethod\nCulture is French\nSet culture to Czech)
D --> E(TestCleanup\nCulture is Czech\nSet culture to Spanish)
E --> F(ClassCleanup\nCulture is Italian - result of ClassInitialize state and not Spanish result of TestCleanup\nSet culture to Hungarian)
F --> G(AssemblyCleanup\nCulture is Japanese - result of AssemblyInitialize state and not Hungarian of ClassCleanup)

When timeout is not set:

flowchart TD

A[AssemblyInitialize\nCulture is English\nSet culture to Japanese] --> B(ClassInitialize\nCulture is Japanese\nSet culture to Italian)
B --> C(TestInitialize\nCulture is Italian\nSet culture to French)
C --> D(TestMethod\nCulture is French\nSet culture to Czech)
D --> E(TestCleanup\nCulture is Czech\nSet culture to Spanish)
E --> F(ClassCleanup\nCulture is Spanish result of TestCleanup\nSet culture to Hungarian)
F --> G(AssemblyCleanup\nCulture is Hungarian result of ClassCleanup)

Fixes AB#2051896

@testplatform-bot
Copy link
Contributor

✅ Successfully linked to Azure Boards work item(s):

@Evangelink Evangelink marked this pull request as ready for review May 16, 2024 07:14
@testplatform-bot
Copy link
Contributor

✅ Successfully linked to Azure Boards work item(s):

1 similar comment
@testplatform-bot
Copy link
Contributor

✅ Successfully linked to Azure Boards work item(s):

@Evangelink Evangelink merged commit c4c6049 into microsoft:main May 16, 2024
1 of 6 checks passed
@Evangelink Evangelink deleted the execution-context-timeout branch May 16, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants