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

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
24 changes: 24 additions & 0 deletions docs/develop/python/testing-suite.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ 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 time-skipping 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
edmondop marked this conversation as resolved.
Show resolved Hide resolved


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


# Notice how this doesn't specify a "scope" like "session" scope. 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()
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
```

## 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