🐛 Ignore collection failures in non-tests

[stdedos]

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: reverbc#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

• Capture and return stdout and stderr (instead of trashing them)
• Fix the 'duplicate-path' error by relative-izing the paths before unioning them
• Update tests to test for both conditions

Additionally, fix two used-before-assignment pylint issues (stdout, stderr) 🤪

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files

@@            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	Coverage Δ
3.10	92.32% <95.65%> (+0.25%)	⬆️
3.11	92.32% <95.65%> (+0.25%)	⬆️
3.6	92.32% <95.65%> (+0.25%)	⬆️
3.7	92.32% <95.65%> (+0.25%)	⬆️
3.8	92.32% <95.65%> (+0.25%)	⬆️
3.9	92.32% <95.65%> (+0.25%)	⬆️
macos-latest	92.32% <95.65%> (+0.25%)	⬆️
ubuntu-20.04	92.32% <95.65%> (+0.25%)	⬆️
ubuntu-latest	92.32% <95.65%> (+0.25%)	⬆️
windows-latest	92.32% <95.65%> (+0.25%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
pylint_pytest/checkers/fixture.py	91.15% <100.00%> (+0.24%)	⬆️
tests/test_cannot_enumerate_fixtures.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Pierre-Sassoulas]

Should we simply disable the message ? The solution is complex and non obvious to me. Maybe it's a nice check except if you're actually testing pytest fixtures instead of using them in your code ?

[stdedos]

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]

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

[stdedos]

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]

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).

[Pierre-Sassoulas]

LGTM !

[webknjaz]

@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]

Apologies @webknjaz. I did those MRs multiple times.

... I obviously missed something 😅

[webknjaz]

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]

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: 69ad82f) but this MR is literally the first functional change merged. I'm afraid is too late to do something meaningful ❤️

[webknjaz]

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:

• https://github.com/re-actors/alls-green — a unified single-gate check for branch protection
• https://github.com/re-actors/checkout-python-sdist — a way to ensure that the source distributions you ship are usable downstream
• https://github.com/pypa/gh-action-pypi-publish — a PyPA-blessed way of publishing to PyPI from GHA; you can also stop using long-living API token secret with this — see https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

[webknjaz]

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]

There you go 🙃 8310a88

[webknjaz]

Yaaaay! 🎉