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

Relax asyncio_mode type definition; it allows to support pytest 6.1+ #264

Merged
merged 14 commits into from Jan 16, 2022

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Jan 14, 2022

Fixes #262

Copy link
Contributor

@seifertm seifertm left a comment

Choose a reason for hiding this comment

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

I don't think this is enough to support pytest 5.4. I set up a test project using code from this PR and pytest 5.4.3 which produces the following error:

$ pytest
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/_pytest/main.py", line 187, in wrap_session
INTERNALERROR>     config._do_configure()
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 820, in _do_configure
INTERNALERROR>     self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/hooks.py", line 308, in call_historic
INTERNALERROR>     res = self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/manager.py", line 93, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/manager.py", line 84, in <lambda>
INTERNALERROR>     self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/tmp/venv/lib/python3.9/site-packages/pytest_asyncio/plugin.py", line 111, in pytest_configure
INTERNALERROR>     config.issue_config_time_warning(LEGACY_MODE, stacklevel=2)
INTERNALERROR> AttributeError: 'Config' object has no attribute 'issue_config_time_warning'

I'm not opposed to raising the minimum required pytest version. Currently we seem to require at least pytest 6.1.

@asvetlov
Copy link
Contributor Author

Uuups. Thanks for cross-checking.
I'll raise the minimal version and extend the GitHub matrix to check both minimal and latest pytest to make sure that the plugin works well.

@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2022

Codecov Report

Merging #264 (2cd678c) into master (087e0b6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #264   +/-   ##
=======================================
  Coverage   93.12%   93.12%           
=======================================
  Files           3        3           
  Lines         262      262           
  Branches       33       33           
=======================================
  Hits          244      244           
  Misses         12       12           
  Partials        6        6           
Impacted Files Coverage Δ
plugin.py 92.99% <0.00%> (ø)

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 087e0b6...2cd678c. Read the comment docs.

Copy link
Contributor Author

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

Done, thanks for the review!

Comment on lines +16 to +19
[testenv:pytest-min]
extras = testing
deps =
pytest == 6.1.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure that the plugin works with minimal requirements.

Comment on lines -46 to +47
def test_async_auto_marked(pytester):
pytester.makepyfile(
def test_async_auto_marked(testdir):
testdir.makepyfile(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required for pytest 6.1 as I see. Not a big problem though, the used API is the same.

@asvetlov asvetlov changed the title Relax asyncio_mode type definition; it allows to support pytest 5.4+ Relax asyncio_mode type definition; it allows to support pytest 6.1+ Jan 15, 2022
@seifertm
Copy link
Contributor

I like that you added a tox env for the minimum pytests version!

I think the added type annotations may have broken this PR, though. On commit c0ca686 I get this error:

(venv) michael@msnote0 /tmp/test $ pytest
Traceback (most recent call last):
  File "/tmp/test/venv/bin/pytest", line 8, in <module>
    sys.exit(console_main())
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 187, in console_main
    code = main()
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 143, in main
    config = _prepareconfig(args, plugins)
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 318, in _prepareconfig
    config = pluginmanager.hook.pytest_cmdline_parse(
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/hooks.py", line 286, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/manager.py", line 93, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/manager.py", line 84, in <lambda>
    self._inner_hookexec = lambda hook, methods, kwargs: hook.multicall(
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/callers.py", line 203, in _multicall
    gen.send(outcome)
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/helpconfig.py", line 100, in pytest_cmdline_parse
    config = outcome.get_result()  # type: Config
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 1003, in pytest_cmdline_parse
    self.parse(args)
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 1280, in parse
    self._preparse(args, addopts=addopts)
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/config/__init__.py", line 1172, in _preparse
    self.pluginmanager.load_setuptools_entrypoints("pytest11")
  File "/tmp/test/venv/lib/python3.9/site-packages/pluggy/manager.py", line 299, in load_setuptools_entrypoints
    plugin = ep.load()
  File "/usr/lib/python3.9/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/usr/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/assertion/rewrite.py", line 171, in exec_module
    exec(co, module.__dict__)
  File "/tmp/test/venv/lib/python3.9/site-packages/pytest_asyncio/__init__.py", line 3, in <module>
    from .plugin import fixture
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "/tmp/test/venv/lib/python3.9/site-packages/_pytest/assertion/rewrite.py", line 171, in exec_module
    exec(co, module.__dict__)
  File "/tmp/test/venv/lib/python3.9/site-packages/pytest_asyncio/plugin.py", line 456, in <module>
    def event_loop(request: pytest.FixtureRequest) -> Iterator[asyncio.AbstractEventLoop]:
AttributeError: module 'pytest' has no attribute 'FixtureRequest'

@asvetlov
Copy link
Contributor Author

asvetlov commented Jan 16, 2022

@seifertm fair point! I've converted the type annotation for FixtureRequest to string, tests on 6.1 are passed now: https://github.com/pytest-dev/pytest-asyncio/runs/4832158093?check_suite_focus=true#step:5:63

@asvetlov
Copy link
Contributor Author

I have no idea why GitHub didn't run checks for yesterday's commits.
Perhaps it was an infrastructure incident.
That's why I missed the FixtureDef problem yesterday.
@seifertm thank you for checking! Again!

@asvetlov
Copy link
Contributor Author

asvetlov commented Jan 16, 2022

I'd like to merge it tomorrow today, make 0.17.1 bugfix to get it ready tomorrow at the beginning of working days.

@asvetlov asvetlov merged commit 36f277d into master Jan 16, 2022
@asvetlov asvetlov deleted the relax-config-type branch January 16, 2022 20:51
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.

Wrong minimum version of pytest
4 participants