[Util] Only run --update-tests functions on failing tests (llvm#155148)

The early exit we relied on to only invoke test updaters for failing
tests requires that there was no output to stdout or stderr, and that
timeouts weren't enabled. When these conditions weren't fulfilled, test
updaters would be invoked even on passing or XFAILed tests.

Author: hnrklssn

llvm/utils/lit/lit/TestRunner.py

@@ -13,6 +13,7 @@
 import tempfile
 import threading
 import typing
+import traceback
 from typing import Optional, Tuple
 
 import io
@@ -47,6 +48,16 @@ def __init__(self, message):
         super().__init__(message)
 
 
+class TestUpdaterException(Exception):
+    """
+    There was an error not during test execution, but while invoking a function
+    in test_updaters on a failing RUN line.
+    """
+
+    def __init__(self, message):
+        super().__init__(message)
+
+
 kIsWindows = platform.system() == "Windows"
 
 # Don't use close_fds on Windows.
@@ -1192,13 +1203,23 @@ def executeScriptInternal(
                 str(result.timeoutReached),
             )
 
-        if litConfig.update_tests:
+        if (
+            litConfig.update_tests
+            and result.exitCode != 0
+            and not timeoutInfo
+            # In theory tests marked XFAIL can fail in the form of XPASS, but the
+            # test updaters are not expected to be able to fix that, so always skip for XFAIL
+            and not test.isExpectedToFail()
+        ):
             for test_updater in litConfig.test_updaters:
                 try:
                     update_output = test_updater(result, test)
                 except Exception as e:
-                    out += f"Exception occurred in test updater: {e}"
-                    continue
+                    output = out
+                    output += err
+                    output += "Exception occurred in test updater:\n"
+                    output += traceback.format_exc()
+                    raise TestUpdaterException(output)
                 if update_output:
                     for line in update_output.splitlines():
                         out += f"# {line}\n"

llvm/utils/lit/lit/worker.py

@@ -13,6 +13,7 @@
 
 import lit.Test
 import lit.util
+from lit.TestRunner import TestUpdaterException
 
 
 _lit_config = None
@@ -75,6 +76,10 @@ def _execute_test_handle_errors(test, lit_config):
     try:
         result = test.config.test_format.execute(test, lit_config)
         return _adapt_result(result)
+    except TestUpdaterException as e:
+        if lit_config.debug:
+            raise
+        return lit.Test.Result(lit.Test.UNRESOLVED, str(e))
     except:
         if lit_config.debug:
             raise

llvm/utils/lit/tests/Inputs/pass-test-update/fail.test

@@ -0,0 +1 @@
+# RUN: not echo "fail"

llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg

@@ -0,0 +1,14 @@
+import lit.formats
+
+config.name = "pass-test-update"
+config.suffixes = [".test"]
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+
+import sys
+import os
+sys.path.append(os.path.dirname(__file__))
+from should_not_run import should_not_run
+lit_config.test_updaters.append(should_not_run)
+

llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test

@@ -0,0 +1,2 @@
+# RUN: echo ""
+

llvm/utils/lit/tests/Inputs/pass-test-update/pass.test

@@ -0,0 +1 @@
+# RUN: echo "passed"

llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py

@@ -0,0 +1,2 @@
+def should_not_run(foo, bar):
+    raise Exception("this test updater should only run on failure")

llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test

@@ -0,0 +1,2 @@
+# XFAIL: *
+# RUN: not echo "failed"

llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test

@@ -0,0 +1,2 @@
+# XFAIL: *
+# RUN: echo "accidentally passed"

llvm/utils/lit/tests/pass-test-update.py

@@ -0,0 +1,39 @@
+# RUN: %{lit} --update-tests --ignore-fail -v %S/Inputs/pass-test-update | FileCheck %s --implicit-check-not Exception
+
+# CHECK:      UNRESOLVED: pass-test-update :: fail.test (1 of 5)
+# CHECK-NEXT: ******************** TEST 'pass-test-update :: fail.test' FAILED ********************
+# CHECK-NEXT: # {{R}}UN: at line 1
+# CHECK-NEXT: not echo "fail"
+# CHECK-NEXT: # executed command: not echo fail
+# CHECK-NEXT: # .---command stdout------------
+# CHECK-NEXT: # | fail
+# CHECK-NEXT: # `-----------------------------
+# CHECK-NEXT: # error: command failed with exit status: 1
+# CHECK-NEXT: Exception occurred in test updater:
+# CHECK-NEXT: Traceback (most recent call last):
+# CHECK-NEXT:   File {{.*}}, line {{.*}}, in {{.*}}
+# CHECK-NEXT:     update_output = test_updater(result, test)
+# CHECK-NEXT:   File "{{.*}}/should_not_run.py", line {{.*}}, in should_not_run
+# CHECK-NEXT:     raise Exception("this test updater should only run on failure")
+# CHECK-NEXT: Exception: this test updater should only run on failure
+# CHECK-EMPTY:
+# CHECK-NEXT: ********************
+# CHECK-NEXT: PASS: pass-test-update :: pass-silent.test (2 of 5)
+# CHECK-NEXT: PASS: pass-test-update :: pass.test (3 of 5)
+# CHECK-NEXT: {{X}}FAIL: pass-test-update :: xfail.test (4 of 5)
+# CHECK-NEXT: XPASS: pass-test-update :: xpass.test (5 of 5)
+# CHECK-NEXT: ******************** TEST 'pass-test-update :: xpass.test' FAILED ********************
+# CHECK-NEXT: Exit Code: 0
+# CHECK-EMPTY:
+# CHECK-NEXT: Command Output (stdout):
+# CHECK-NEXT: --
+# CHECK-NEXT: # {{R}}UN: at line 2
+# CHECK-NEXT: echo "accidentally passed"
+# CHECK-NEXT: # executed command: echo 'accidentally passed'
+# CHECK-NEXT: # .---command stdout------------
+# CHECK-NEXT: # | accidentally passed
+# CHECK-NEXT: # `-----------------------------
+# CHECK-EMPTY:
+# CHECK-NEXT: --
+# CHECK-EMPTY:
+# CHECK-NEXT: ********************