test_runner: Write failed tests with a side effect.

`DiscoverRunner.run_tests` has a return type of `int`. While
`Runner.run_tests` has a wildly different `Tuple[bool, List[str]]`.

This refactors it so that we have the correct return type, by passing
the additional information about failed tests through a side effect to directly
write the failed tests to a file.

Note that we have to make `failed_tests_path` optional as otherwise the method
signature will not be compatible with the supertype.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>

tools/test-backend

@@ -162,12 +162,6 @@ def get_failed_tests() -> List[str]:
         return []
 
 
-def write_failed_tests(failed_tests: List[str]) -> None:
-    if failed_tests:
-        with open(FAILED_TEST_PATH, "wb") as f:
-            f.write(orjson.dumps(failed_tests))
-
-
 @contextlib.contextmanager
 def block_internet() -> Iterator[None]:
     # Monkey-patching - responses library raises requests.ConnectionError when access to an unregistered URL
@@ -429,10 +423,12 @@ def main() -> None:
             reverse=options.reverse,
             keepdb=True,
         )
-        failures, failed_tests = test_runner.run_tests(
-            suites, full_suite=full_suite, include_webhooks=include_webhooks
+        failures = test_runner.run_tests(
+            suites,
+            failed_tests_path=FAILED_TEST_PATH,
+            full_suite=full_suite,
+            include_webhooks=include_webhooks,
         )
-    write_failed_tests(failed_tests)
 
     templates_not_rendered = test_runner.get_shallow_tested_templates()
     # We only check the templates if all the tests ran and passed

zerver/lib/test_runner.py

@@ -8,6 +8,7 @@ from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Ty
 from unittest import TestLoader, TestSuite, runner
 from unittest.result import TestResult
 
+import orjson
 from django.conf import settings
 from django.db import ProgrammingError, connections
 from django.test import runner as django_runner
@@ -377,10 +378,11 @@ class Runner(DiscoverRunner):
         self,
         test_labels: List[str],
         extra_tests: Optional[List[unittest.TestCase]] = None,
+        failed_tests_path: Optional[str] = None,
         full_suite: bool = False,
         include_webhooks: bool = False,
         **kwargs: Any,
-    ) -> Tuple[bool, List[str]]:
+    ) -> int:
         self.setup_test_environment()
         try:
             suite = self.build_suite(test_labels, extra_tests)
@@ -417,11 +419,15 @@ class Runner(DiscoverRunner):
         # a Django connection to be rolled back mid-test.
         with get_sqlalchemy_connection():
             result = self.run_suite(suite)
+            assert isinstance(result, TextTestResult)
         self.teardown_test_environment()
         failed = self.suite_result(suite, result)
         if not failed:
             write_instrumentation_reports(full_suite=full_suite, include_webhooks=include_webhooks)
-        return failed, result.failed_tests
+        if failed_tests_path and result.failed_tests:
+            with open(failed_tests_path, "wb") as f:
+                f.write(orjson.dumps(result.failed_tests))
+        return failed
 
 
 def get_test_names(suite: Union[TestSuite, ParallelTestSuite]) -> List[str]: