Add TestCase.flushWarnings

[exarkun]

TestCase.assertWarns is limited in that applications can only express a single warning using it. It is also a somewhat inconvenient API, even for the simplest warning tests, in that it requires filename and line number information which is somewhat confusing to developers.

TestCase.callDeprecated is similarly limited. Additionally, in trying to avoid the confusing API for assertWarns, it presents an interface which is very narrow. Instead of a warning type and message, it takes only a version number. Additionally, it only works with the deprecated decorator, not warnings in general.

This leaves trial without a general tool for making assertions about warnings. I suggest adding flushWarnings. Trial will collect warnings emitted during the execution of a test. flushWarnings will allow any number of warnings to be cleared from this collection mechanism and returned to the caller. Any unflushed warnings at the end of a test will be re-emitted so as to be visible to whatever is running trial. flushWarnings will support rudamentary filtering, similar to that offered by flushLoggedErrors.

This will work for the use case of multiple warnings. It removes the possibility for confusing intermediate stack levels to confuse the warning system or the developer. It gives all available information to the developer so that any assertion may be made. It preserves reporting of all unexpected or unhandled warnings. If the filtering supported is based on function objects, it removes the confusing filename/linenumber aspects present in the assertWarns API. Both assertWarns and callDeprecated can be implemented in terms of this mechanism, as can other higher-level test helpers.

See discussion on #3266 for more details (mostly historic).

[exarkun]

(In [25012]) Branching to 'flushwarnings-3487'

[exarkun]

(In [25013]) merge flushWarnings changes from #3266 branch

refs #3487 refs #3266

[exarkun]

Ready for review. The branch should also resolve #3433 and #3427.

[z3p]

And go towards fixing #2710.

[glyph]

Reviewing.

[glyph]

Uh, put the review keyword back.

[glyph]

• warnings no longer show up in the test output. For example:

  #test_warnings.py
  from twisted.trial.unittest import TestCase
  import warnings
  class WarningWarner(TestCase):
      def test_warning(self):
          warnings.warn("warning!", category=UserWarning)
  

  results in:

  $ trial test_warnings.py
  test_warnings
    WarningWarner
      test_warning ...                                                       [OK]
  
  -------------------------------------------------------------------------------
  Ran 1 tests in 0.002s
  
  PASSED (successes=1)
  

