Skip to content
This repository was archived by the owner on Dec 8, 2022. It is now read-only.

Conversation

@webknjaz
Copy link

@webknjaz webknjaz commented Aug 17, 2021

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

@webknjaz
Copy link
Author

Hey @nedbat, could you check this PR in the context of your report #20?

webknjaz referenced this pull request in nedbat/scriv Aug 17, 2021
Copy link

@ssbarnea ssbarnea left a 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.

webknjaz added a commit to sanitizers/octomachinery that referenced this pull request Aug 18, 2021
@whg517
Copy link

whg517 commented Aug 30, 2021

I hope this feature will be incorporated soon because my F6401 is really annoying me. Sometimes it just show cannot-enumerate-pytest-fixtures and I don't know what went wrong,

Copy link
Owner

@reverbc reverbc left a 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}),
Copy link
Owner

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?

@reverbc reverbc mentioned this pull request Aug 30, 2021
3 tasks
@nedbat
Copy link

nedbat commented Aug 31, 2021

@webknjaz thanks, this does fix my issue.

bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Feb 24, 2022
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Feb 24, 2022
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Mar 2, 2022
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
bruno-fs added a commit to bruno-fs/quipucords that referenced this pull request Mar 3, 2022
When reverbc/pylint-pytest#22 gets merged we might reenable this check.
@huxuan
Copy link

huxuan commented Mar 28, 2022

Any further progress on this? Really appreciate if this mr can be merged with a new version released.

@nedbat
Copy link

nedbat commented Mar 28, 2022

I've confirmed that this pull request prevents the errors I reported.

@marcgibbons
Copy link

@reverbc Can this get merged & published?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

7 participants