unittest: cleanup unexpected success handling

[antonblr]

Closes #7393

EDIT: we're preserving the original behavior - unexpected success in @unittest.expectedFailure should have FAILED outcome. See discussion in #7393

---

Learning pytest internals more and decided to give it a shot. Let me know if there is a better, pytest way of implementing that.

Behaviour chages:

• unuttest unexpected success is XPASS (exit 0) when not strict (was: FAIL, exit: 1)
• twisted trial unexpected success is XPASS (exit 0) when not strict (was: FAIL, exit: 1)

[bluetech]

Would be great to get this fixed!

I left a couple of comments/questions.

Also, should add a bugfix changelog entry for this, some users would probably be affected by it.

[antonblr]

is that a bugfix or feature ?

[antonblr]

@bluetech - please have a look. The original behavior is preserved this time.

Example:

import pytest
import unittest
from twisted.trial import unittest as tw_unittest 

class ExpectedFailureTestCase(unittest.TestCase):
    @unittest.expectedFailure
    def test_expected_failure(self):
        self.assertTrue(False)

    @unittest.expectedFailure
    def test_unexpected_success(self):
        self.assertTrue(True)


class ExpectedFailureTwistedTestCase(tw_unittest.TestCase):
    todo = "not ready"

    def test_expected_failure(self):
        self.assertTrue(False)

    def test_unexpected_success(self):
        self.assertTrue(True)


class TestPytestExpectedFailure:
    @pytest.mark.xfail(reason="not ready")
    def test_expected_failure(self):
        assert False

    @pytest.mark.xfail(reason="not ready")
    def test_unexpected_success(self):
        assert True

Running with xfail_strict=false:

$pytest -s -v ut.py -o xfail_strict=false

ut.py::ExpectedFailureTestCase::test_expected_failure XFAIL
ut.py::ExpectedFailureTestCase::test_unexpected_success FAILED
ut.py::ExpectedFailureTwistedTestCase::test_expected_failure XFAIL (<Todo reason='not ready' errors=None>)
ut.py::ExpectedFailureTwistedTestCase::test_unexpected_success FAILED
ut.py::TestPytestExpectedFailure::test_expected_failure XFAIL (not ready)
ut.py::TestPytestExpectedFailure::test_unexpected_success XPASS (not ready)

================================================================================================= FAILURES =================================================================================================
_____________________________________________________________________________ ExpectedFailureTestCase.test_unexpected_success ______________________________________________________________________________
Unexpected success
__________________________________________________________________________ ExpectedFailureTwistedTestCase.test_unexpected_success __________________________________________________________________________
Unexpected success: not ready
========================================================================================= short test summary info ==========================================================================================
FAILED ut.py::ExpectedFailureTestCase::test_unexpected_success
FAILED ut.py::ExpectedFailureTwistedTestCase::test_unexpected_success
XPASS ut.py::TestPytestExpectedFailure::test_unexpected_success not ready
================================================================================= 2 failed, 3 xfailed, 1 xpassed in 0.19s ==================================================================================

Running with xfail_strict=true:

$ pytest -s -v ut.py -o xfail_strict=true

ut.py::ExpectedFailureTestCase::test_expected_failure XFAIL
ut.py::ExpectedFailureTestCase::test_unexpected_success FAILED
ut.py::ExpectedFailureTwistedTestCase::test_expected_failure XFAIL (<Todo reason='not ready' errors=None>)
ut.py::ExpectedFailureTwistedTestCase::test_unexpected_success FAILED
ut.py::TestPytestExpectedFailure::test_expected_failure XFAIL (not ready)
ut.py::TestPytestExpectedFailure::test_unexpected_success FAILED

================================================================================================= FAILURES =================================================================================================
_____________________________________________________________________________ ExpectedFailureTestCase.test_unexpected_success ______________________________________________________________________________
Unexpected success
__________________________________________________________________________ ExpectedFailureTwistedTestCase.test_unexpected_success __________________________________________________________________________
Unexpected success: not ready
____________________________________________________________________________ TestPytestExpectedFailure.test_unexpected_success _____________________________________________________________________________
[XPASS(strict)] not ready
========================================================================================= short test summary info ==========================================================================================
FAILED ut.py::ExpectedFailureTestCase::test_unexpected_success
FAILED ut.py::ExpectedFailureTwistedTestCase::test_unexpected_success
FAILED ut.py::TestPytestExpectedFailure::test_unexpected_success
======================================================================================= 3 failed, 3 xfailed in 0.19s =======================================================================================

[bluetech]

LGTM, thanks! Ended up as a nice clean up at least.

[nicoddemus]

Great work @antonblr!

Sorry for joining late, but seems we missed a CHANGELOG entry. Seems like an improvement to me; arguably a bugfix too, but I would rather release this in the next minor release rather than part of a bug fix release.

[antonblr]

Hi @nicoddemus - what do you think should be outlined in CHANGELOG?
Initially there was a behavior change, so I added a changelog entry. But now it seem more like just internals cleanup.

The only thing I think might worth to outline is the change to outcome message for unittest twisted todo - it was representation of the Todo instance, but now it is Todo.reason (just the actual reason). What do you think?

[nicoddemus]

Initially there was a behavior change, so I added a changelog entry. But now it seem more like just internals cleanup.

Ahh I misunderstood then, I thought we had change how unexpectedPass behaved. 👍

Even so a trivial changelog entry explaining the internal clean up is worthwhile, it might help users which hit a corner case brought up by the refactoring (but is not really necessary, just a nice to have thing).