Skip to content

EDU-3510: Python: Added paragraph that warns about pytest.fixture(scope="session") with test server #3113

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

EDU-3510: Python: Added paragraph that warns about pytest.fixture(scope="session") with test server #3113

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f8a1cd8
Added mention about fixtures
edmondop Sep 28, 2024
2143a1f
Merge branch 'main' into issue-3111
fairlydurable Oct 10, 2024
d6b6475
Merge branch 'main' into issue-3111
fairlydurable Nov 18, 2024
92b692e
Update docs/develop/python/testing-suite.mdx
edmondop Dec 2, 2024
089330f
Update docs/develop/python/testing-suite.mdx
edmondop Dec 2, 2024
15f497f
Merge branch 'main' into issue-3111
edmondop Dec 2, 2024
0db5697
Update docs/develop/python/testing-suite.mdx
edmondop Dec 3, 2024
45851f2
Implementing feedback from PR about using explicit asynchronous fixtures
edmondop Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/develop/python/testing-suite.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,34 @@ Some SDKs have support or examples for popular test frameworks, runners, or libr

One recommended framework for testing in Python for the Temporal SDK is [pytest](https://docs.pytest.org/), which can help with fixtures to stand up and tear down test environments, provide useful test discovery, and make it easy to write parameterized tests.

### A note on pytest fixtures

When using pytest fixtures to setup databases, application servers and common infrastructure it is not unusual to use the `session` scope to ensure the fixture is shared among multiple tests. However, with the test server it is important that each test is executed in isolation. Launching a test server per test introduce negligible overhead and ensures your test do not conflict with each other


```python
import pytest
import temporalio.testing.WorkflowEnvironment


# This might break because it will reuse the workflow environment
pytest.fixture(scope="session")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pytest.fixture(scope="session")
@pytest.fixture(scope="session")

If you do keep this, need @

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!

def workflow_environment():
return WorkflowEnvironment.start_time_skipping()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this works without @pytest_asyncio.fixture and async and await. Have you tested this code? And for many, they may prefer a "session" scoped start_local instead of time skipping even though this only demonstrates time skipping.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, was using the auto-mode in my tests https://pytest-asyncio.readthedocs.io/en/latest/reference/configuration.html but I agree that being explicit works better

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be addressed on @cretz

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears the workflow_environment() function is now duplicated in this snippet

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to show the good and bad example, do you think one of the two fixtures should be named differently ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it's a bit confusing to even have a bad example unless it's labeled more clearly and in a separate snippet. Many of our users just grab the first snippet and move on.

So in this case, we do want start_local to be used by users and used across tests, but if they need start_time_skipping you are correct that it should be per test. So maybe we should have both forms, named differently, with comments above each on why start-local is session scoped and start-time-skipping is not.


def test_1(workflow_environment):
pass

def test_2(workflow_environment):
pass

# Correct: if you don't specify a scope, the default scope is function,
# so a new workflow environment will be created for each test
@pytest.fixture
def workflow_environment():
return WorkflowEnvironment.start_time_skipping()
```

## Testing Activities {#test-activities}

An Activity can be tested with a mock Activity environment, which provides a way to mock the Activity context, listen to Heartbeats, and cancel the Activity.
Expand Down