Skip to content

Flake8 integration #11

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

Closed
ilyakamens opened this issue Feb 4, 2024 · 19 comments
Closed

Flake8 integration #11

ilyakamens opened this issue Feb 4, 2024 · 19 comments

Comments

@ilyakamens
Copy link

Hi,

I just discovered this repository via pytest-dev/pytest#5706 after being mildly surprised that @pytest.mark.only didn't exist. Thanks for building this!

Would it be possible to add a Flake8 plugin to prevent commits? I see the Pylint integration, but my company doesn't use Pylint, and I don't want to introduce that just for this.

Thank you.

@theY4Kman
Copy link
Owner

I see that Flake8 also supports AST-based plugins, so I don't see why not. I've not written one before (or, really, ever used a flake8 config that someone else hadn't written); what sort of configuration might you expect out of a plugin? Do they commonly have configs, anywho?

And sorry for the late response!

@theY4Kman
Copy link
Owner

I made some good progress on this tonight, FYI. I didn't realize flake8 plugins simply used the built-in ast module. That certainly makes things easier!

@theY4Kman
Copy link
Owner

Alright, I opened up #12 with my implementation. Would you mind pulling it down and seeing if it works for you?

I have a tarball source dist if that makes things easier: pytest_only-2.1.0.tar.gz

@ilyakamens
Copy link
Author

Hi! Sorry for the delay. Thanks for doing this! Is there any configuration I need to add to make this work? For reference, we have the following in our tox.ini file:

[flake8]
max-line-length = 100
inline-quotes = double
exclude = devops/*
extend-ignore = D100,D104,D105,E203,E402,E741

@ilyakamens
Copy link
Author

I just ran the following:

pip install git+https://github.com/theY4Kman/pytest-only.git@55ce0042c2c74d3a913c3b45ec8d454c9eb8cbe5#egg=pytest-only

But I was still able to commit a change that included @pytest.mark.only. Running the following, it doesn't look like pytest-only is registered as a plugin:

$ python -m flake8 --version
6.0.0 (flake8-docstrings: 1.7.0, flake8-isort: 6.0.0, flake8-quotes: 3.3.2, mccabe: 0.7.0, pycodestyle:
2.10.0, pyflakes: 3.0.1) CPython 3.10.10 on Linux

What am I missing?

@theY4Kman
Copy link
Owner

Hmm, straight from the git ref? I'd still expect pip to perform the build-system steps just fine. I looked at some other poetry-using flake8 plugins/extensions, and it seemed to need an entry point under flake8.extension, keyed as the error prefix produced by the plugin. I see this got captured by the wheel, but GitHub wouldn't accept the .whl extension. Basically, I'm wondering if that entry point got lost — somehow — when installing by git ref. Would you mind trying this wheel in a zip? :P

pytest_only-2.1.0-py3-none-any.whl.zip

I see its /pytest_only-2.1.0.dist-info/entry_points.txt looks like

[flake8.extension]
PTO=pytest_only.ext.flake8:PytestOnlyMarkChecker

[pytest11]
only=pytest_only.plugin

I would expect to see this reproduced in your site-packages, as well

@ilyakamens
Copy link
Author

Sure thing. What should I do with pytest_only-2.1.0-py3-none-any.whl / where should I put it?

@theY4Kman
Copy link
Owner

You ought to simply be able to do pip install pytest_only-2.1.0-py3-none-any.whl

@ilyakamens
Copy link
Author

I'll try again in just a few moments.

@ilyakamens
Copy link
Author

Here I'm installing it as described:

root@e65f6cb58885:/app# pip install pytest_only-2.1.0-py3-none-any.whl
Processing ./pytest_only-2.1.0-py3-none-any.whl
Requirement already satisfied: pytest>=7.1 in /usr/local/lib/python3.10/site-packages (from pytest-only==2.1.0) (7.4.2)
Requirement already satisfied: pluggy<2.0,>=0.12 in /usr/local/lib/python3.10/site-packages (from pytest>=7.1->pytest-only==2.1.0) (0.13.1)
Requirement already satisfied: exceptiongroup>=1.0.0rc8 in /usr/local/lib/python3.10/site-packages (from pytest>=7.1->pytest-only==2.1.0) (1.1.0)
Requirement already satisfied: tomli>=1.0.0 in /usr/local/lib/python3.10/site-packages (from pytest>=7.1->pytest-only==2.1.0) (1.2.3)
Requirement already satisfied: packaging in /usr/local/lib/python3.10/site-packages (from pytest>=7.1->pytest-only==2.1.0) (23.2)
Requirement already satisfied: iniconfig in /usr/local/lib/python3.10/site-packages (from pytest>=7.1->pytest-only==2.1.0) (1.1.1)
Installing collected packages: pytest-only
  Attempting uninstall: pytest-only
    Found existing installation: pytest-only 2.0.0
    Uninstalling pytest-only-2.0.0:
      Successfully uninstalled pytest-only-2.0.0
Successfully installed pytest-only-2.1.0

And hey! It's working:

$ git commit -a
nlp/tests/test_utils.py:85:6: PTO01: Unexpected focused test using pytest.mark.only: def test_metrics

Thank you!

@theY4Kman
Copy link
Owner

Excellent! Lemme merge that PR and cut a release, then :) I'll be going with that 2.1.0 version number.

@theY4Kman
Copy link
Owner

Version 2.1.0 released on PyPI

@ilyakamens
Copy link
Author

Hello! Me again. So, it worked locally, but I get the following in Jenkins:

image

Any idea what could be going on here?

@theY4Kman
Copy link
Owner

theY4Kman commented Mar 9, 2024 via email

@ilyakamens
Copy link
Author

Thank you kindly!

@theY4Kman
Copy link
Owner

Alright, version 2.1.1 has been pushed to PyPI.

The risk of future issues should be far diminished... but not nil :P If something crops up again, I might find a library for interpreting the AST. While the AST is easy to handle, it's also not the semantic level this plugin needs to be operating at — like, there are a big handful of different ways to perform assignments, but this plugin only needs to know what value was assigned to pytestmark.

@ilyakamens
Copy link
Author

Just merged this update into main. I think this is working as expected. Thanks again!

@theY4Kman
Copy link
Owner

My pleasure :)

@jack-zins
Copy link

Getting an error with chained assignments with the flake8 extension. Opened a PR with test cases and proposed fix. #13

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

3 participants