unittest subTest failure causes result to be omitted from listing

[zware]

BPO	25894
Nosy	@pitrou, @rbtcollins, @ezio-melotti, @bitdancer, @voidspace, @ambv, @vadmium, @zware, @serhiy-storchaka
PRs	bpo-25894: Always report skipped and failed subtests separately #28082

^{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 = None
closed_at = <Date 2021-09-10.16:00:39.878>
created_at = <Date 2015-12-17.05:49:35.850>
labels = ['type-bug', 'library', '3.11']
title = 'unittest subTest failure causes result to be omitted from listing'
updated_at = <Date 2021-09-10.16:00:39.877>
user = 'https://github.com/zware'

bugs.python.org fields:

activity = <Date 2021-09-10.16:00:39.877>
actor = 'lukasz.langa'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-10.16:00:39.878>
closer = 'lukasz.langa'
components = ['Library (Lib)']
creation = <Date 2015-12-17.05:49:35.850>
creator = 'zach.ware'
dependencies = []
files = []
hgrepos = []
issue_num = 25894
keywords = ['patch']
message_count = 12.0
messages = ['256580', '256588', '256590', '256604', '256631', '257146', '261834', '400573', '400700', '401101', '401586', '401587']
nosy_count = 9.0
nosy_names = ['pitrou', 'rbcollins', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'lukasz.langa', 'martin.panter', 'zach.ware', 'serhiy.storchaka']
pr_nums = ['28082']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25894'
versions = ['Python 3.11']

[zware]

The title can barely be called accurate; the description of the problem isn't easy to condense to title length. Here's the issue:

$ cat subtest_test.py 
import os
import unittest

class TestClass(unittest.TestCase):

    def test_subTest(self):
        for t in map(int, os.environ.get('tests', '1')):
            with self.subTest(t):
                if t > 1:
                    raise unittest.SkipTest('skipped')
                self.assertTrue(t)

if __name__ == '__main__':
    unittest.main()
$ ./python.exe subtest_test.py 
.

---

Ran 1 test in 0.000s

OK
$ tests=01 ./python.exe subtest_test.py

======================================================================
FAIL: test_subTest (main.TestClass) (<subtest>)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "subtest_test.py", line 12, in test_subTest
    self.assertTrue(t)
AssertionError: 0 is not true

---

Ran 1 test in 0.001s

FAILED (failures=1)
$ tests=012 ./python.exe subtest_test.py
s
======================================================================
FAIL: test_subTest (main.TestClass) (<subtest>)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "subtest_test.py", line 12, in test_subTest
    self.assertTrue(t)
AssertionError: 0 is not true

---

Ran 1 test in 0.001s

FAILED (failures=1, skipped=1)

Note that on the first run, the short summary is ".", as expected. The second is "", when one of the subTests fails, but then the third is "s", when one subtest fails but another is skipped. This also extends to verbose mode:

$ ./python.exe subtest_test.py -v
test_subTest (__main__.TestClass) ... ok

---

Ran 1 test in 0.001s

OK
$ tests=01 ./python.exe subtest_test.py -v
test_subTest (main.TestClass) ...
======================================================================
FAIL: test_subTest (main.TestClass) (<subtest>)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "subtest_test.py", line 12, in test_subTest
    self.assertTrue(t)
AssertionError: 0 is not true

---

Ran 1 test in 0.001s

FAILED (failures=1)
$ tests=012 ./python.exe subtest_test.py -v
test_subTest (main.TestClass) ... skipped 'skipped'

======================================================================
FAIL: test_subTest (main.TestClass) (<subtest>)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "subtest_test.py", line 12, in test_subTest
    self.assertTrue(t)
AssertionError: 0 is not true

---

Ran 1 test in 0.001s

FAILED (failures=1, skipped=1)

Note the first run shows "... ok", the second "... ", and the third "... skipped 'skipped'"

I'm unsure what the solution should be. There should at least be some indication that the test finished, but should mixed results be reported as 'm' ("mixed results" in verbose mode), or should failure/error take precedence, or should every different result be represented?

[vadmium]

Okay, so you have a test with subtests. You have presented three cases:

1. Single subtest which passes. No problem I assume.

2. Two subtests: 1st fails, 2nd passes. This is how subtests are normally used, so I guess there is no problem. Is that right?

3. After two subtests have already run (one of which failed), SkipTest is raised. I guess you want the test results to be reported better in this case.

What is the use case? Why not skip the test before any subtests are started?

[zware]

Martin Panter added the comment:

Okay, so you have a test with subtests. You have presented three cases:

1. Single subtest which passes. No problem I assume.

Or several subtests which pass. No problems.

2. Two subtests: 1st fails, 2nd passes. This is how subtests are normally used, so I guess there is no problem. Is that right?

Any of multiple subtests fail, and there is no indication in the
"summary line" (the line that is usually "..........................",
a dot for each successful test). When a a regular test fails, an F
(or an E, if the raised exception was anything but
self.failureException) is added to the line; when any subtests fail,
nothing is added. If you have 10 tests methods that use subtests, and
any subtest in each method fails, your summary line will be blank. In
verbose mode, you'd get "test_one ... test_two ... test_three ... ..."
(note lack of newlines) instead of the expected "test_one ...
FAILURE\ntest_two ... FAILURE\ntest_three ... FAILURE\n..." (note the
newlines).

3. After two subtests have already run (one of which failed), SkipTest is raised. I guess you want the test results to be reported better in this case.

What is the use case? Why not skip the test before any subtests are started?

Only the subtest is skipped (which should be valid, or documented as
not valid), and the order of the subtests doesn't matter:

$ tests=210 ./python.exe subtest_test.py -v
test_subTest (__main__.TestClass) ... skipped 'skipped'

