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.

Sign up for GitHub

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

Jump to bottom

v0.23.2 will cause INTERNALERROR if ImportWarning is raised in an imported module #713

Closed
Stausssi opened this issue Dec 7, 2023 · 14 comments · Fixed by #728
Closed

v0.23.2 will cause INTERNALERROR if ImportWarning is raised in an imported module #713

Stausssi opened this issue Dec 7, 2023 · 14 comments · Fixed by #728
Labels
bug
Milestone
v0.23

Comments

@Stausssi
Copy link

Stausssi commented Dec 7, 2023

Hi all,

I've run into an INTERNALERROR while pytest was collecting the tests / activating the plugins starting from pytest-asyncio v0.23.2.
I have a file src/module/module.py with the contents

# ...

if not Path("foo/bar").exists():
    raise ImportWarning("Please import from ... directory only")

# ...

and src/module/__init__.py with

from .module import foobar

When actually testing the module, obviously Path.exists() is mocked.
This worked fine with pytest-asyncio v0.21.1 but causes an error with pytest-asyncio v0.23.2:

$ pytest --cov --cov-report term-missing
============================= test session starts ==============================
platform linux -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0
rootdir: /home/runner/work/my-repository/my-repository
plugins: cov-4.1.0, asyncio-0.23.2, anyio-3.7.1
asyncio: mode=Mode.STRICT
collected 13 items
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/main.py", line 271, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>                          ^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/main.py", line 324, in _main
INTERNALERROR>     config.hook.pytest_collection(session=session)
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_hooks.py", line 493, in __call__
INTERNALERROR>     return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_manager.py", line 115, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_callers.py", line 152, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_result.py", line 114, in get_result
INTERNALERROR>     raise exc.with_traceback(exc.__traceback__)
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_callers.py", line 77, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>           ^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/main.py", line 335, in pytest_collection
INTERNALERROR>     session.perform_collect()
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/main.py", line 675, in perform_collect
INTERNALERROR>     self.items.extend(self.genitems(node))
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/main.py", line 842, in genitems
INTERNALERROR>     rep = collect_one_node(node)
INTERNALERROR>           ^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/runner.py", line 546, in collect_one_node
INTERNALERROR>     ihook.pytest_collectstart(collector=collector)
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_hooks.py", line 493, in __call__
INTERNALERROR>     return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_manager.py", line 115, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_callers.py", line 113, in _multicall
INTERNALERROR>     raise exception.with_traceback(exception.__traceback__)
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pluggy/_callers.py", line 77, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>           ^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/pytest_asyncio/plugin.py", line 612, in pytest_collectstart
INTERNALERROR>     pyobject = collector.obj
INTERNALERROR>                ^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/python.py", line 310, in obj
INTERNALERROR>     self._obj = obj = self._getobj()
INTERNALERROR>                       ^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/python.py", line 528, in _getobj
INTERNALERROR>     return self._importtestmodule()
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/python.py", line 617, in _importtestmodule
INTERNALERROR>     mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
INTERNALERROR>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/site-packages/_pytest/pathlib.py", line 567, in import_path
INTERNALERROR>     importlib.import_module(module_name)
INTERNALERROR>   File "/opt/hostedtoolcache/Python/3.12.0/x64/lib/python3.12/importlib/__init__.py", line 90, in import_module
INTERNALERROR>     return _bootstrap._gcd_import(name[level:], package, level)
INTERNALERROR>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 1381, in _gcd_import
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 1354, in _find_and_load
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 1325, in _find_and_load_unlocked
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 929, in _load_unlocked
INTERNALERROR>   File "<frozen importlib._bootstrap_external>", line 994, in exec_module
INTERNALERROR>   File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
INTERNALERROR>   File "/home/runner/work/my-repository/my-repository/src/module/__init__.py", line 3, in <module>
INTERNALERROR>     from .module import foobar
INTERNALERROR>   File "/home/runner/work/my-repository/my-repository/src/module/module.py", line 34, in <module>
INTERNALERROR>     raise ImportWarning(
INTERNALERROR> ImportWarning: Please import from ...
@seifertm seifertm added the bug label Dec 7, 2023
@seifertm seifertm added this to the v0.23.3 milestone Dec 7, 2023
@seifertm
Copy link
Contributor

seifertm commented Dec 7, 2023

I understand that pytest-asyncio isn't supposed to trigger an INTERNALERROR.

As far as I understand, pytest imports the src/module/__init__.py during test collection. This, in turn, imports src/module/module.py, which raises the ImportWarning. How does pytest deal with this case? In other words what's the expected behavior?

If I raise an ImportWarning in a test module, the test run fails. Your case is slightly different, because the warning appears in a non-test module.

@Stausssi
Copy link
Author

Stausssi commented Dec 7, 2023 •
edited

I'm not quite sure if src/module/__init__.py is actually imported during collection. The tests are contained in a different module with src/module being imported via importlib.import_module. So there is no way pytest should discover the module before actually running the tests.

I've added a small print("Collection") statement to src/module/__init__.py and spotted the following:

pytest-asyncio 0.21.1

platform darwin -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0
plugins: cov-4.1.0, asyncio-0.21.1, anyio-3.7.1
asyncio: mode=Mode.STRICT
collecting ...
collected 38 items

pytest-asyncio 0.23.2

platform darwin -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0
plugins: cov-4.1.0, asyncio-0.23.2, anyio-3.7.1
asyncio: mode=Mode.STRICT
collecting ... Collection
collected 13 items

Note that Collection is only printed with pytest-asyncio 0.23.2.

@sanderr
Copy link

sanderr commented Dec 11, 2023

I have the impression that the new version more greedily loads __init__.py files than before. Consider a test setup with resources that may look like a Python package. They are expected to be loaded by certain tests under specific circumstances. With this release, pytest loads them at startup instead:

tests/
└── resources
    └── __init__.py

With __init__.py:

raise Exception("This file should not be loaded by pytest")

Running pytest tests/ now results in this exception because the file is loaded. With 0.21.1 this is not the case.

The scenario I'm describing takes place inside the tests/ directory, but otherwise it looks similar to the issue described above.

@sanderr sanderr mentioned this issue Dec 11, 2023
Closed
@seifertm
Copy link
Contributor

Thanks for the investigations. I'll read have a look at how pytest handles these issues during collection and get back to you.

@seifertm seifertm mentioned this issue Dec 20, 2023
Closed
@wouterdb wouterdb mentioned this issue Dec 22, 2023
Closed
@seifertm seifertm mentioned this issue Dec 24, 2023
Merged
@seifertm
Copy link
Contributor

#728 addresses the INTERNALERROR caused by the ImportWarning.

It does not change the "more greedy" import of init.py.
@sanderr was this just an observation of yours or is this behaviour causing issues when upgrading to v0.23?

@sanderr
Copy link

sanderr commented Dec 24, 2023

In our case it is actually causing issues. In short, we have Python files in our tests dir that should only be loaded in a specific context. I'll elaborate in more detail below. We could probably work around it if we need to, but for now we have pinned pytest-asyncio to the previous version until this discussion reaches a conclusion.

Our specifix use case: we are developing a Python project with a plugin system. Put simply, these plugins are Python packages that register themselves at load time. They are expected to be loaded by a custom importlib finder/loader and they do not work as standalone package.

Since this plugin system is an integral part of the application we're developing, we test their behavior thoroughly. Therefore we have a bunch of these plugin "packages" as resources underneath our tests directory. They are meant as nothing more than data: our application will find and load them when appropriate. They will raise an error when loaded as a standalone package, which pytest v0.23 now does.

@seifertm
Copy link
Contributor

Understood. That's a fancy (but valid) use case :)
I'll move this to a separate issue, since it's not directly related to the ImportWarning issue described by the OP.

@seifertm seifertm mentioned this issue Dec 24, 2023
Closed
@seifertm seifertm modified the milestones: v0.23.3, v0.23 Jan 1, 2024
@seifertm
Copy link
Contributor

seifertm commented Jan 1, 2024

@Stausssi This should be resolve with pytest-asyncio v0.23.3

@karolpawlowski
Copy link

@Stausssi This should be resolve with pytest-asyncio v0.23.3

Hi @seifertm , for me the issue with related imports still exists in v0.23.3. Freezing at v0.21.1 still helps.

@seifertm
Copy link
Contributor

seifertm commented Jan 2, 2024

@karolpawlowski I assume you're referring to the bug that causes an INTERNALERROR at test collection time and not about importing __init__.py files (which was moved to #729)

Can you provide a code snipped to reproduce the INTERNALERROR?

@seifertm seifertm reopened this Jan 2, 2024
@karolpawlowski
Copy link

Ah, yes. I have the same traceback that is currently in #729 . You could close this one. Thanks.

@seifertm seifertm closed this as completed Jan 2, 2024
@Stausssi
Copy link
Author

Stausssi commented Jan 3, 2024

@Stausssi This should be resolve with pytest-asyncio v0.23.3

For me the issue still exists with the same traceback. Please let me know if I can assist in any way.

@seifertm
Copy link
Contributor

seifertm commented Jan 3, 2024

I think you're experiencing two separate issues leading to the same result.

The first is that any warnings raised in modules cause an INTERNALERROR:

raise ImportWarning()

async def test_errors_out():
    pass

This was caused by the fact that module imports were triggered before the actual collection phase. As a result, any exceptions weren't handled correctly by pytest. (To be clear, this is not a pytest issue, but an issue with pytest-asyncio)

The other issue is that __init__.py modules are loaded more eagerly, which wasn't the case in older versions pytest-asyncio (or pytest without plugins). For the sake of traceability, I created a separate issue for this (see #729).

The problem is that the loading of __init__.py files is still performed before the actual test collection phase and circumvents the pytest error handling. Your __init__.py file triggers a module import outside of the collection phase, which, in turn, crashes the pytest run.

@Stausssi Unless you have a strong opinion against it, I suggest to keep this issue closed and would appreciate if you'd subscribe to updates on #729.

@Stausssi
Copy link
Author

Stausssi commented Jan 3, 2024

Can confirm. If I remove the __init__.py everything works as intended. Tracking in #729 is fine for me, you can keep this closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
v0.23
Development

Successfully merging a pull request may close this issue.

Address internal pytest errors during collection seifertm/pytest-asyncio
4 participants
@karolpawlowski @sanderr @seifertm @Stausssi