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

ignore options are not respected #8

Closed
ssbarnea opened this issue Jul 23, 2016 · 15 comments
Closed

ignore options are not respected #8

ssbarnea opened this issue Jul 23, 2016 · 15 comments

Comments

@ssbarnea
Copy link

Here is what happens when someone is trying to add some ignore options:

Editing setup.cfg and adding

[pytest]
flake8-ignore=D D102

And while running it we can observe that the ignore option was loaded due to the changed header but right below it we can also observe that it was clearly not working.

We even tried alternative separator (comma) and in this case it has the same (non-filtering) behaviour but it does not display the ignoring text in the header.

 python -m pytest --flake8
=============================================================================================== test session starts ===============================================================================================
platform darwin -- Python 3.5.1, pytest-2.9.2, py-1.4.31, pluggy-0.3.1 -- /usr/local/opt/python3/bin/python3.5
cachedir: .cache
rootdir: /Users/sorins/Documents/dev/os/wstools, inifile: setup.cfg
plugins: timeout-1.0.0, asyncio-0.3.0, cov-2.2.1, flake8-0.5, instafail-0.3.0, pep8-1.0.6, tornado-0.4.4, xdist-1.14
collected 4 items

tests/test_wsdl.py FAILED
tests/test_wsdl.py::WSDLToolsTestCase::test_all PASSED
tests/test_wstools_net.py FAILED
tests/test_wstools_net.py::test_runner PASSED

==================================================================================================== FAILURES =====================================================================================================
__________________________________________________________________________________________ FLAKE8-check(ignoring D,D102) __________________________________________________________________________________________
/Users/sorins/Documents/dev/os/wstools/tests/test_wsdl.py:48:1: D102 Missing docstring in public method

Just to increase the level of non-sense about the implementation we already have these ignores configured in the setup.cfg

[flake8]
ignore=D,E402

Flake8 is able to use these ignores but pytest-flake8 not only that is expecting other options (why?!) but based on the documentation it does expect them with space separators instead of commas.

I tried to add pytest-flake8 to several projects that I maintain just because they were already using pytest and consolidating the build logic did make sense. Still, it seems that it never works.

@tholo
Copy link
Owner

tholo commented Oct 7, 2016

I think this might have changed with flake8 3.x -- can you verify whether this is still an issue?

@stevenh
Copy link

stevenh commented Nov 24, 2016

I can confirm that with flake8 3.2.1 and pytest-flake8 0.8.1 flake8-ignore directives are not applied.

@stevenh
Copy link

stevenh commented Nov 24, 2016

I've just discovered the issue is with pytest. It seems that in recent versions it changed the way it loads config resulting in it not loading the config it used to do (setup.cfg) instead loading pytest.ini and hence the config for pytest-flake8 wasn't being set at all.

@mforbes
Copy link
Contributor

mforbes commented Dec 1, 2016

Is there a workaround? This is a killer when you rely on automated flake8 tests.

mforbes added a commit to mforbes/pytest-flake8 that referenced this issue Dec 12, 2016
Ignore options are properly loaded from ``setup.cfg`` but then overwritten.  Here we simply extend this list.
@mforbes
Copy link
Contributor

mforbes commented Dec 12, 2016

I have added a simple potential fix to this, but don't know the logic behind the new option managing, so this needs review.

@mjtorn
Copy link

mjtorn commented Dec 21, 2016

@mforbes I tested your alteration and at least it seems to work. Now it respects whatever I have in [flake8] which is nice. I'm not involved with this project, nor do I know if this is something you'd like to add into a pull request, but the README would be even better if it said that you can just reuse your regular flake8 settings.

@mforbes
Copy link
Contributor

mforbes commented Jan 5, 2017

@mjtorn I am looking into this a bit, but I need input from @tholo. I do not understand the purpose of the present options-parsing code which has several issues:

  1. According to the docs, configuring ignore options for flake8 requires commas to separate the commands. This flake8-ignore options however break if commas are used, thus one requires one of the following formats which are inconsistent:

    # contents of setup.cfg
    [tool:pytest]
    flake8-ignore = E201 E231
    
    [flake8]
    ignore = E201, E231

    or

    # contents of setup.cfg
    [tool:pytest]
    flake8-ignore = E201
                    E231
    
    [flake8]
    ignore = E201,
             E231

    (Note also that the configuration section has now been changed to [tool:pytest])

Is there a use-case where one needs to control these separately? If so, it would be good to at least use a consistent format. If not, I would suspect that it is best to let flake8 read its own configuration. From my reading of the source, ignore is the only option that was being clobbered -- all other should still work from the flake8 configuration section.

@mforbes
Copy link
Contributor

mforbes commented Jan 5, 2017

It looks like @andras-tim added some of these options. Perhaps he can explain why the additional options are needed instead of just relying on the [flake8] configuration section (though the original code is due toe @tholo)?

@ssbarnea
Copy link
Author

Due to this bug I had to remove pytest-flake8 usage from all the modules that I maintain. While I do like the idea of being able to include flake8 as part of pytest execution, I do also find it unreliable.

@mforbes is right, I see zero reasons why this module would need its own ignore flags when flake8 already has them. This only creates confusion and bugs. I should also mention that even with this bug I found impossible to replicate the flake8 ignore list to pytest-flake8 ignore list. Somehow it seems that is ignoring some of these ignores :(

@mjtorn
Copy link

mjtorn commented Jan 30, 2017

So is something happening with this?

@mjtorn
Copy link

mjtorn commented Aug 7, 2017

Funny thing, this bit me for the first time in a while again ;)

Dealt with it, but it would be nice to see a resolution.

Thanks!

@mjtorn
Copy link

mjtorn commented Oct 2, 2017

Nothing? Anything?

@andras-tim
Copy link
Contributor

It looks like @andras-tim added some of these options. Perhaps he can explain why the additional options are needed instead of just relying on the [flake8] configuration section (though the original code is due toe @tholo)?

@mforbes, I prefer a magic solution, like pass through all flake8-XXX=YYY options to --XXX=YYY arguments.

mikemill added a commit to mikemill/pytest-flake8 that referenced this issue Oct 2, 2017
@richard-reece
Copy link

I am also seeing the issue that an "ignore" setting in the [flake8] section of setup.cfg is ignored. PR #21 solves this for me.

@tholo tholo closed this as completed in 8293d0b Oct 23, 2017
@richard-reece
Copy link

Thank you!

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

7 participants