[ci] Failure bot: fixed auto-retry on transient errors

The auto retry tool previously treated all ERROR log lines as fatal failures.
This blocked retries for transient infrastructure issues such as network drops.

This change introduces a classification for failure markers: strict markers signal
real test failures, while generic markers are ignored when a known transient crash
is detected. This allows the system to correctly proceed with retries for flaky builds.

Author: stktyagi

.github/actions/bot-ci-failure/analyze_failure.py

@@ -6,16 +6,23 @@
 from google import genai
 from google.genai import types
 
-# Keywords that indicate an automated test failure (as opposed to
-# QA-only / commit-message-only failures).
-TEST_FAILURE_MARKERS = (
+# Strict markers that unequivocally indicate a failing test or assertion.
+STRICT_TEST_FAILURE_MARKERS = (
     "FAIL:",
-    "ERROR:",
     "FAILED (",
-    "Traceback (most recent call last):",
     "AssertionError",
 )
 
+# Generic markers that could be from a test failure (e.g., TypeError)
+# OR from a transient infrastructure crash.
+GENERIC_TEST_FAILURE_MARKERS = (
+    "ERROR:",
+    "Traceback (most recent call last):",
+)
+
+# Combined for functions that need to extract blocks of failed tests
+TEST_FAILURE_MARKERS = STRICT_TEST_FAILURE_MARKERS + GENERIC_TEST_FAILURE_MARKERS
+
 # Patterns that indicate transient / infrastructure failures which are
 # not caused by the contributor's code.
 TRANSIENT_FAILURE_MARKERS = (
@@ -149,10 +156,25 @@ def process_error_logs(content):
             continue
         seen_bodies.add(body_key)
         total_unique_jobs += 1
-        job_has_test_failure = any(m in body for m in TEST_FAILURE_MARKERS)
-        if _is_transient_failure(body) and not job_has_test_failure:
+        is_transient = _is_transient_failure(body)
+        # 1. Strict markers (e.g., "FAIL:") ALWAYS mean a real test broke.
+        has_strict_failure = any(m in body for m in STRICT_TEST_FAILURE_MARKERS)
+        # 2. Generic markers (e.g., "Traceback") could be a code bug OR an infrastructure crash.
+        has_generic_failure = any(m in body for m in GENERIC_TEST_FAILURE_MARKERS)
+        if has_strict_failure:
+            # Genuine test failures (AssertionError, FAIL:) always block auto-retry.
+            job_has_test_failure = True
+        elif is_transient:
+            # If we detected a known transient error (like a network drop), we assume
+            # the generic "ERROR:" and "Traceback" strings belong to that crash.
+            # We safely forgive them to allow the auto-retry to trigger.
+            job_has_test_failure = False
+        else:
+            # There was no transient error. Therefore, if we found generic failures
+            # (like a SyntaxError or TypeError in the code), it is a real test failure.
+            job_has_test_failure = has_generic_failure
+        if is_transient and not job_has_test_failure:
             transient_jobs += 1
-        # Detect real test failures and keep only the failing parts.
         if job_has_test_failure:
             tests_failed = True
             body = _extract_failed_tests(body)
@@ -368,6 +390,9 @@ def main():
          issue" and mention the CI has been restarted automatically if applicable.
        - For Coveralls failures specifically, mention it is a known flaky
          service and not actionable by the contributor.
+       - If there are ALSO real test failures (like AssertionErrors) in the logs,
+         tell the contributor to fix the real test failures first and push a new commit.
+         Do NOT tell them the CI restarted automatically if real test failures exist.
 
     5. **Build/Infrastructure/Other** (missing dependencies, Docker errors,
        setup failures that are NOT transient)

.github/actions/bot-ci-failure/test_analyze_failure.py

@@ -259,15 +259,55 @@ def test_single_block_no_job_headers(self):
         self.assertFalse(tests_failed)
         self.assertFalse(transient)
 
-    def test_transient_only_true_when_all_transient(self):
+    def test_pure_transient_error_allows_retry(self):
+        """Ensure a purely transient error correctly flags for a retry."""
         content = (
-            "===== JOB 100 =====\n"
-            "marionette.errors.MarionetteException: connection lost\n"
-            "===== JOB 200 =====\n"
-            "NS_ERROR_ABORT in browser\n"
+            "===== JOB 1 =====\n"
+            "Setting up Selenium...\n"
+            "selenium.common.exceptions.InvalidSessionIdException: Message: Session is not active\n"
         )
-        _, _, transient = process_error_logs(content)
-        self.assertTrue(transient)
+        text, tests_failed, transient_only = process_error_logs(content)
+        self.assertFalse(tests_failed)
+        self.assertTrue(transient_only)
+
+    def test_transient_forgives_generic_traceback(self):
+        """Ensure transient crashes with generic tracebacks don't falsely trigger test failures."""
+        content = (
+            "===== JOB 2 =====\n"
+            "Traceback (most recent call last):\n"
+            "  File 'urllib3/connectionpool.py', line 703\n"
+            "ERROR: Could not install packages due to an OSError: HTTPSConnectionPool\n"
+            "Network is unreachable\n"
+        )
+        text, tests_failed, transient_only = process_error_logs(content)
+        self.assertFalse(tests_failed)
+        self.assertTrue(transient_only)
+
+    def test_strict_failure_blocks_transient_retry(self):
+        """Ensure a strict failure (AssertionError) blocks retry even if a transient error exists."""
+        content = (
+            "===== JOB 3 =====\n"
+            "FAIL: test_user_login\n"
+            "AssertionError: True is not False\n"
+            "...\n"
+            "Connection reset by peer\n"
+        )
+        text, tests_failed, transient_only = process_error_logs(content)
+        self.assertTrue(tests_failed)
+        self.assertFalse(transient_only)
+
+    def test_pure_generic_bug_blocks_retry(self):
+        """Ensure a generic traceback without a transient error is flagged as a real code bug."""
+        content = (
+            "===== JOB 4 =====\n"
+            "Traceback (most recent call last):\n"
+            "  File 'app/models.py', line 45, in save\n"
+            "TypeError: 'NoneType' object is not subscriptable\n"
+            "ERROR: Process exited with code 1\n"
+        )
+        text, tests_failed, transient_only = process_error_logs(content)
+        self.assertTrue(tests_failed)
+        self.assertFalse(transient_only)
 
     def test_transient_only_false_when_mixed(self):
         content = (
@@ -288,6 +328,16 @@ def test_coveralls_only_is_transient(self):
         _, _, transient = process_error_logs(content)
         self.assertTrue(transient)
 
+    def test_transient_marker_containing_error_keyword(self):
+        content = (
+            "===== JOB 100 =====\n"
+            "ERROR: Could not install packages due to an OSError: HTTPSConnectionPool\n"
+            "Network is unreachable\n"
+        )
+        text, tests_failed, transient_only = process_error_logs(content)
+        self.assertTrue(transient_only)
+        self.assertFalse(tests_failed)
+
 
 class TestNormalizeForDedup(unittest.TestCase):
     """Tests for _normalize_for_dedup."""