- Notifications
You must be signed in to change notification settings - Fork 9
🐛 Ignore collection failures in non-tests #22
base: master
Are you sure you want to change the base?
Conversation
This avoids mysterious errors: reverbc/pylint-pytest#20
ssbarnea left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks ok but please remove emoji from PR before merging it. I have nightmares about emoji in git changelog.
Refs: * reverbc/pylint-pytest#20 * reverbc/pylint-pytest#21 TODO: Revert once reverbc/pylint-pytest#22 is merged.
| I hope this feature will be incorporated soon because my F6401 is really annoying me. Sometimes it just show |
reverbc left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the PR. I validate it and it should avoid raising warnings in unrelated modules.
Also please help to note this in CHANGELOG.md
| if (ret != pytest.ExitCode.OK or legitimate_failure_paths) and is_test_module: | ||
| self.add_message( | ||
| 'cannot-enumerate-pytest-fixtures', | ||
| args=' '.join(legitimate_failure_paths | {node.file}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that there's a potential duplication from this line: legitimate_failure_paths can contain relative paths, but the node.file is in absolute path. In my env it's throwing out pytest warning with duplicated paths:
sandbox/test_parent.py:1:0: F6401: pylint-pytest plugin cannot enumerate and collect pytest fixtures. Please run `pytest --fixtures --collect-only sandbox/test_parent.py /Users/reverbc/Workspace/pylint-pytest/sandbox/test_parent.py` and resolve any potential syntax error or package dependency issues (cannot-enumerate-pytest-fixtures) Where the sandbox/test_parent.py and /Users/reverbc/Workspace/pylint-pytest/sandbox/test_parent.py are pointing to the same file.
Can you help to unify the path strings, maybe with str(Path(...).absolute()) for both collection_report.nodeid and node.file?
| @webknjaz thanks, this does fix my issue. |
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
| Any further progress on this? Really appreciate if this mr can be merged with a new version released. |
| I've confirmed that this pull request prevents the errors I reported. |
| @reverbc Can this get merged & published? |
This change applies the pre-existing patterns to identify if the files with collection problems are tests. It is then used to eliminate the false-positives of F6401 (cannot-enumerate-pytest-fixtures).
As a side effect, this patch also includes precise file paths that may be used to reproduce the problem.
Fixes #20
Fixes #21