Warns #897

hunse · 2015-07-28T23:08:58Z

The recwarn function argument is useful, but sometimes I just want to make sure that a warning of a particular type is thrown, and this takes a number of lines with recwarn. I added a warns fixture to ensure that warnings are thrown. Similar to raises, but for warnings.

nicoddemus · 2015-07-28T23:14:32Z

CHANGELOG

@@ -120,6 +120,9 @@
 2.7.3 (compared to 2.7.2)
 -----------------------------

+- Add 'warns' to assert that warnings are thrown (like 'raises').


I think this should be in the changelog for 2.8.

Oh yeah, sorry, I rebased and forgot to move it.

nicoddemus · 2015-07-28T23:16:51Z

testing/python/warns.py

+                          (RuntimeWarning, SyntaxWarning),
+                          lambda: warnings.warn('w3', UserWarning)))
+
+    @pytest.mark.skipif('sys.version < "2.5"')


This is not necessary as pytest supports 2.6+ only. 😄

Oh, sweet! Again, just copying from raises.py.

nicoddemus · 2015-07-28T23:18:39Z

Thanks for the PR @hunse! 😄

nicoddemus · 2015-07-28T23:39:07Z

Hmmm could you write some docs for pytest.warns? I think you might add it to assert.txt... make sure to add a .. versionadded:: 2.8 directive as well. 😄

hunse · 2015-07-29T15:56:40Z

Okay, doc is added.

nicoddemus · 2015-07-29T16:01:10Z

doc/en/assert.txt

+        warnings.warn("another warning", RuntimeWarning)
+
+    assert len(record) == 1  # check that only one warning was raised
+    assert record[0].message.message == "another warning"


message.message? Is this a typo?

No, that's actually the way you have to do it, because of how warnings.WarningMessage is set up. WarningMessage.message returns the actual exception, not the message, so you have to call message again. Though your question made me realize that in Python3, warnings no longer have a message attribute, just an args one, so I've changed things to work in Python3 as well.

nicoddemus · 2015-07-29T16:03:07Z

Nice @hunse, much appreciated!

If nobody has any more comments, I will merge this tomorrow. 😄

hunse · 2015-07-29T18:27:05Z

Okay, changed the docs around and added the link to recwarn. The only thing I'm worried about now is that the recwarn page says it's about "Asserting deprecation and other warnings", which makes it sound like it's the way to do that, and it's always confusing when there are two ways to do things. Should the recwarn page be rewritten so it's more about recording warnings? I'm not quite sure how I would word it, though.

nicoddemus · 2015-07-29T21:31:05Z

Hmm good point. I see two options:

Add a link back to pytest.warns in recwarn, explaining its intended use for simple cases;
Move pytest.warns docs to recwarn and keep it all in the same place;

After your points I'm thinking the latter. What do you think?

hunse · 2015-07-29T21:34:53Z

Yeah, I'm thinking the same thing. I was also wondering if it would be good to have the same interface for warnings recorded by recwarn and those recorded by pytest.warns.

hunse · 2015-07-29T21:38:18Z

Though it might be nice to have something after pytest.raises that at least points to the recwarn document. I think a lot of people might read through pytest.raises and then think "but how do I do this for warnings?"

nicoddemus · 2015-07-29T21:59:50Z

I was also wondering if it would be good to have the same interface for warnings recorded by recwarn and those recorded by pytest.warns.

I think so, but don't know how difficult that would be to implement.

I think a lot of people might read through pytest.raises and then think "but how do I do this for warnings?"

Agreed. How about a one liner after pytest.raises docs like: "To capture warnings, take a look at <link to recwarn>"?

hunse · 2015-07-30T17:38:27Z

Okay, warns and recwarn use the same code, now. It seems a lot cleaner. @nicoddemus, let me know what you think, and I'll update the documentation.

nicoddemus · 2015-07-30T17:51:27Z

_pytest/recwarn.py