======================================================================
FAIL: test_subTest (main.TestClass) (<subtest>)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "subtest_test.py", line 14, in test_subTest
    self.assertTrue(t)
AssertionError: 0 is not true

---

Ran 1 test in 0.001s

FAILED (failures=1, skipped=1)

But, the summary makes it seem as though the entire test was skipped.

Hopefully this makes it a bit clearer :)

[bitdancer]

I believe this was discussed at the time subTest was added and deemed an acceptable tradeoff for a simpler implementation. I'm not sure it is, but I'm not prepared to write code to fix it :) I'm bothered every time I see this, but I have to admit that the tracebacks are the most important feedback and you do get those.

[vadmium]

Yes now I understand. If a subtest fails, there is no status update (not even a newline in verbose mode), and each subtest skip triggers a separate status update.

My gut feeling is that any subtest failure should be counted as the whole test failing. I’m not sure how the failure vs error cases should be handled. Maybe error should trump failure.

Judging by <https://bugs.python.org/issue16997#msg180259\>, Antoine intended for SkipTest to skip subtests. But I’m not sure that be reported as the whole test being skipped.

[ezio-melotti]

My gut feeling is that any subtest failure should be counted as the
whole test failing. I’m not sure how the failure vs error cases should
be handled. Maybe error should trump failure.

I think the priority should be error > failure > skip > pass.
IOW, pass should never be reported if any of the other 3 happen, skip should be reported only if there are no errors/failures, and errors should trump failures.

[rbtcollins]

The basic model is this:

• a test can have a single outcome [yes, the api is ambiguous, but there it is]
• subtests let you identify multiple variations of a single test (note the id tweak etc) and *may* be reported differently

We certainly must not report the test as a whole passing if any subtest did not pass.

Long term I want to remove the error/failure partitioning of exceptions; its not actually useful.

The summary for the test, when subtests are used, should probably enumerate the states.

test_foo (3 passed, 2 skipped, 1 failure)

in much the same way the run as a whole is enumerated.

[serhiy-storchaka]

Were not subtests proposed as a more flexible replacement of parametrized tests? I think that every subtest should be counted as a separate test case: in verbose mode it should output a separate line and in non-verbose mode it should output a separate character.

[serhiy-storchaka]

PR 28082 is a draft that implements this idea. Skipped and failed (but not successfully passed) subtests are now reported separately, as a character (sFE) or a line ("skipped", "FAIL", "ERROR"). The description of the subtest is included in the line. For example:

$ tests=.sFE ./python test_issue25894.py 
sFE

======================================================================
ERROR: test_subTest (main.TestClass) [3] (t='E')
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/test_issue25894.py", line 15, in test_subTest
    raise Exception('error')
    ^^^^^^^^^^^^^^^^^^^^^^^^
Exception: error

======================================================================
FAIL: test_subTest (main.TestClass) [2] (t='F')
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/test_issue25894.py", line 13, in test_subTest
    self.fail('failed')
    ^^^^^^^^^^^^^^^^^^^
AssertionError: failed

---

Ran 1 test in 0.001s

FAILED (failures=1, errors=1, skipped=1)

$ tests=.sFE ./python test_issue25894.py -v
test_subTest (__main__.TestClass) ... 
  test_subTest (__main__.TestClass) [1] (t='s') ... skipped 'skipped'
  test_subTest (__main__.TestClass) [2] (t='F') ... FAIL
  test_subTest (__main__.TestClass) [3] (t='E') ... ERROR

======================================================================
ERROR: test_subTest (main.TestClass) [3] (t='E')
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/test_issue25894.py", line 15, in test_subTest
    raise Exception('error')
    ^^^^^^^^^^^^^^^^^^^^^^^^
Exception: error

======================================================================
FAIL: test_subTest (main.TestClass) [2] (t='F')
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/test_issue25894.py", line 13, in test_subTest
    self.fail('failed')
    ^^^^^^^^^^^^^^^^^^^
AssertionError: failed

---

Ran 1 test in 0.001s

FAILED (failures=1, errors=1, skipped=1)

As a side effect, the test description is also repeated for every error in the test cleanup code (in tearDown() and doCleanup()).

Similar changes should be added also in RegressionTestResult. If apply bpo-45057 first they will be much simpler.

bpo-29152 can be related. If call addError() and addFailure() from addSubTest(), PR 28082 should be rewritten.

[serhiy-storchaka]

Any suggestions for the format of output? Currently PR 28082 formats lines for subtest skipping, failure or error with a 2-space identation. Lines for skipping, failure or error in tearDown() or functions registered with addCallback() do not differ from a line skipping, failure or error in the test method itself.

I am not sure about backporting this change. On one hand, it fixes an old flaw in the unittest output. On other hand, the change affects not only subtests and can confuse programs which parse the unittest output because the test descriptions can occur multiple times.

[ambv]

New changeset f0f29f3 by Serhiy Storchaka in branch 'main':
bpo-25894: Always report skipped and failed subtests separately (GH-28082)
f0f29f3

[ambv]

Any suggestions for the format of output? Currently PR 28082 formats lines for subtest skipping, failure or error with a 2-space identation. Lines for skipping, failure or error in tearDown() or functions registered with addCallback() do not differ from a line skipping, failure or error in the test method itself.

I'm fine with that. Ultimately I don't think differentiating subtest status from method status is that important.

I am not sure about backporting this change.

Since we're too late for 3.10.0 and it isn't a trivially small change that changes test output, I think we shouldn't be backporting it. There are tools that parse this output in editors. I'm afraid that it's too risky since we haven't given the community enough time to test for output difference.

I'm marking this as resolved. Thanks for your patch, Serhiy! If you feel strongly about backporting, we'd have to reopen and mark this as release blocker.