Integrate pytest warnings #2072

nicoddemus · 2016-11-21T10:54:13Z

This PR starts the work to integrate fschulze/pytest-warnings.

I managed to preserve history and authorship, and the integration itself was smooth.

It is working as can be seen here:

============================= test session starts =============================
platform win32 -- Python 3.5.0, pytest-3.1.0.dev0, py-1.4.31, pluggy-0.4.0
rootdir: C:\pytest, inifile: tox.ini
plugins: hypothesis-3.5.0
collected 10 items

test_warnings.py ..........

=========================== pytest-warning summary ============================
WW0 in .tmp\test_warnings.py:7 the following warning was recorded:
 test_warnings.py:11: PendingDeprecationWarning: Pending
  warnings.warn(PendingDeprecationWarning('Pending'))

WW0 in .tmp\test_warnings.py:7 the following warning was recorded:
 test_warnings.py:14: DeprecationWarning: DeprecationWarning
  warnings.warn(DeprecationWarning('DeprecationWarning'))

================ 10 passed, 2 pytest-warnings in 0.02 seconds =================

Some points I would like to discuss before moving this further:

I think we should rename pytest-warnings back to just warnings, since it now it displays system warnings as well as pytest's own warnings.
The output is 3 lines per warning, I kind like that other summaries occupies only one line per summary.
Minor: test_warnings.py:14: should include the name of the function as well IMHO (test_warnings.py:test_foo:14:).
Surprisingly, pytest's own test suite is generating tons of warnings, you can see the full list here. I think we should fix those and then enable the -W error switch so future warnings break immediately.
The large number of warnings generated in such a relatively small project makes me wonder if we should leave the option to "ignore" by default. It will probably scare users the first time they execute 3.1.0 because I'm sure it will spew a ton of warnings. Then users can enable this gradatively and select which warnings they want to handle.
A small issue, but when we execute tests using testdir which raise warnings, the warnings are also displayed in the "main" session. I made a quick experiment to fix this by deleting the warnings after they were printed by the terminal reporter, but that didn't work. I will have to investigate more.

Would love to hear your feedback.

This is useful when to use regular expressions, like for example ignore a bunch of dynamic messages --filterwarnigns 'ignore:Please use assert.* instead.:' Or ignore all the warnings in a sub-backage --filterwarnigns 'ignore:::package.submodule.*' This is also available in the ini file as the filterwarnigns options

Add a warning option which does not escape its arguments.

…erage reporting.

nicoddemus · 2016-11-21T10:56:00Z

I see we only capture warnings around pytest_runtest_call. I think we should also capture for setup and teardown as well.

nicoddemus · 2016-11-21T11:27:27Z

Also we might want to include a mechanism where the user opts to only see warnings from their own projects?

…t-warnings

nicoddemus · 2016-11-22T19:07:18Z

Btw there are failing tests because now there are more "pytest-warnings" being displayed, for various reasons. Would like to nail down the discussion before fixing those.

nicoddemus · 2016-12-08T21:51:05Z

Gentle pinging @fschulze, @RonnyPfannschmidt, @hpk42, @The-Compiler and @flub

Would like to get the discussion going. 😁

fschulze · 2016-12-09T09:48:56Z

No one does anything about stuff if it's ignored, so I would vote against ignoring by default. It's better to add info how to hide warnings. Especially selectively like for modules you don't care about or specific types of warnings. A pointer to docs when there are more then 5 warnings perhaps.

The error code can go (discussed during the sprint).
Including the function name would indeed be nice.
Maybe we could use output that can be c'n'p into the -W option? Haven't thought that through.
I'm not sure we can do a one line output. We print which test caused the warning and we print where the warning originated in the code base. Both are very useful.

I agree that we should capture setup/teardown as well.

The original plan was to also replace the calls to .warn in the pytest code base with the python warning system. Not sure if that is feasible or not.

Currently warnings aren't transferred when using xdist, that's a major blocker IMO and I've got no clue what is involved to fix that.

nicoddemus · 2016-12-26T22:53:55Z

Thanks @fschulze, I will evolve the implementation a bit based on your feedback!

nicoddemus · 2017-02-18T15:05:09Z

FIxed the issue with testdir by using warnings.catch_warnings instead of WarningsRecorder during the hook call.

WarningsRecorder calls the original warnings.showwarnings function, which in its original form
only writes the formatted warning to sys.stdout, while warnings.catch_warnings doesn't.

Calling the original warnings.showwarnings has the effect that nested WarningsRecorder all catch the warnings:

with WarningsRecorder() as rec1:
    with WarningsRecorder() as rec2:
        warnings.warn(UserWarning, 'some warning')

(both rec1 and rec2 sees the warning)

When running tests with testdir, the main pytest session would then see the warnings created by
the internal code being tested (if any), and the main pytest session would end up with warnings as well.

I'm considering removing WarningsRecorder in favor of catch_warnings, but will do so in a separate PR.

