-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[lit] Make not still fail if the called process returns a signal #174298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lit] Make not still fail if the called process returns a signal #174298
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-testing-tools Author: Aiden Grossman (boomanaiden154) ChangesThis is the behavior of the main not binary that was not preserved in Full diff: https://github.com/llvm/llvm-project/pull/174298.diff 6 Files Affected:
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 7a233b238f7e2..fb0f0da568276 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -783,6 +783,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
procs = []
proc_not_counts = []
+ proc_not_fail_if_crash = []
default_stdin = subprocess.PIPE
stderrTempFiles = []
opened_files = []
@@ -913,6 +914,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
if not_crash:
args = not_args + args
not_count = 0
+ elif not_args == ['not']:
+ pass
else:
not_args = []
@@ -1008,6 +1011,10 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
if old_umask != -1:
os.umask(old_umask)
proc_not_counts.append(not_count)
+ if not not_crash and not_args == ['not']:
+ proc_not_fail_if_crash.append(True)
+ else:
+ proc_not_fail_if_crash.append(False)
# Let the helper know about this process
timeoutHelper.addProcess(procs[-1])
except OSError as e:
@@ -1063,7 +1070,10 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
if res == -signal.SIGINT:
raise KeyboardInterrupt
if proc_not_counts[i] % 2:
- res = 1 if res == 0 else 0
+ if proc_not_fail_if_crash[i]:
+ res = 1 if res <= 0 else 0
+ else:
+ res = 1 if res == 0 else 0
elif proc_not_counts[i] > 1:
res = 1 if res != 0 else 0
diff --git a/llvm/utils/lit/tests/Inputs/shtest-not-posix/fail-signal.py b/llvm/utils/lit/tests/Inputs/shtest-not-posix/fail-signal.py
new file mode 100644
index 0000000000000..01db32585d071
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not-posix/fail-signal.py
@@ -0,0 +1,6 @@
+#!/usr/bin/env python
+
+import os
+import signal
+
+os.kill(os.getpid(), signal.SIGABRT)
diff --git a/llvm/utils/lit/tests/Inputs/shtest-not-posix/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-not-posix/lit.cfg
new file mode 100644
index 0000000000000..37f1a141a48ab
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not-posix/lit.cfg
@@ -0,0 +1,8 @@
+import lit.formats
+
+config.name = "shtest-not-posix"
+config.suffixes = [".txt"]
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+config.substitutions.append(("%{python}", '"%s"' % (sys.executable)))
diff --git a/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal-crash.txt b/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal-crash.txt
new file mode 100644
index 0000000000000..8f7638a7970df
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal-crash.txt
@@ -0,0 +1,3 @@
+# Check that the builtin not passes when we fail with a signal with --crash
+
+# RUN: not --crash %{python} fail-signal.py
diff --git a/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal.txt b/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal.txt
new file mode 100644
index 0000000000000..be3506b3322c7
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-not-posix/not-signal.txt
@@ -0,0 +1,3 @@
+# Check that the builtin not still fails when we fail with a signal.
+
+# RUN: not %{python} fail-signal.py
diff --git a/llvm/utils/lit/tests/shtest-not-posix.py b/llvm/utils/lit/tests/shtest-not-posix.py
new file mode 100644
index 0000000000000..6468d5ea48450
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-not-posix.py
@@ -0,0 +1,13 @@
+# Check the not command correctly handles POSIX signals
+
+# UNSUPPORTED: system-windows
+
+# RUN: not %{lit} -a %{inputs}/shtest-not-posix \
+# RUN: | FileCheck -match-full-lines %s
+
+# CHECK: -- Testing: 2 tests{{.*}}
+
+# CHECK PASS: shtest-not-posix :: not-signal-crash.txt (1 of 2)
+
+# CHECK: FAIL: shtest-not-posix :: not-signal.txt (2 of 2)
+# CHECK: # error: command failed with exit status: 1
|
|
✅ With the latest revision this PR passed the Python code formatter. |
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
This is the behavior of the main not binary that was not preserved in the internal shell. Make it so that the builtin not command does actually fail if we end up with a signal rather than just a non-zero exit code. Pull Request: llvm#174298
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
This is the behavior of the main not binary that was not preserved in the internal shell. Make it so that the builtin not command does actually fail if we end up with a signal rather than just a non-zero exit code. Pull Request: llvm#174298
…signal This is the behavior of the main not binary that was not preserved in the internal shell. Make it so that the builtin not command does actually fail if we end up with a signal rather than just a non-zero exit code. Reviewers: petrhosek, ilovepi, jdenny-ornl, arichardson Pull Request: llvm/llvm-project#174298
|
I think It could be the test is wrong and this just exposed it. |
That would be my guess. If you run the command locally and you get a signal, then the test needs to be updated to pass |
|
Yeah seems that was it, made #174499, thx |
`not` behavior change in #174298 requires `--crash` passed now. Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
…(#174499) `not` behavior change in llvm/llvm-project#174298 requires `--crash` passed now. Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
This is the behavior of the main not binary that was not preserved in the internal shell. Make it so that the builtin not command does actually fail if we end up with a signal rather than just a non-zero exit code. Reviewers: petrhosek, ilovepi, jdenny-ornl, arichardson Pull Request: llvm#174298
`not` behavior change in llvm#174298 requires `--crash` passed now. Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
This is the behavior of the main not binary that was not preserved in
the internal shell. Make it so that the builtin not command does
actually fail if we end up with a signal rather than just a non-zero
exit code.