theY4Kman / pytest-only

pytest plug-in to run single tests with the "only" mark (perfect for use with auto-rerunners!)
https://pypi.org/project/pytest-only/
MIT License
18 stars 5 forks source link

Flake8 integration #11

Closed ilyakamens closed 8 months ago

ilyakamens commented 9 months ago

Hi,

I just discovered this repository via https://github.com/pytest-dev/pytest/issues/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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

theY4Kman commented 8 months ago

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

ilyakamens commented 8 months ago

I'll try again in just a few moments.

ilyakamens commented 8 months ago

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 commented 8 months ago

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

theY4Kman commented 8 months ago

Version 2.1.0 released on PyPI

ilyakamens commented 8 months ago

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

image

Any idea what could be going on here?

theY4Kman commented 8 months ago

Ah, the plug-in is doing some brittle handling of the AST :P It appears it's assuming all assignments done at class or module level are performed directly upon names, not attributes. It looks like it breaks if a.b = c is used... which is embarrassing :P I'll fix it later tonight (along with some more expensive tests) and cut a patch release.

On Fri, Mar 8, 2024, 17:24 Ilya Kamens @.***> wrote:

Hello! Me again. So, it worked locally, but I get the following in Jenkins: image.png (view on web) https://github.com/theY4Kman/pytest-only/assets/3293811/253a1895-d724-4ea4-a72b-3e9058766ca9

Any idea what could be going on here?

— Reply to this email directly, view it on GitHub https://github.com/theY4Kman/pytest-only/issues/11#issuecomment-1986509248, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAIIMGMM2CG3FQGCCU5SFTYXI3A3AVCNFSM6AAAAABCY5X7QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGUYDSMRUHA . You are receiving this because you modified the open/close state.Message ID: @.***>

ilyakamens commented 8 months ago

Thank you kindly!

theY4Kman commented 8 months ago

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 commented 8 months ago

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

theY4Kman commented 8 months ago

My pleasure :)

jack-zins commented 5 months ago

Getting an error with chained assignments with the flake8 extension. Opened a PR with test cases and proposed fix. https://github.com/theY4Kman/pytest-only/pull/13