Skip to content

[compiler-rt][test] Add REQUIRES: shell to focus-function.test with for-loop #106150

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

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

Harini0924
Copy link
Contributor

@Harini0924 Harini0924 commented Aug 26, 2024

This patch adds REQUIRES: shell to the focus-function.test because the lit internal shell does not support the for loop syntax. This will make the test file unsupported when running llvm-lit with its internal shell implementation, which is enabled by turning on the LIT_USE_INTERNAL_SHELL=1.

fixes: #102380

This patch add REQUIRES: shell to focus-function.test because lit internal shell
can't support the for loop syntax in this test.
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (Harini0924)

Changes

This patch adds REQUIRES: shell to the focus-function.test because the lit internal shell does not support the for loop syntax. This will make the test file unsupported when running llvm-lit with its internal shell implementation, which is enabled by turning on the LIT_USE_INTERNAL_SHELL=1.

fixes: #106111


Full diff: https://github.com/llvm/llvm-project/pull/106150.diff

1 Files Affected:

  • (modified) compiler-rt/test/fuzzer/focus-function.test (+1-1)
diff --git a/compiler-rt/test/fuzzer/focus-function.test b/compiler-rt/test/fuzzer/focus-function.test
index ec4a03c95a6355..09a64b043afc11 100644
--- a/compiler-rt/test/fuzzer/focus-function.test
+++ b/compiler-rt/test/fuzzer/focus-function.test
@@ -1,7 +1,7 @@
 # Tests -focus_function
 #
 # TODO: don't require linux.
-# REQUIRES: linux
+# REQUIRES: shell, linux
 UNSUPPORTED: target=aarch64{{.*}}
 
 RUN: %cpp_compiler %S/OnlySomeBytesTest.cpp -o %t-exe

@Harini0924 Harini0924 changed the title Add REQUIRES: shell to focus-function.test [compiler-rt][test] Add REQUIRES: shell to focus-function.test with for-loop Aug 26, 2024
@fmayer
Copy link
Contributor

fmayer commented Aug 26, 2024

general nit about these CLs: i think it wouldn't hurt to add a comment next to the REQUIRES: shell saying why

@ilovepi
Copy link
Contributor

ilovepi commented Aug 26, 2024

general nit about these CLs: i think it wouldn't hurt to add a comment next to the REQUIRES: shell saying why

That's a fair point. @Harini0924, would you mind addressing that here an in your other PRs?

@Harini0924 Harini0924 requested a review from fmayer August 26, 2024 23:39
@Harini0924 Harini0924 merged commit 3b79468 into llvm:main Aug 27, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[llvm-lit] lit internal shell failing to parse and execute for-loop syntax
4 participants