DiscoverRunner doesn't counts unexpectedSuccess as an error

[Baptiste Mispelon]

I noticed today that my CI test suite was green even though its console output printed:

FAILED (expected failures=13, unexpected successes=1)

It turns out that Django's default DiscoverRunner does not count "unexpected successes" as errors when deciding the return code of its test management command. [1]

So even though it prints "FAILED", the command returns with an exit code of 0 which my CI (correctly) interprets as a success.

The fix seems quite simple:

django/test/runner.py

@@ -875 +875 @@
         teardown_test_environment()
 
     def suite_result(self, suite, result, **kwargs):
-        return len(result.failures) + len(result.errors)
+        return len(result.failures) + len(result.errors) + len(result.unexpectedSuccesses)
 
     def _get_databases(self, suite):
         databases = {}

But I wonder if that would be considered backwards incompatible.
I also think this is a pretty serious issue and I wonder if backports would be considered for this.

[1] ​https://github.com/django/django/blob/60503cc747eeda7c61bab02b71f8f55a733a6eea/django/test/runner.py#L877-L878

[Mariusz Felisiak]

Thanks for the report! As far as I'm aware this doesn't qualify for a backport and yes it's backward incompatible. This behavior is in Django from the very beginning (e.g. we take errors into account from 2007, see c1a73d80da65694319d803160b5e400e11318213). Python's TestResult treats unexpected successes as a tests failure since version ​3.4.

I'd change this in Django 4.1 without a deprecation period.

[Baptiste Mispelon]

Thanks for the quick triage and the git history research.

I'll get started on a PR today.

Meanwhile, if anyone finds this ticket looking for a workaround, it's easy enough to write a custom runner and point to it in settings.TEST_RUNNER:

class CustomTestRunner(DiscoverRunner):
    """
    A custom test runner to get around Django bug
    https://code.djangoproject.com/ticket/33264
    """
    def suite_result(self, suite, result, **kwargs):
        return len(result.failures) + len(result.errors) + len(result.unexpectedSuccesses)

[Baptiste Mispelon]

Here is the PR: ​https://github.com/django/django/pull/15060 ✨

[Tim Graham]

I'll accept this change as correct, but I'll also mention that I've used expectedFailure to prevent tests that sometimes fail (e.g. ​https://github.com/cockroachdb/django-cockroachdb/commit/97aaf0bd3abff19b1405215d377b5c91e00cfc84) from causing a test suite to fail. I guess I'll have to instead skip these tests.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 91acfc3:

Fixed #33264 -- Made test runner return non-zero error code for unexpected successes.