This has the benefical side-effect of not calling the original warnings.showwarnings function, which in its original form only writes the formatted warning to sys.stdout. Calling the original warnings.showwarnings has the effect that nested WarningsRecorder all catch the warnings: with WarningsRecorder() as rec1: with WarningsRecorder() as rec2: warnings.warn(UserWarning, 'some warning') (both rec1 and rec2 sees the warning) When running tests with `testdir`, the main pytest session would then see the warnings created by the internal code being tested (if any), and the main pytest session would end up with warnings as well.

The rationale of using node ids is that users can copy/paste it to run a chosen test

nicoddemus · 2017-03-21T02:37:59Z

I think this is now ready for review! 😅

coveralls · 2017-03-21T02:56:54Z

Coverage decreased (-0.08%) to 92.585% when pulling 916d272 on nicoddemus:integrate-pytest-warnings into de8607d on pytest-dev:features.

The-Compiler · 2017-03-21T05:21:55Z

The CI seems to disagree 😉

The-Compiler · 2017-03-21T05:26:46Z

tox.ini

+                # produced by path.local
+                ignore:bad escape.*:DeprecationWarning:re
+                # produced by path.readlines
+                ignore:.*U.*mode is deprecated:DeprecationWarning


Those should probably be fixed in pylib

pytest-dev/py#116 is there to fix this

- pytester was creating a 'pexpect' directory to serve as temporary dir, but due to the fact that xdist adds the current directory to sys.path, that directory was being considered as candidate for import as a package. The directory is empty and a warning was being raised about it missing __init__ file, which is now turned into an error by our filterwarnings config in pytest.ini. - Decided to play it safe and ignore any warnings during `pytest.importorskip`. - pytest-xdist and execnet raise two warnings which should be fixed upstream: pytest-dev/pytest-xdist/issues/133

For some reason pypy raises this warning in the line that the catch_warnings block was added: ______________________________ ERROR collecting ______________________________ C:\ProgramData\chocolatey\lib\python.pypy\tools\pypy2-v5.4.1-win32\lib-python\2.7\pkgutil.py:476: in find_loader loader = importer.find_module(fullname) c:\pytest\.tox\pypy\site-packages\_pytest\assertion\rewrite.py:75: in find_module fd, fn, desc = imp.find_module(lastname, path) <builtin>/?:3: in anonymous ??? E ImportWarning: Not importing directory 'c:\users\bruno\appdata\local\temp\pytest-of-Bruno\pytest-3192\testdir\test_cmdline_python_package0' missing __init__.py

coveralls · 2017-03-22T01:54:04Z

Coverage decreased (-0.07%) to 92.587% when pulling 74b54ac on nicoddemus:integrate-pytest-warnings into de8607d on pytest-dev:features.

nicoddemus · 2017-03-22T01:54:24Z

Summary of the problems that lead to CI failing in some environments:

pytester was creating a pexpect directory to serve as temporary dir, but due to the fact that
xdist adds the current directory to sys.path, that directory was being considered as candidate
for import as a package. The directory is empty and a warning was being raised about
it missing __init__ file, which is now turned into an error by our filterwarnings config
in pytest.ini.
Because of this problem, decided to play it safe and ignore any warnings during pytest.importorskip.
pytest-xdist and execnet raise two warnings which should be fixed upstream: Warnings generated by pytest-xdist pytest-xdist#133
For some reason pypy raises an ImportWarning in one of the tests, but not in CPython. Decided to ignore that warning in that particular test.

Turning warnings into errors is fun. 😎

RonnyPfannschmidt · 2017-03-22T09:18:17Z

_pytest/pytester.py

@@ -1008,7 +1008,7 @@ def spawn_pytest(self, string, expect_timeout=10.0):
        The pexpect child is returned.

        """
-        basetemp = self.tmpdir.mkdir("pexpect")
+        basetemp = self.tmpdir.mkdir("temp-pexpect")


that must have been so puzzleing

RonnyPfannschmidt · 2017-03-22T09:24:32Z

looks good to me, unless someone notices something i missed, i'll merge this afternoon,

great dob 👍

RonnyPfannschmidt · 2017-03-22T15:42:39Z

@nicoddemus that bit again ^^ , thanks, i'll merge after its green

nicoddemus · 2017-03-22T15:43:37Z

Heh just noticed it this morning, but was busy so only now managed to get to it. 😁

coveralls · 2017-03-22T16:03:12Z

Coverage decreased (-0.07%) to 92.587% when pulling 0c1c258 on nicoddemus:integrate-pytest-warnings into de8607d on pytest-dev:features.

QuLogic · 2017-03-23T06:41:27Z

Will this work correctly with xdist? cf. fschulze/pytest-warnings#3 and pytest-dev/pytest-xdist#92.

fschulze and others added 13 commits June 22, 2016 12:21

Initial commit.

6b135c8

Add MANIFEST.in.

b9c4ecf

Prepare 0.1 release.

3feee0c

Fix version in readme.

bc5a8c7

Merge pull request #2 from Carreau/filter-regex

28621b0

Add a warning option which does not escape its arguments.

Bump version, add changelog entry and move stuff around for added cov…

f229b57

…erage reporting.

Prepare pytest-warnings 0.2.0.

ce13806

Bump.

6ec0c3f

Moving all stuff to a subdirectory to try to retain history

e31421a

Merge c:\pytest-warnings\ into integrate-pytest-warnings

9db32ae

Rename code to _pytest.warnings and delete old files from the repository

1da1906

Add tests and integrated the original code into the core

26ca5a7

nicoddemus changed the base branch from master to features November 21, 2016 10:54

Merge remote-tracking branch 'upstream/features' into integrate-pytes…

bd343ef

…t-warnings

Merge branch 'features' into integrate-pytest-warnings

a7643a5

nicoddemus added 5 commits February 18, 2017 13:08

Change warning output

e24081b

Also capture warnings during setup/teardown

de09023

Rename internal option to disable_warnings

bddb922

Display node ids and the warnings generated by it

272afa9

The rationale of using node ids is that users can copy/paste it to run a chosen test

nicoddemus mentioned this pull request Mar 13, 2017

Refactor recwarn to use warnings.catch_warnings instead of custom code #2303

Merged

Fixed tests

337f891

nicoddemus added 4 commits March 20, 2017 20:06

Add __future__ imports to warnings module

3373e02

Avoid displaying the same warning multiple times for an item

d027f76

Clean up warnings generated by pytest's own suite

fa56114

Add docs for the warnings functionality

eabe3ee

nicoddemus changed the title ~~Integrate pytest warnings [WIP]~~ Integrate pytest warnings Mar 21, 2017

Fix test on linux

916d272

The-Compiler reviewed Mar 21, 2017

View reviewed changes

nicoddemus mentioned this pull request Mar 22, 2017

Warnings generated by pytest-xdist pytest-dev/pytest-xdist#133

Open

nicoddemus added 2 commits March 21, 2017 22:17

RonnyPfannschmidt reviewed Mar 22, 2017

View reviewed changes

RonnyPfannschmidt approved these changes Mar 22, 2017

View reviewed changes

Add CHANGELOG entry

0c1c258

RonnyPfannschmidt merged commit 2a130da into pytest-dev:features Mar 22, 2017

QuLogic mentioned this pull request Mar 23, 2017

Use some more pytest plugins: warnings & rerunfailures matplotlib/matplotlib#8346

Merged

nicoddemus deleted the integrate-pytest-warnings branch March 27, 2017 16:52

This was referenced Mar 28, 2017

re-enable all warnings by default #1191

Closed

Integrate pytest-warnings plugin into core #1803

Closed

Replace internal warning system to standard warnings #1804

Closed

blueyed mentioned this pull request Jul 21, 2017

doc: filterwarnings is only documented in README #2603

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate pytest warnings #2072

Integrate pytest warnings #2072

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 22, 2016

nicoddemus commented Dec 8, 2016

fschulze commented Dec 9, 2016

nicoddemus commented Dec 26, 2016

nicoddemus commented Feb 18, 2017

nicoddemus commented Mar 21, 2017

coveralls commented Mar 21, 2017

The-Compiler commented Mar 21, 2017

The-Compiler Mar 21, 2017

RonnyPfannschmidt Mar 21, 2017

coveralls commented Mar 22, 2017

nicoddemus commented Mar 22, 2017 •

edited

Loading

RonnyPfannschmidt Mar 22, 2017

RonnyPfannschmidt commented Mar 22, 2017

RonnyPfannschmidt commented Mar 22, 2017

nicoddemus commented Mar 22, 2017

coveralls commented Mar 22, 2017

QuLogic commented Mar 23, 2017

Integrate pytest warnings #2072

Integrate pytest warnings #2072

Conversation

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 21, 2016

nicoddemus commented Nov 22, 2016

nicoddemus commented Dec 8, 2016

fschulze commented Dec 9, 2016

nicoddemus commented Dec 26, 2016

nicoddemus commented Feb 18, 2017

nicoddemus commented Mar 21, 2017

coveralls commented Mar 21, 2017

The-Compiler commented Mar 21, 2017

The-Compiler Mar 21, 2017

Choose a reason for hiding this comment

RonnyPfannschmidt Mar 21, 2017

Choose a reason for hiding this comment

coveralls commented Mar 22, 2017

nicoddemus commented Mar 22, 2017 • edited Loading

RonnyPfannschmidt Mar 22, 2017

Choose a reason for hiding this comment

RonnyPfannschmidt commented Mar 22, 2017

RonnyPfannschmidt commented Mar 22, 2017

nicoddemus commented Mar 22, 2017

coveralls commented Mar 22, 2017

QuLogic commented Mar 23, 2017

nicoddemus commented Mar 22, 2017 •

edited

Loading