pylint-dev / pylint-pytest

A Pylint plugin to suppress pytest-related false positives.
https://pypi.org/project/pylint-pytest/
MIT License
14 stars 3 forks source link

πŸ› Ignore collection failures in non-tests #15

Closed stdedos closed 9 months ago

stdedos commented 10 months ago

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 reverbc#20 Fixes reverbc#21

Signed-off-by: Sviatoslav Sydorenko wk@sydorenko.org.ua

Replayed; Source PR: https://github.com/reverbc/pylint-pytest/pull/22

Additionally, satisfied the https://github.com/pylint-dev/pylint-pytest's .pre-commit-config.yaml

Signed-off-by: Stavros Ntentos 133706+stdedos@users.noreply.github.com

Additionally, fix two used-before-assignment pylint issues (stdout, stderr) πŸ€ͺ

Signed-off-by: Stavros Ntentos 133706+stdedos@users.noreply.github.com

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (3cee43d) 92.25% compared to head (b8f9dc4) 92.50%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #15 +/- ## ========================================== + Coverage 92.25% 92.50% +0.24% ========================================== Files 18 18 Lines 542 560 +18 Branches 104 109 +5 ========================================== + Hits 500 518 +18 Misses 29 29 Partials 13 13 ``` | [Flag](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | Coverage Ξ” | | |---|---|---| | [3.10](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [3.11](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [3.6](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [3.7](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [3.8](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [3.9](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [macos-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [ubuntu-20.04](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [ubuntu-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | | [windows-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `92.32% <95.65%> (+0.25%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | Coverage Ξ” | | |---|---|---| | [pylint\_pytest/checkers/fixture.py](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev#diff-cHlsaW50X3B5dGVzdC9jaGVja2Vycy9maXh0dXJlLnB5) | `91.15% <100.00%> (+0.24%)` | :arrow_up: | | [tests/test\_cannot\_enumerate\_fixtures.py](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/15?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev#diff-dGVzdHMvdGVzdF9jYW5ub3RfZW51bWVyYXRlX2ZpeHR1cmVzLnB5) | `100.00% <100.00%> (ΓΈ)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

stdedos commented 10 months ago

I think this is a sanity check whether pytest can process a ficture created at the repository pylint is checking.

I am not sure how much "valuable" it is in CI scenarios (pytest will fail running in that case anyway). Might be ... for the lazy? A "quick" way to get feedback (given that pylint might run after every file save)?

I haven't dug the git-log for that. We can disable it by default on v2 though?

Pierre-Sassoulas commented 10 months ago

I meant disable it in pylint-pytest for our own repository, not disable it for users.

stdedos commented 10 months ago

I meant disable it in pylint-pytest for our own repository, not disable it for users.

I'd ... need to see that when we revisit re-enabling pylint warnings.

... or I didn't understand the question πŸ˜“

Pierre-Sassoulas commented 10 months ago

I'm not sure I understood this MR well. I'm going to focus on compatibility with pylint 3.0 for now (maybe using the addon in pylint itself would help me understand how it works then).

webknjaz commented 4 months ago

@stdedos nice that you picked up my old PR. Too bad that I'm only seeing this now and had the plugin version pinned in my configs for all these years :(

For future, I recommend either re-pushing unmodified commits or at least using the Co-Authored-By: trailers so that the patches are credited correctly: https://hynek.me/til/easier-crediting-contributors-github/. This would've probably notified me earlier too (I think)...

stdedos commented 4 months ago

Apologies @webknjaz. I did those MRs multiple times.

... I obviously missed something :sweat_smile:

webknjaz commented 4 months ago

Yep, it seems like you changed the authorship to Signed-Off-By and dropped the metadata from the commit's built-in fields, losing this information. AFAIK Signed-Off-By should only be added by somebody having a permission from the author as it's used as a DSO in many places, basically as a declaration that the author promises they're in legal possession of a patch being sent. My understanding is that the common recommendations here are the same as for the copyright lines in code β€” only one adding such a line in the first place is supposed to touch it and other people would need to add their lines separately. IANAL though, this is my personal understanding of the meaning of such trailers.

stdedos commented 4 months ago

Yep, it seems like you changed the authorship to Signed-Off-By and dropped the metadata from the commit's built-in fields, losing this information

Basically I'm trying to keep each commit in-line with a Green CI. As CI was done "after" your commit, "I had no option other to re-write it". But ofc I could've kept author/committer and/or Co-authored-by:.


Yeah - in retrospect, I would've done much less: Just pull the commit.

I have done shenanigans to remediate ill-crafted commits (see: https://github.com/pylint-dev/pylint-pytest/commit/69ad82f1f07a6a1cb33e33266ad46f2fcb006b15) but this MR is literally the first functional change merged. I'm afraid is too late to do something meaningful :heart:

webknjaz commented 4 months ago

You did a great job resurrecting the project regardless :) Just wanted to log the confusion, that's all.

P.S. Since you mentioned liking evergreen CI and maintenance ergonomics, here's a few of my GitHub Actions that you may enjoy having here:

webknjaz commented 4 months ago

I'm afraid is too late to do something meaningful ❀️

That's actually fixable by making a true merge commit (not a squash or rebase) from the original branch and having it as this natural merge in master. But it's probably not something to care about.

stdedos commented 3 months ago

There you go :upside_down_face: https://github.com/pylint-dev/pylint-pytest/commit/8310a88916e4b0b7462fc525c56d60b9af4dc126

webknjaz commented 3 months ago

Yaaaay! πŸŽ‰