unittest TestResult wasSuccessful returns True when there are unexpected successes

[gpshead]

BPO	20165
Nosy	@gpshead, @voidspace, @serhiy-storchaka
Files	issue20165-gps01.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2014-01-20.09:13:00.112>
created_at = <Date 2014-01-07.19:25:21.692>
labels = ['type-bug']
title = 'unittest TestResult wasSuccessful returns True when there are unexpected successes'
updated_at = <Date 2021-12-16.11:56:09.259>
user = 'https://github.com/gpshead'

bugs.python.org fields:

activity = <Date 2021-12-16.11:56:09.259>
actor = 'serhiy.storchaka'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2014-01-20.09:13:00.112>
closer = 'gregory.p.smith'
components = []
creation = <Date 2014-01-07.19:25:21.692>
creator = 'gregory.p.smith'
dependencies = []
files = ['33363']
hgrepos = []
issue_num = 20165
keywords = ['patch']
message_count = 8.0
messages = ['207582', '207583', '207614', '207629', '207677', '207734', '208530', '408695']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'michael.foord', 'python-dev', 'serhiy.storchaka', 'slattarini']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue20165'
versions = ['Python 3.4']

[gpshead]

Python 3.3.3+ (3.3:28337a8fb502+, Jan  7 2014, 01:32:44)
[GCC 4.6.3] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import unittest
>>> r = unittest.result.TestResult()
>>> r.wasSuccessful()
True
>>> r.addUnexpectedSuccess("weird!")
>>> r.wasSuccessful()
True

An unexpected success is not a good thing and indicates a problem.

the wasSuccessful() method should include a check for len(self.unexpectedSuccesses) == 0 as part of its condition.

[gpshead]

somewhat related - https://code.google.com/p/unittest-ext/issues/detail?id=22

[voidspace]

I'm pretty sure this has been debated before (and the status quo is the result). Trying to find the issue.

[voidspace]

Hmmm... TestTools has wasSuccessful return False on an unexpected success [1] and I can't find an issue for any previous discussion.

I don't use unexpected success, so I have no particular horse in this race but it seems more logical that wasSuccessful should return False when there are unexpected successes.

[1] https://code.launchpad.net/~jml/testtools/unexpected-success-2/+merge/42050

[gpshead]

I'm not comfortable changing this for 2.7 or 3.3 in case some code is unfortunately is depending on this behavior. But as it is it does seem like the kind of thing that can hide problems (tests that are passing that are not expected to).

Here's a patch for 3.4 (minus the documentation update that'll need to explicitly mention this with a versionchanged::).

[slattarini]

Since I too was bitten by this issue, I'd like to support Gregory's
request, and report my rationale for changing the current behaviour.

With the current behaviour, we could see (and I *have* seen) scenarios
like this:

1. A test exposing a known bug is written, and the test is marked
   as "expected failure".

2. Some refactoring and code improvements follow.

3. They improve the overall behaviour/correctness of the program
   or library under test, to the point of fixing the bug "behind
   the scenes".

4. The developer doesn't notice the fix though, since the testing
   framework doesn't warn him "strongly enough" of the fact that the
   test marked as expected failure has begun to pass. (To reiterate,
   the current behaviour of just printing a warning saying "some test
   passed unexpectedly" on the standard output is not good enough of
   a warning; it's easy to miss, and worse, it's *certain* that it
   will be missed if the tests are run by some CI systems or a similar
   wrapper system -- those would only report failures due to non-zero
   exit statuses.)

5. Without noticing that the issue has been fixed, the developer does
   some further changes, which again re-introduce the bug, or a
   similar one that the test still marked as "expected failure" could
   easily catch.

6. That test starts to fail again; but since it has remained marked
   as "expected failure" all along, the fact is not reported to the
   developer in any way. So the bug has managed to sneak back in,
   unnoticed.

In addition to this rationale, another (weaker) reason to change the
existing behaviour would be the "principle of least surprise". Among
other widely used testing framework (for python, or language-agnostic)
many of those which support the concept of "expected failure" will
throw hard errors by default when a test marked as expected failure
starts to pass; among these are:

• Python libs:

  • py.test (http://pytest.org)
  • Nose (https://pypi.python.org/pypi/nose/1.3.0)

• Language-agnostic protocols/frameworks:

  • the TAP protocol (the standard in the Perl world)
    (http://en.wikipedia.org/wiki/Test_Anything_Protocol)
  • the Automake "Simple Tests" framework
    (http://www.gnu.org/software/automake/manual/html_node/Tests.html)

[python-dev]

New changeset 1e75ab9fd760 by Gregory P. Smith in branch 'default':
Fixes Issue bpo-20165: The unittest module no longer considers tests marked with
http://hg.python.org/cpython/rev/1e75ab9fd760

[serhiy-storchaka]

See also bpo-22815.