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

Document tmp_path fixture does not support concurrent runs of the same test by default #11790

Closed
faph opened this issue Jan 8, 2024 · 7 comments · Fixed by #11800
Closed
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification

Comments

@faph
Copy link
Contributor

faph commented Jan 8, 2024

The tmp_path fixture documentation (https://docs.pytest.org/en/7.1.x/how-to/tmp_path.html#the-tmp-path-fixture) states that temporary directories are unique for each "test invocation". This is correct in the sense that each test function is associated by a single unique temporary directory.

However, this is not correct when considering running the same test function concurrently by distinct pytest invocations. We are doing this all the time using Tox where tests are executed from multiple virtualenvs (different Python versions), concurrently.

In that scenario, the tmp_path generated directory is shared/clashes between multiple invocations.

I appreciate that the solution is to uniqueify the temp path prefix, similar to what the pytest-xdist extension does. With Tox, that is trivial to achieve. Reading the Pytest docs, I do not necessarily know that I need that solution.

@faph
Copy link
Contributor Author

faph commented Jan 8, 2024

This is related, but distinct, from #11789

@RonnyPfannschmidt
Copy link
Member

multiple concurrent pytest processes should always have different basetemp directories

@faph
Copy link
Contributor Author

faph commented Jan 8, 2024

@RonnyPfannschmidt Thanks, yes I understand that now. But I would NOT know by reading the how-to docs, right?

edit: inserted "NOT"

@RonnyPfannschmidt
Copy link
Member

indeed, the wording probably should use "unique for the current test" as the implication is a bit of a hidden rabbit hole

@RonnyPfannschmidt RonnyPfannschmidt added type: docs documentation improvement, missing or needing clarification good first issue easy issue that is friendly to new contributor labels Jan 8, 2024
@faph
Copy link
Contributor Author

faph commented Jan 12, 2024

@nicoddemus | @RonnyPfannschmidt Could I suggest the docs to be more explicit about concurrent execution? I agree that the #11800 more accurately reflects reality. However, I feel it is still somewhat ambigious in what "current test" means...

@nicoddemus
Copy link
Member

@faph Sure, feel free to open a PR suggesting a change you feel clarifies things. 👍

@faph
Copy link
Contributor Author

faph commented Jan 17, 2024

@nicoddemus What about something like #11830?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants