Fail on subtest failure

[maxnikulin]

Avoid confusion with tests having failed subtests accounted as passed.

• Store failed subtest count as an Item attribute.
• Force additional exception for passed or skipped
  tests if some failures happened in subtests.

Accounting and reporting are not ideal:

• Count of failed tests is incremented once more.
• Since all subtest exception are reported earlier, it is impossible
  to make some of them main failure reason.

Alleviate #11 a bit.

[maxnikulin]

Before: passed

============================= test session starts ==============================
platform linux -- Python 3.8.0, pytest-5.4.3, py-1.8.2, pluggy-0.13.1
rootdir: /home/ptst/pytest-subtests-test
plugins: subtests-0.3.2.dev3+g31050a9
collected 1 item

test_demo.py F..                                                         [100%]

=================================== FAILURES ===================================
___________________________ test_demo [binary math] ____________________________

subtests = SubTests(ihook=<pluggy.hooks._HookRelay object at 0x7f5d4a6ff130>, suspend_capture_ctx=<bound method CaptureManager.gl...state='resumed' _in_suspended=False> _capture_fixture=None>>, request=<SubRequest 'subtests' for <Function test_demo>>)

    def test_demo(subtests):
        with subtests.test("binary math"):
>           assert 1 + 1 == 10
E           assert (1 + 1) == 10

test_demo.py:4: AssertionError
=========================== short test summary info ============================
FAILED test_demo.py::test_demo - assert (1 + 1) == 10
========================= 1 failed, 1 passed in 0.08s ==========================

After: additional failure

============================= test session starts ==============================
platform linux -- Python 3.8.0, pytest-5.4.3, py-1.8.2, pluggy-0.13.1
rootdir: /home/ptst/pytest-subtests-test
plugins: subtests-0.3.2.dev3+g31050a9
collected 1 item

test_demo.py F.F                                                         [100%]

=================================== FAILURES ===================================
___________________________ test_demo [binary math] ____________________________

subtests = SubTests(ihook=<pluggy.hooks._HookRelay object at 0x7fba4d841130>, suspend_capture_ctx=<bound method CaptureManager.gl...state='resumed' _in_suspended=False> _capture_fixture=None>>, request=<SubRequest 'subtests' for <Function test_demo>>)

    def test_demo(subtests):
        with subtests.test("binary math"):
>           assert 1 + 1 == 10
E           assert (1 + 1) == 10

test_demo.py:4: AssertionError
__________________________________ test_demo ___________________________________

item = <Function test_demo>

    def check_failed_subtests(item):
        failed = getattr(item, _ATTR_COUNTER, 0)
        if failed > 0:
>           raise SubTestFailed.from_count(failed)
E           pytest_subtests.SubTestFailed: Failed subtests: 1

../pytest-subtests/pytest_subtests.py:223: SubTestFailed
=========================== short test summary info ============================
FAILED test_demo.py::test_demo - assert (1 + 1) == 10
FAILED test_demo.py::test_demo - pytest_subtests.SubTestFailed: Failed subtes...
========================= 2 failed, 0 passed in 0.09s ==========================

[RonnyPfannschmidt]

Im - 1 on this, from my poc subtests contain failures, i believe letting them leak to the test directly or in aggregate should be opt in

[maxnikulin]

Reports of failures in tests marked as expected failures should be suppressed (unless run with --run-xfail option). It should be easily achievable on the top of these changes (and rather useless without them).

Any other considerations or use cases when leaking of reports and propagation of results in proposed way are desired or vice versa makes results more confusing?

[nicoddemus]

Im - 1 on this, from my poc subtests contain failures, i believe letting them leak to the test directly or in aggregate should be opt in

IIUC, this PR makes the behavior similar to how TestCase.subTest works:

import unittest

class T(unittest.TestCase):

    def test(self):
        with self.subTest("test"):
            assert 0

if __name__ == "__main__":
    unittest.main()

λ py -3.7 .tmp\test_ut_sub.py

======================================================================
FAIL: test (__main__.T) [test]
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".tmp\test_ut_sub.py", line 8, in test
    assert 0
AssertionError

----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (failures=1)

My initial intention (at least) with this plugin would be to mimic the reporting behavior of TestCase.subTest.

So I'm on the fence of the behavior we should adopt.

[nicoddemus]

Btw thanks @maxnikulin for the PR!

[RonnyPfannschmidt]

as far as i can tell current behavour is closer to unittest, as unittest only reports the subtest failure and the executed "items"

so from my pov we could do better reporting, but its still a stronger deviation to report the surrounding test as failure together with the subtest

[maxnikulin]

Totally agree that additional failure report should be avoided somehow.

I suppose we just disagree on relative importance of issues. I believe that it is really confusing when failed test is reported as successful (collected == passed)

collected 2 items
...
= 1 failed, 2 passed in 0.11s =

You could not tolerate additional stack trace and increased failure counter despite correct number of passed tests

= 2 failed, 1 passed in 0.12s =

I do not have strong opinion which deviation is stronger. It seams that failure number could easily overrun collected items count.

I am afraid, some changes in pytest core are required for accurate fix but I do not have a clear plan. It may require to allow list of exceptions for test result instead of single one. A workaround that could easily became a source of abuses is to introduce class Ignore(Outcome) exception that blocks increasing of any counter on test completion.

[RonnyPfannschmidt]

@maxnikulin i would actually like better numbers, to me the proposed solution seems unfortunately does that by introducing tracekbacks that i would consider "incorrect"

i'd much prefer the correct nubmers by fixing the stats, but yes that needs a core intervention

[maxnikulin]

i would actually like better numbers, to me the proposed solution seems unfortunately does that by introducing tracekbacks that i would consider "incorrect"

I have an idea to add a special report that suppresses output of traceback. One failed subtest will still increment failure counter by 2.

See #29 however for approach with reporting of subtests postponed till pytest_runtest_makereport. Again it is some trade off.

[maxnikulin]

@RonnyPfannschmidt, do you think the following is better than traceback report for the whole test? Certainly, exact form of the failure report could be discussed, consider current variant as a proof of concept.

============================= test session starts ==============================
platform linux -- Python 3.6.9, pytest-5.4.3, py-1.5.2, pluggy-0.13.1 -- /home/ptst/venv/36-pytest-dev/bin/python
cachedir: .pytest_cache
rootdir: /home/ptst/pytest-subtests-test
plugins: xdist-1.32.0, forked-1.1.3, subtests-0.3.2.dev4+gd942684
gw0 I
[gw0] linux Python 3.6.9 cwd: /home/ptst/pytest-subtests-test
[gw0] Python 3.6.9 (default, Apr 18 2020, 01:56:04)  -- [GCC 8.4.0]
gw0 [1]

scheduling tests via LoadScheduling

test_demo.py::test_demo 
[gw0] [100%] FAILED test_demo.py::test_demo 
[gw0] [100%] PASSED test_demo.py::test_demo 
[gw0] [100%] FAILED test_demo.py::test_demo 

=================================== FAILURES ===================================
___________________________ test_demo [binary math] ____________________________
[gw0] linux -- Python 3.6.9 /home/ptst/venv/36-pytest-dev/bin/python

subtests = SubTests(ihook=<pluggy.hooks._HookRelay object at 0x7f18e4a7eef0>, suspend_capture_ctx=<bound method CaptureManager.gl...state='resumed' _in_suspended=False> _capture_fixture=None>>, request=<SubRequest 'subtests' for <Function test_demo>>)

    def test_demo(subtests):
        with subtests.test("binary math"):
>           assert 1 + 1 == 10
E           assert 2 == 10
E             +2
E             -10

test_demo.py:4: AssertionError
__________________________________ test_demo ___________________________________
[gw0] linux -- Python 3.6.9 /home/ptst/venv/36-pytest-dev/bin/python
Failed subtests: 1
=========================== short test summary info ============================
FAILED test_demo.py::test_demo - assert 2 == 10
FAILED test_demo.py::test_demo
========================= 1 failed, 0 passed in 0.40s ==========================

[nicoddemus]

Given it is not a consensus how this should be handled, lets close this for now.

Thanks again @maxnikulin for the PR!