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

Warnings do not become errors with --strict #1381

Closed
allanlewis opened this issue Feb 12, 2016 · 7 comments
Closed

Warnings do not become errors with --strict #1381

allanlewis opened this issue Feb 12, 2016 · 7 comments

Comments

@allanlewis
Copy link

In Pytest 2.8.7, the documentation for strict reads:

$ py.test -h | grep strict
  --strict              run pytest in strict mode, warnings become errors.

However, this doesn't appear to be the case. Running this file through py.test raises a warning as TestFoo has a custom __init__:

class TestFoo(object):
    def __init__(self):
        pass

    def test_bar(self):
        pass


def test_baz():
    pass

This doesn't cause py.test to exit non-zero, which is fine:

$ py.test -rw -qq test_foo.py
.
======================================= pytest-warning summary ========================================
WC1 /tmp/tmp.2scCkMZkLV/test_foo.py cannot collect test class 'TestFoo' because it has a __init__ constructor
$ echo $?
0

However, running with --strict also exits zero, despite the documentation snippet I quoted above:

$ py.test -rw -qq --strict test_foo.py
.
======================================= pytest-warning summary ========================================
WC1 /tmp/tmp.2scCkMZkLV/test_foo.py cannot collect test class 'TestFoo' because it has a __init__ constructor
$ echo $?
0

Am I missing something here? Shouldn't this return non-zero?

@RonnyPfannschmidt
Copy link
Member

that is indeed a documentation misstake

the original warnings where warnings about unknown markers,
the warning concept was expanded without taking that into accout

there is need for some discussion on how to solve this propperly

@rsyring
Copy link

rsyring commented Aug 8, 2017

FWIW, I would like to see an option that would turn warnings as described by the OP into errors and cause tests to fail. Whether that is --strict or a new option doesn't matter to me much.

Thanks.

@nicoddemus
Copy link
Member

nicoddemus commented Aug 8, 2017

@rsyring -Werror does that already... if the documentation is not clear about that we should definitely improve that. 👍

@rsyring
Copy link

rsyring commented Aug 8, 2017

@nicoddemus running with -Werror does make tests fail that emit warnings. However, the warnings from pytest itself do not fail:

$ py.test -Werror kegdemo
WARNING - kegdemo - Insecure keyring backend detected, keyring substitution unavailable. Run this app's keyring command for more info.
F..
======================================== FAILURES =========================================
___________________________________ TestCLI.test_hello ____________________________________
Traceback (most recent call last):
  File "/home/rsyring/projects/keg-demo-repo/kegdemo/tests/test_cli.py", line 12, in test_hello
    assert 'Hello World from Keg Demo!\n' == result.output
AssertionError: assert 'Hello World from Keg Demo!\n' == 'WARNING - keg...m Keg Demo!\n'
  + WARNING - kegdemo - Insecure keyring backend detected, keyring substitution unavailable. Run this app's keyring command for more info.
    Hello World from Keg Demo!
==================================== warnings summary =====================================
projects/keg-demo-repo/kegdemo/tests/test_views.py::TestApp
  cannot collect test class 'TestApp' because it has a __init__ constructor

-- Docs: http://doc.pytest.org/en/latest/warnings.html
1 failed, 2 passed, 1 warnings in 0.02 seconds

@nicoddemus
Copy link
Member

@rsyring oh you are definitely right, sorry about that.

We plan to refactor the internal pytest warnings mechanism to use standard python warnings (#2452), so -Werror will work with them. 👍

@blueyed
Copy link
Contributor

blueyed commented Jun 11, 2018

I guess this is resolved by now?

@nicoddemus
Copy link
Member

Yeah I think the underlying issue is, thanks for the ping @blueyed

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