Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monkeypatching stdout leads to AttributeError in flake8 #9217

Closed
jaraco opened this issue Oct 20, 2021 · 9 comments
Closed

Monkeypatching stdout leads to AttributeError in flake8 #9217

jaraco opened this issue Oct 20, 2021 · 9 comments

Comments

@jaraco
Copy link
Contributor

jaraco commented Oct 20, 2021

In PyCQA/flake8#1419, a user reported an issue with the most recent release of flake8 in which an AttributeError will occur if sys.stdout is patched with an object without a buffer attribute.

This issue is triggered when pytest patches sys.stdout and the pytest-flake8 plugin is used.

According to the flake8 issue, it's the responsibility of this project to address the issue as monkeypatcher of sys.stdout.

@asottile
Copy link
Member

pytest is not the patcher in this case -- pytest-flake8 is

@jaraco
Copy link
Contributor Author

jaraco commented Oct 20, 2021

I did search the codebase for pytest-flake8 and the term stdout did not appear in the code, so I checked that pytest does do some patching and it does, so that's what led me to believe pytest was the cause.

@jaraco
Copy link
Contributor Author

jaraco commented Oct 20, 2021

Seems the issue was already reported at tholo/pytest-flake8#81, but Google failed me (an exact search for the error message did not find the issue).

@sigmavirus24
Copy link

@jaraco https://github.com/tholo/pytest-flake8/blob/9db176caba1738df680bf574619b7f0f2dd85ac4/pytest_flake8.py#L119 is the part that it uses monkey-patching. The reality is that pytest-flake8 is:

  • incorrectly patching stdout
  • using internals of Flake8 documented as unstable and not a public API
  • despite using internals is using the least optimal way of collecting violations reported by Flake8 by reading stdout instead of building code to report it directly in memory so they don't need to patch anything.

This almost makes me want to distribute a pytest plugin for flake8 from flake8

@thomascobb
Copy link

This almost makes me want to distribute a pytest plugin for flake8 from flake8

I think that would make quite a few people quite a bit happier. I'd certainly back it, and would help in any way I could...

@asottile
Copy link
Member

imo running flake8 inside pytest is an antipattern (it certainly is going to be much much slower) so it probably isn't going to happen

@sigmavirus24
Copy link

sigmavirus24 commented Nov 10, 2021

I should have emphasized "almost"

I know flake8 in purest pytest simplifies things for some teams (mine included at work) but it's still not a good pattern

@RonnyPfannschmidt
Copy link
Member

with the advent of pre-commit my personal opinion is that given the available tooling the test runner by now is absolutely the wrong place for quick code checks

@sigmavirus24
Copy link

While I agree pytest is the worst place for this, there are systems that are limited and out of a developers control sometimes and the only way to be sure any edit to the code is compliant is via pytest-pyflakes or pytest-flake8. I don't think the need is abated by local machine tooling, especially as GitHub tries to push people into the cloud for development. I hate it, but there's reasons why people need this

declension added a commit to quodlibet/quodlibet that referenced this issue Jul 26, 2022
* See pytest-dev/pytest#9217
* Somehow we are pulling in pytest-flake8 or something else that patches stdout
* This seems to be the standard / only real solution currently
* Can only be seen when flake8 reports errors...
declension added a commit to quodlibet/quodlibet that referenced this issue Jul 26, 2022
* See pytest-dev/pytest#9217
* Somehow we are pulling in pytest-flake8 or something else that patches stdout
* This seems to be the standard / only real solution currently
* Can only be seen when flake8 reports errors...
robertodr added a commit to dev-cafe/parselglossy that referenced this issue Aug 17, 2022
The `pytest-flake8` plugin broke with a recent update of `flake8`.
According to this thread: pytest-dev/pytest#9217 (comment)
running these kinds of checks within `pytest` is an anti-pattern and should be avoided.
So I've moved `black`, `mypy`, and `flake8` checks away from their pytest plugins.
gdubicki added a commit to voxpupuli/puppetboard that referenced this issue Oct 23, 2022
gdubicki added a commit to voxpupuli/puppetboard that referenced this issue Oct 23, 2022
gdubicki added a commit to voxpupuli/puppetboard that referenced this issue Oct 24, 2022
gdubicki added a commit to voxpupuli/puppetboard that referenced this issue Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants