[llvm] [Util] Only run --update-tests functions on failing tests (PR #155148)
Henrik G. Olsson via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 25 09:13:34 PDT 2025
https://github.com/hnrklssn updated https://github.com/llvm/llvm-project/pull/155148
>From 061960f44f91b6d5010977532551facbaf263455 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Sun, 24 Aug 2025 00:17:04 -0700
Subject: [PATCH 1/3] [Util] Only run --update-tests functions on failing tests
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.
---
llvm/utils/lit/lit/TestRunner.py | 26 ++++++++++--
llvm/utils/lit/lit/worker.py | 5 +++
.../tests/Inputs/pass-test-update/fail.test | 1 +
.../lit/tests/Inputs/pass-test-update/lit.cfg | 14 +++++++
.../Inputs/pass-test-update/pass-silent.test | 2 +
.../tests/Inputs/pass-test-update/pass.test | 1 +
.../Inputs/pass-test-update/should_not_run.py | 3 ++
.../tests/Inputs/pass-test-update/xfail.test | 2 +
.../tests/Inputs/pass-test-update/xpass.test | 2 +
llvm/utils/lit/tests/pass-test-update.py | 40 +++++++++++++++++++
10 files changed, 93 insertions(+), 3 deletions(-)
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/fail.test
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/pass.test
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test
create mode 100644 llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test
create mode 100644 llvm/utils/lit/tests/pass-test-update.py
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index f2c5c6d0dbe93..516c63ec9e78e 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/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,22 @@ def executeScriptInternal(
str(result.timeoutReached),
)
- if litConfig.update_tests:
+ if (litConfig.update_tests
+ and result.exitCode != 0
+ and not timeoutInfo
+ # Don't run test updaters on XPASS failures -
+ # they are not expected to be able to fix them
+ 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"
diff --git a/llvm/utils/lit/lit/worker.py b/llvm/utils/lit/lit/worker.py
index 8e78bfd45d38b..0e36f3e9a2424 100644
--- a/llvm/utils/lit/lit/worker.py
+++ b/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
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test b/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test
new file mode 100644
index 0000000000000..a410ef9293129
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/fail.test
@@ -0,0 +1 @@
+# RUN: not echo "fail"
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg b/llvm/utils/lit/tests/Inputs/pass-test-update/lit.cfg
new file mode 100644
index 0000000000000..e0dbc3220678c
--- /dev/null
+++ b/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)
+
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test b/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test
new file mode 100644
index 0000000000000..07cafbc66a897
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/pass-silent.test
@@ -0,0 +1,2 @@
+# RUN: echo ""
+
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test b/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test
new file mode 100644
index 0000000000000..3a12c5fee4bb0
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/pass.test
@@ -0,0 +1 @@
+# RUN: echo "passed"
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
new file mode 100644
index 0000000000000..adba049753594
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
@@ -0,0 +1,3 @@
+def should_not_run(foo, bar):
+ raise Exception("this test updater should only run on failure")
+
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test b/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test
new file mode 100644
index 0000000000000..044b6a19b4a19
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/xfail.test
@@ -0,0 +1,2 @@
+# XFAIL: *
+# RUN: not echo "failed"
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test b/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test
new file mode 100644
index 0000000000000..e0c1c39762ee9
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/xpass.test
@@ -0,0 +1,2 @@
+# XFAIL: *
+# RUN: echo "accidentally passed"
diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py
new file mode 100644
index 0000000000000..b3cb7057ef163
--- /dev/null
+++ b/llvm/utils/lit/tests/pass-test-update.py
@@ -0,0 +1,40 @@
+# 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: ********************
+
>From 4e7c5381a39b2cc8f941d7f6742be6c9d9a626cb Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Sun, 24 Aug 2025 11:12:35 -0700
Subject: [PATCH 2/3] format python code
---
llvm/utils/lit/lit/TestRunner.py | 3 ++-
llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py | 1 -
llvm/utils/lit/tests/pass-test-update.py | 1 -
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 516c63ec9e78e..5dec66f2ceacd 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1203,7 +1203,8 @@ def executeScriptInternal(
str(result.timeoutReached),
)
- if (litConfig.update_tests
+ if (
+ litConfig.update_tests
and result.exitCode != 0
and not timeoutInfo
# Don't run test updaters on XPASS failures -
diff --git a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
index adba049753594..0fda62c832f08 100644
--- a/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
+++ b/llvm/utils/lit/tests/Inputs/pass-test-update/should_not_run.py
@@ -1,3 +1,2 @@
def should_not_run(foo, bar):
raise Exception("this test updater should only run on failure")
-
diff --git a/llvm/utils/lit/tests/pass-test-update.py b/llvm/utils/lit/tests/pass-test-update.py
index b3cb7057ef163..e3efb9df919c4 100644
--- a/llvm/utils/lit/tests/pass-test-update.py
+++ b/llvm/utils/lit/tests/pass-test-update.py
@@ -37,4 +37,3 @@
# CHECK-NEXT: --
# CHECK-EMPTY:
# CHECK-NEXT: ********************
-
>From 310614253ddb5be748e5d750909c52508923f451 Mon Sep 17 00:00:00 2001
From: "Henrik G. Olsson" <h_olsson at apple.com>
Date: Mon, 25 Aug 2025 09:12:50 -0700
Subject: [PATCH 3/3] clarify comment
---
llvm/utils/lit/lit/TestRunner.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 5dec66f2ceacd..31e3ed679d431 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1207,8 +1207,8 @@ def executeScriptInternal(
litConfig.update_tests
and result.exitCode != 0
and not timeoutInfo
- # Don't run test updaters on XPASS failures -
- # they are not expected to be able to fix them
+ # 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:
More information about the llvm-commits
mailing list