-        warnings.warn_explicit = oldwarn_explicit
-        warnings.warn = oldwarn
-    if not l:
-        __tracebackhide__ = True


Keep __tracebackhide__ in the body of the function, this way when it fails the traceback will point to the point of the call, not to the internal details of this function.

nicoddemus · 2015-07-30T18:04:25Z

@hunse excellent work, thanks for the nice refactoring! 😄

nicoddemus · 2015-07-30T18:09:15Z

About the docs, I think it makes sense to add pytest.warns together with other documentation related to warnings in recwarn.txt, what do you think?

hunse · 2015-07-30T18:35:07Z

Agreed.

nicoddemus · 2015-07-30T19:02:51Z

_pytest/recwarn.py

-        warnings.simplefilter('default')
-        def reset_filters():
-            warnings.filters[:] = oldfilters
-        request.addfinalizer(reset_filters)
    wrec = WarningsRecorder()


Minor: this fits nicely as a yield_fixture:

@pytest.yield_fixture def recwarn(): wrec = WarningsRecorder() with wrec: warnings.simplefilter('default') yield wrec

(should've seen it before, sorry! 😅)

Oh wow, that's cool. I didn't even know that was possible!

hunse · 2015-07-30T21:20:52Z

_pytest/recwarn.py

+        return iter(self._list)
+
+    def __len__(self):
+        return len(self._list)


@nicoddemus: how would I add documentation for these kinds of methods in recwarn.txt? There seems like there should be a clearer way than just adding them as .. method::.

You can use auto directives from sphinx... you can see them in action here.

Works in a similar manner to `raises`, but for warnings instead of exceptions. Also refactored `recwarn.py` so that all the warning recording and checking use the same core code.

nicoddemus · 2015-08-03T23:26:47Z

@hunse could you fix the flakes error? After this I think we can merge this. 😄

hunse · 2015-08-04T01:27:04Z

I have no idea why it's getting that error. The Travis build says it ran on bd3877b, which is not the most recent update. Can you force it to rerun? There's no reason that it should not be able to import WarningsChecker from raises.py; in fact, the file that it's complaining about runs fine, i.e. it can import WarningsChecker. It doesn't make any sense because that file must have run as part of the test suite, which passed, so I don't know why flakes is finding problems.

nicoddemus · 2015-08-04T01:40:24Z

Oh I didn't even saw the error when I commented... it seems like travis misinterpreted some rebase done in the branch. I tried re-submitting the job, but it insists on running against commit bd3877b...

Anyway, I just tested it and locally and everything passes, so we can ignore that error. 😄

Warns

nicoddemus · 2015-08-04T01:48:39Z

Thanks a lot for the work @hunse!

Cheers,
🍻

The-Compiler · 2015-08-04T04:18:00Z

Hm - it seems the same error shows up on Travis now that it's merged to master.

nicoddemus · 2015-08-04T11:14:09Z

Oh dear... I will take a look at this later then. No idea why flakes is acting up (if anybody wants to take a shot a it, please be my guest);

hunse · 2015-08-04T16:27:17Z

The other thing that is very confusing (and makes it very annoying to debug) is that this error doesn't seem to appear when running flakes locally. At least I didn't have it appear on my machine, and @nicoddemus didn't see it either. Maybe this is a bug in flakes that was fixed, and Travis just needs an update?

The-Compiler · 2015-08-04T17:08:33Z

Travis uses tox for the tests, so it should use a recent pyflakes version.

Maybe try running git clean -xdf to clean up any cache files etc.? (Make sure there's nothing important untracked in git status first)

The-Compiler · 2015-08-04T17:13:37Z

I can actually reproduce this when doing -i ALL=https://devpi.net/hpk/dev/ - in my experience, devpi sometimes fails to pick up new upstream versions, so I'm guessing that's what's happening...

@hpk42 - can you force an update? Can we somehow make tox only use devpi for the needed packages (pylib I guess?) and use PyPI directly for everything else?

…lakes issue See pytest-dev#897 for discussion

nicoddemus · 2015-08-04T23:31:45Z

As a workaround, I moved import WarningsChecker to a local import and then flakes passed, but we should update flakes on devpi when we can.

hpk42 · 2015-09-28T15:50:33Z

Seems like this PR broke issue #1037

`deprecated_call` now looks for PendingDeprecationWarnings, as it did previously but was broken by pytest-dev#897. Fixes pytest-dev#1037. Also added a test so this does not happen again.

`deprecated_call` used to accept any warning. As of pytest-dev#897, it is now specific to DeprecationWarnings, and another commit in this PR extends this to PendingDeprecationWarnings. This commit makes sure this stays the case.

nicoddemus reviewed Jul 28, 2015
View reviewed changes

hunse force-pushed the warns branch from a209af8 to e1da327 Compare July 28, 2015 23:16

nicoddemus reviewed Jul 28, 2015
View reviewed changes

hunse force-pushed the warns branch from af7818e to 2457518 Compare July 28, 2015 23:30

hunse force-pushed the warns branch 2 times, most recently from ed55cb0 to 684139b Compare July 29, 2015 15:56

nicoddemus reviewed Jul 29, 2015
View reviewed changes

hunse force-pushed the warns branch 2 times, most recently from 3e6a12c to 8dc6102 Compare July 29, 2015 16:26

hunse force-pushed the warns branch from 8dc6102 to 4c60780 Compare July 29, 2015 21:24

nicoddemus reviewed Jul 30, 2015
View reviewed changes

hunse reviewed Jul 30, 2015
View reviewed changes

hunse added 2 commits July 30, 2015 23:28

Added myself to AUTHORS

aac371c

Added warns to assert warnings are thrown

52b4eb6

Works in a similar manner to `raises`, but for warnings instead of exceptions. Also refactored `recwarn.py` so that all the warning recording and checking use the same core code.

hunse force-pushed the warns branch from 509efb1 to 52b4eb6 Compare July 31, 2015 03:28

nicoddemus added a commit that referenced this pull request Aug 4, 2015

Merge pull request #897 from hunse/warns

d761bff

Warns

nicoddemus merged commit d761bff into pytest-dev:master Aug 4, 2015

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Aug 4, 2015

Move WarningsChecker import in test to a local import to workaround f…

02e742b

…lakes issue See pytest-dev#897 for discussion

nicoddemus mentioned this pull request Aug 4, 2015

Move WarningsChecker import in test to a local import to workaround flakes issue #914

Merged

hunse mentioned this pull request Aug 5, 2015

Always warn in unit tests in case they run twice. nengo/nengo#717

Merged

hunse mentioned this pull request Sep 28, 2015

deprecated_call detects pending warnings again #1081

Merged

nicoddemus mentioned this pull request Nov 24, 2015

Inconsistent behavior of pytest.deprecated_call() #1190

Closed

Warns #897

Warns #897

Conversation

hunse commented Jul 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus commented Jul 28, 2015

nicoddemus commented Jul 28, 2015

hunse commented Jul 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus commented Jul 29, 2015

hunse commented Jul 29, 2015

nicoddemus commented Jul 29, 2015

hunse commented Jul 29, 2015

hunse commented Jul 29, 2015

nicoddemus commented Jul 29, 2015

hunse commented Jul 30, 2015

Choose a reason for hiding this comment

nicoddemus commented Jul 30, 2015

nicoddemus commented Jul 30, 2015

hunse commented Jul 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus commented Aug 3, 2015

hunse commented Aug 4, 2015

nicoddemus commented Aug 4, 2015

nicoddemus commented Aug 4, 2015

The-Compiler commented Aug 4, 2015

nicoddemus commented Aug 4, 2015

hunse commented Aug 4, 2015

The-Compiler commented Aug 4, 2015

The-Compiler commented Aug 4, 2015

nicoddemus commented Aug 4, 2015

hpk42 commented Sep 28, 2015