• With your branch merged:

  glyph@nhuvasarim:~$ python -Werror /home/glyph/Projects/Twisted/trunk/bin/trial test_warnings.py 
  test_warnings
    WarningWarner
      test_warning ...                                                       [OK]
  Traceback (most recent call last):
    File "/home/glyph/Projects/Twisted/trunk/bin/trial", line 24, in <module>
      run()
    File "/home/glyph/Projects/Twisted/trunk/twisted/scripts/trial.py", line 361, in run
      test_result = trialRunner.run(suite)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 816, in run
      return self._runWithoutDecoration(test)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 842, in _runWithoutDecoration
      run()
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 837, in <lambda>
      run = lambda: suite.run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 291, in run
      TestSuite.run(self, result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1443, in run
      test(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1431, in __call__
      return self.run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 164, in run
      super(LoggedSuite, self).run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1443, in run
      test(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1431, in __call__
      return self.run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 137, in run
      test(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1431, in __call__
      return self.run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 137, in run
      test(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1431, in __call__
      return self.run(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/runner.py", line 137, in run
      test(result)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 740, in __call__
      return self.run(*args, **kwargs)
    File "/home/glyph/Projects/Twisted/trunk/twisted/trial/unittest.py", line 1164, in run
      warnings.warn_explicit(**w)
    File "/usr/lib/python2.5/warnings.py", line 102, in warn_explicit
      raise message
  UserWarning: warning!
  

• On the less serious side, think _Warning could use some documentation explaining what it is, not just what its attributes are.
• Sadly, the branch otherwise looks great. After staring at it for a while I'm not really sure what the problems are, so I'm going to give it back to you for fixing rather than continue trying to provide more useful feedback.

[exarkun]

(In [25135]) a bit more docs for _Warning

refs #3487

[exarkun]

(In [25136]) better docs for _Warning

refs #3487

[exarkun]

warnings no longer show up in the test output. For example:

Warnings show up now. They're not printed to stdout, as they used to be, but written to the reporter's output stream by the reporter which also implements a simple form of duplicate removal.

With your branch merged:

If configuration dictates that a warning is actually an exception, an error is now added to the result for the test which didn't flush that warning. ie, Trial doesn't crash if Python is invoked with -Werror anymore.

On the less serious side, think _Warning could use some documentation explaining what it is, not just what its attributes are.

I improved the docs a bit.

Sadly, the branch otherwise looks great. After staring at it for a while I'm not really sure what the problems are, so I'm going to give it back to you for fixing rather than continue trying to provide more useful feedback.

I added 3 hidden bugs for the next reviewer, to keep it interesting. ;)

[exarkun]

#3433 was a duplicate of #2820, by the way. I think the branch will also resolve #3506.

[glyph]

Reviewing.

[glyph]

1. It's a bit weird that when warnings are configured as errors, a warning emitted at module scope will raise an immediate exception, but a warning emitted in the course of a test will report a test failure but not actually raise an exception. This would seem to strongly discourage the emission of warnings at module scope. Maybe that's a good thing. I don't have any specific feedback to this point, and I think the current strategy is acceptable. However, it should probably be accepted.
2. Reporter._publisher is undocumented.
3. The declaration of Reporter.__init__ is greater than 80 characters long.
4. Reporter._observeWarnings should have a @param declaration describing event. It might be helpful to also describe it specifically as an argument to log.addObserver.
5. Some unused imports:

   twisted/test/test_log.py:11: 'util' imported but unused
   twisted/test/test_log.py:13: 'reflect' imported but unused
   twisted/trial/test/test_log.py:10: 'defer' imported but unused
   twisted/trial/test/test_log.py:11: 'reflect' imported but unused
   twisted/trial/test/test_log.py:12: 'runner' imported but unused
   

Otherwise this is looking good; no functional issues that I can see, and I did a bunch of interactive testing this time. When you've cleaned up these minor issues, you can go ahead and merge.

I think you should probably file a ticket for documenting the behavior of warnings-as-errors though; although this is probably part of a larger task of documenting trial in general, since its documentation is pretty pathetic.

[exarkun]

(In [25249]) Address most review feedback

refs #3487

[exarkun]

1. ... However, it should probably be accepted.

I'm not sure what this means.

I think you should probably file a ticket for documenting the behavior of warnings-as-errors though;

Filed #3520.

[exarkun]

(In [25250]) fix epytext markup

refs #3487

[exarkun]

(In [25251]) Fix test_missingSource on Python 2.4 when PIL is installed

Due to bugs in linecache, having the missing source file be named __init__.py is incredibly unlikely to work. Hack around this in the test by not testing that case. If inspect.getsource actually works right, this functionality will work right. If it doesn't, then it won't.

refs #3487

[exarkun]

In addition to the above fixes, I also needed to add path case normalization so that filtering worked properly on Windows.

Here's the full diff since the last review: http://twistedmatrix.com/trac/changeset?new=branches%2Fflushwarnings-3487%4025262&old=branches%2Fflushwarnings-3487%4025157

[exarkun]

Oh, and here are recent build results:

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/flushwarnings-3487&num_builds=2

[glyph]

Replying to exarkun:

1. ... However, it should probably be accepted.

I'm not sure what this means.

Sorry, that was a left-over sentence from a different edit of phrasing the other stuff. The point was, the code is OK as it is, but it should be documented.

[glyph]

Thanks very much for the build results and inter-review patch.

No further problems I can see, although I wish I were more confident in my assessment. Merge away, and hopefully I won't look stupid as a result :).

[exarkun]

(In [25279]) Merge flushwarnings-3487

Author: radix, itamar, exarkun Reviewer: glyph Fixes: #3487 Fixes: #3427 Fixes: #2820 Fixes: #3506

Introduce TestCase.flushWarnings, an API for interacting with the Python warnings module in unit tests. By default, the default trial reporter will write warnings emitted by tests to its output stream (no longer does the runner print them to stdout). flushWarnings may be used to prevent this from happening in particular tests, and its return value may be used to make assertions about what warnings have been emitted. TestCase.assertWarns and TestCase.callDeprecated are both now implemented in terms of this more flexible API.

The default reporter includes rudamentary support for duplicate suppression: each warning will be reported only once per test, but if a warning is emitted by two or more tests, it will be reported for each test (isolating tests from each other further by making warning behavior the same regardless of what other tests have run).

There is also some support for failing tests based on warnings emitted. If the Python warnings filters system is used to turn a warning into an exception, any test which causes such a warning to be emitted will have an error attributed to it.