Skip to content

Do not try to initialize async fixtures without explicit asyncio mark in strict mode #307

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

Merged
merged 5 commits into from
Mar 15, 2022

Conversation

seifertm
Copy link
Contributor

This fixes a bug that breaks compatibility with pytest_trio (see #298).

The PR:

  • Adds pytest-trio to the test dependencies.

  • Removes test with obsolete "forbid_global_loop".

    forbid_global_loop was an option to pytest.mark.asyncio which was removed in v0.6.0. The two subprocess tests are otherwise identical. Therefore, one of the tests was removed along with the obsolete option.

  • Fixes a bug that causes a package-scoped event_loop fixture to leak into other tests.

    The expected behaviour is that the event_loop fixture defined in tests/sessionloop/conftest.py is torn down when all tests in tests/sessionloop are complete. Running the tests with the pytest option --setup-show pointed out that the fixture is torn down at the end of the test session, instead. This is an unintended side effect of the sessionloop test which may affect other tests in the test suite.

    Reducing the fixture scope from "package" to "module" results in the expected behaviour. The module was renamed to reflect the fact that the tests do not use a session scope.

  • Ignores subprocess tests when running on CPython 3.7.

    When run with Python 3.7, asyncio.subprocess.create_subprocess_exec seems to be affected by an issue that prevents correct cleanup. Tests using pytest-trio will report that signal handling is already performed by another library and fail. [1] This is possibly a bug in CPython 3.7, so I suggest we ignore this test for that Python version.

    I've been shaving this yak for some time and this is my theory:
    CPython 3.7 uses asyncio.streams.StreamReader and asyncio.streams.StreamWriter to implement asyncio.streams.StreamReaderProtocol and asyncio.subprocess.SubprocessStreamProtocol. StreamReaderProtocol contained cyclic references between the reader and the protocol, which prevented garbage collection. While StreamReaderProtocol received a patch [2], SubprocessStreamProtocol, which is used by create_subprocess_exec, possibly has the same problem, but was not patched as part of CPython 3.7.

    [1] pytest-trio exits with TrioInternalError on Python 3.7 when pytest-asyncio is installed python-trio/pytest-trio#126
    [2] bpo-34638: Store a weak reference to stream reader to break strong references loop python/cpython#9201

…sts.

The expected behaviour is that the `event_loop` fixture defined in `tests/sessionloop/conftest.py` is torn down when all tests in `tests/sessionloop` are complete. Running the tests with the pytest option --setup-show pointed out that the fixture is torn down at the end of the test session, instead. This is an unintended side effect of the sessionloop test which may affect other tests in the test suite.

Reducing the fixture scope from "package" to "module" results in the expected behaviour. The module was renamed to reflect the fact that the tests do not use a session scope.

Signed-off-by: Michael Seifert <[email protected]>
forbid_global_loop was an option to pytest.mark.asyncio which was removed in v0.6.0. The two subprocess tests are otherwise identical. Therefore, one of the tests was removed along with the obsolete option.

Signed-off-by: Michael Seifert <[email protected]>
When run with Python 3.7 asyncio.subprocess.create_subprocess_exec seems to be
affected by an issue that prevents correct cleanup. Tests using pytest-trio
will report that signal handling is already performed by another library and
fail. [1] This is possibly a bug in CPython 3.7, so we ignore this test for
that Python version.

CPython 3.7 uses asyncio.streams.StreamReader and asyncio.streams.StreamWriter
to implement asyncio.streams.StreamReaderProtocol and
asyncio.subprocess.SubprocessStreamProtocol. StreamReaderProtocol contained
cyclic references between the reader and the protocol, which prevented
garbage collection. While StreamReaderProtocol received a patch [2],
SubprocessStreamProtocol, which is used by create_subprocess_exec, possibly
has the same problem, but was not patched as part of CPython 3.7.

That's why we ignore this test for CPython 3.7.

[1] python-trio/pytest-trio#126
[2] python/cpython#9201

Signed-off-by: Michael Seifert <[email protected]>
This allows testing compatibility between pytest-trio and pytest-asyncio.

Signed-off-by: Michael Seifert <[email protected]>
… mark in strict mode.

This fixes a bug that breaks compatibility with pytest_trio.

Closes pytest-dev#298

Signed-off-by: Michael Seifert <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #307 (b194dae) into master (82d212b) will increase coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
+ Coverage   92.67%   93.09%   +0.41%     
==========================================
  Files           3        3              
  Lines         273      275       +2     
  Branches       40       41       +1     
==========================================
+ Hits          253      256       +3     
  Misses         12       12              
+ Partials        8        7       -1     
Impacted Files Coverage Δ
plugin.py 92.96% <0.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82d212b...b194dae. Read the comment docs.

@seifertm seifertm merged commit f979af9 into pytest-dev:master Mar 15, 2022
@seifertm seifertm deleted the trio-fixture-compatibilty branch October 23, 2023 06:15
@seifertm seifertm restored the trio-fixture-compatibilty branch October 23, 2023 08:17
@seifertm seifertm deleted the trio-fixture-compatibilty branch October 23, 2023 08:30
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.

2 participants