Open whg517 opened 3 years ago
After debugging the source code, I found out that the cause of my problems was an error in pylint-pytest
when running pytest
to collect fixtures from source code, and then pylint-pytest
passed the error to PyLint.
My source code had a type annotation error that caused pytest to look for a fixture from that module that was wrong, and the error was passed to pylint. But why there is a different output is not clear to me.
From debugging the source code, we know that pylint-pytest
registers itself with pylint, and when pylint checks all files, it passes the files to pylint-pytest's FixtureChecker
.
The visit_module
method in the FixtureChecker
passes the file to pytest, running pytest <module_file> --fixtures --collect-only
, At the same time load the FixtureCollector
plug-in into pytest.
In pytest_collectreport
, if an error is reported by pytest, it is logged and the error information is passed to pytest.
I don't think this logic makes sense. Pytest should only collect fixtures from the test modules, and instead of collecting fixtures from all modules, Pylint-Pytest should filter out the source code when PyLint checks.
Hope to fix this problem.
I'm seeing this today as well.
Curious about this statement:
Pytest should only collect fixtures from the test modules, and instead of collecting fixtures from all modules, Pylint-Pytest should filter out the source code when PyLint checks.
That makes sense to me, because I don't think I'd want to put fixtures in source code, but how would pylint (and subsequently pytest) know the difference between source and test code?
Hi @miketheman
In pylint-pytest
logic, pylint-pytest
or pytest
does not know the difference between the source code and the test code. Because pylint
checks all files, for example pylint src tests
checks all python files in both directories. So pylint-pytest
simply passes the file to
pytest
that pytest
needs to check and lets pytest
collect fixtures in that file.
If aone source file is bad small, pytest will error.I had these problems because one of the methods in the source code had the wrong return value type. However, it should be indicated exactly by pylint
, but the information above obviously doesn't pinpoint exactly where the problem is and what caused it. It just tells you that the loading fixture is wrong. And it has nothing to do with the wrong file.
If we only talk about pytest
, pytest
will load its own configuration before running, such as from pytest.ini
or setup.cfg
, depending on pytest itself. For example, I configured it in setup.cfg
[tool:pytest]
testpaths = tests
python_files = tests.py test_*.py
Then pytest will only look for the test code in the tests
directory. It also filters the non-test source code files in the directory.
From this point on, if I fix this problem, I have a hypothesis:
Refer to pytest's filter logic for test files and filter out any logic that does not conform to the rules before passing the files to pytest for inspection.
Since I was having conflicts with dynaconf
when using the pylint
plugin pylint-django
, I lowered my detection requirements and switched to the less stringent flake8
detection code. And that's just one of my current alternatives.
@whg517 Thanks for the excellent explanation! This appears similar to #16 and #20 - so possibly if this is fixed, all three will be resolved.
Refer to pytest's filter logic for test files and filter out any logic that does not conform to the rules before passing the files to pytest for inspection.
I did some debugging and came to a similar conclusion: if there's a way for the plugin to skip non-pytest files altogether, it'd fix the problem. Maybe even w/o taping into _pytest
this plugin could introduce a wildcard setting for including/excluding files?
P.S. In my case, the underlying collection error was caused by the fact that the project uses implicit namespaces and so feeding full file path to modules confuses Python when it tries to resolve relative imports (because it doesn't know what the root package is). It is easy to reproduce once you run the proper command (in this case python -m pytest --fixtures --collect-only octomachinery/github/models/events.py
):
=========================================================================== ERRORS ===========================================================================
___________________________________________________ ERROR collecting octomachinery/github/models/events.py ___________________________________________________
octomachinery/github/models/events.py:17: in <module>
from ...utils.asynctools import aio_gather
E ImportError: attempted relative import beyond top-level package
Any = typing.Any
Iterable = typing.Iterable
Mapping = typing.Mapping
TYPE_CHECKING = False
TextIO = <class 'typing.TextIO'>
Type = typing.Type
Union = typing.Union
_GidgetHubEvent = <class 'gidgethub.sansio.Event'>
__builtins__ = <builtins>
__cached__ = '/home/runner/work/octomachinery/octomachinery/octomachinery/github/models/__pycache__/events.cpython-39.pyc'
__doc__ = 'Generic GitHub event containers.' __file__ = '/home/runner/work/octomachinery/octomachinery/octomachinery/github/models/events.py'
__loader__ = <_frozen_importlib_external.SourceFileLoader object at 0x7ff8f7a0ab80>
__name__ = 'models.events'
__package__ = 'models'
__spec__ = ModuleSpec(name='models.events', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7ff8f7a0ab80>, origin='/home/runner/work
/octomachinery/octomachinery/octomachinery/github/models/events.py')
annotations = _Feature((3, 7, 0, 'beta', 1), (3, 10, 0, 'alpha', 0), 16777216)
attr = <module 'attr' from '/home/runner/work/octomachinery/octomachinery/.tox/pre-commit/lib/python3.9/site-packages/attr/__init__.py'>
cast = <function cast at 0x7ff8f97e5430>
json = <module 'json' from '/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/json/__init__.py'>
pathlib = <module 'pathlib' from '/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/pathlib.py'>
uuid = <module 'uuid' from '/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/uuid.py'>
warnings = <module 'warnings' from '/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/warnings.py'>
___________________________________________________ ERROR collecting octomachinery/github/models/events.py ___________________________________________________
ImportError while importing test module '/home/runner/work/octomachinery/octomachinery/octomachinery/github/models/events.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/importlib/__init__.py:127: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
octomachinery/github/models/events.py:17: in <module>
from ...utils.asynctools import aio_gather
E ImportError: attempted relative import beyond top-level package
================================================================== short test summary info ===================================================================
ERROR octomachinery/github/models/events.py - ImportError: attempted relative import beyond top-level package
ERROR octomachinery/github/models/events.py
=========================================================== no tests collected, 2 errors in 0.09s ============================================================
P.P.S. Another problem is that the error message gives an example with a dummy file path making it hard to debug. I think the message could be improved to specifically mention what file exactly causes the problem, instead of saying it's path/to/current/module.py
.
This should fix it: https://github.com/reverbc/pylint-pytest/pull/22.
cc @reverbc @ssbarnea
Pytest should only collect fixtures from the test modules, and instead of collecting fixtures from all modules, Pylint-Pytest should filter out the source code when PyLint checks.
I believe we have other use cases that pytest
should care about non-test modules.
Similar to #2 use case, test author can use separate modules (let's call them helper modules) to organize their fixtures/helpers if they have a large and complex test framework. In that case, if there's a coding error in one of the helper modules and causing the pytest
failed to enumerate the fixtures, pylint-pytest
will not be able to suppress the FP. That was the main intention when I introduced this new warning, and unfortunately the fixes in #22 will not be able to prevent that from happening since it'll ignore the collection errors from helper modules.
I'm now more inclined to temporary remove this cannot-enumerate-pytest-fixtures
check if it cannot correctly identify which modules actually contain pytest fixtures until we have a better solution to detect that.
I do not think so. Checking the test modules should load dependencies in the background anyway. If they are loaded properly, they'll return the fixtures. If not, they'll cause errors.
The problem with pointing pytest at random files is that those may be loaded in different ways (runpy vs scripts) and depending on whether there's __init__.py
the module loader may or may not recognize the package called tests
, for example.
So I'm pretty sure that pointing at the test files will identify the missing fixtures one way or another.
Describe the bug
Can you explain the prompt 'F6401' when I run pylint
pylint-pytest plugin cannot enumerate and collect pytest fixtures. Please run `pytest --fixtures --collect-only path/to/current/module.py` and resolve any potential syntax error or package dependency issues (Can-enumerate-pytest-fixtures)
is the reason?I would like to know how it works, or why it appears, and sometimes has different outputs. The same code, sometimes two, sometimes more. I was depressed.
I did run
pytest --fixtures --collect-only
without any unusual hints and my tests were normal.Description:
After I fine-tune my existing code, including running
pylint
,pytest
, andisort
, everything works. I added a new packageexecutor
with three modules, one is the abstract module ofbase.py
, two are corresponding to different implementation modules(local.py
,docker.py
).Then I run
isort
, andpylint
works fineThen I import the base class and two implementation classes in the module's
__init__.py
file, and add a factory method.When I run
pylint
again, the input tells me that some of the test modules have problems with F6401.Again, I want to emphasize that everything was fine until I added this module. But now I just added the source code of this module, this exception will appear.
What makes it even more confusing to me is that the module I'm prompted doesn't include any fixtures. I ran
pylint
again and found that F6401 has more test modules (several times more than last time).I've been using PyLint for a new project to check for a mode-by-module migration, and when I migrate to this module, I can't continue.
To Reproduce
Package versions
(add any relevant pylint/pytest plugin here)
load-plugins = pylint_pytest,
Folder structure
File content :
When I start this code, I get F6401, and when I comment it out, everything is fine.
crawlerstack_spiderkeeper.executor::__init__.py
pylint output with the plugin :
Why is it different before and after?
(Optional) pytest output from fixture collection
Expected behavior
Everything is ok
Additional context Add any other context about the problem here.