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

pytest thread safety should be better documented #12356

Closed
4 tasks done
ngoldbaum opened this issue May 22, 2024 · 5 comments · Fixed by #12359
Closed
4 tasks done

pytest thread safety should be better documented #12356

ngoldbaum opened this issue May 22, 2024 · 5 comments · Fixed by #12359

Comments

@ngoldbaum
Copy link
Contributor

ngoldbaum commented May 22, 2024

  • a detailed description of the bug or problem you are having
  • output of pip list from the virtual environment you are using
  • pytest and operating system versions
  • minimal example if possible

pytest.warns is a subclass of the standard library warnings.catch_warnings context manager, which is not thread safe (this is noted in the docs).

If it's not possible for pytest to have a thread-safe version, it would be nice if the docs for pytest.warns could note that it's implemented using warnings.catch_warnings and is not thread safe for the same reason.

pytest 8.2.1 on MacOS Sonoma.

Fails about 10-20% of the time on my machine:

import warnings
import threading


def raise_warning():
    warnings.warn(RuntimeWarning())


def test_pytest_warns():
    b = threading.Barrier(2)

    def catch_warning():
        b.wait()
        with pytest.warns(RuntimeWarning):
            raise_warning()

    task1 = threading.Thread(target=catch_warning)
    task2 = threading.Thread(target=catch_warning)

    task1.start()
    task2.start()
    task1.join()
    task2.join()
@The-Compiler
Copy link
Member

pytest in its entirety is not intended to be thread-safe IIRC.

@RonnyPfannschmidt
Copy link
Member

pytest.warns is implemented in terms of the stdlib catch-warnings helper - which is inherently not threadsave

@ngoldbaum
Copy link
Contributor Author

pytest in its entirety is not intended to be thread-safe IIRC.

TIL

Best to avoid using pytest constructs inside a thread, then.

This fact could be emphasized more in the docs, I don't see any mention of threads at all besides the discussion about unhandled exceptions.

@ngoldbaum
Copy link
Contributor Author

ngoldbaum commented May 23, 2024

pytest.warns is implemented in terms of the stdlib catch-warnings helper - which is inherently not threadsave

Yes, I noted that in the issue description. The issue is a request for that fact to be noted in the docs, like the docs for the stdlib helper, if making it thread safe is out of scope for the library.

@nicoddemus
Copy link
Member

Makes sense to note somewhere that pytest's constructs/helpers are not thread safe.

If somebody wants to take a look and update the docs, would love to review a PR. 👍

@ngoldbaum ngoldbaum changed the title pytest.warns is not thread safe pytest thread safety should be better documented May 23, 2024
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 a pull request may close this issue.

4 participants