-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clangd] Make lit tests work with the internal shell #169539
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
[clangd] Make lit tests work with the internal shell #169539
Conversation
Created using spr 1.3.7
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Aiden Grossman (boomanaiden154) ChangesThis makes all of the clangd tests work with the internal shell.
Full diff: https://github.com/llvm/llvm-project/pull/169539.diff 4 Files Affected:
diff --git a/clang-tools-extra/clangd/test/CMakeLists.txt b/clang-tools-extra/clangd/test/CMakeLists.txt
index 42fc3506641f2..bdcc94dc52ebb 100644
--- a/clang-tools-extra/clangd/test/CMakeLists.txt
+++ b/clang-tools-extra/clangd/test/CMakeLists.txt
@@ -3,6 +3,7 @@ set(CLANGD_TEST_DEPS
ClangdTests
clangd-indexer
split-file
+ IndexBenchmark
)
if(CLANGD_BUILD_XPC)
diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
index 07ebe1009a78f..5a87a87e2f63a 100644
--- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -7,7 +7,9 @@
# RUN: cp -r %S/Inputs/include-cleaner %t/include
# RUN: echo '-I%t/include' > %t/compile_flags.txt
# Create a config file enabling include-cleaner features.
-# RUN: echo $'Diagnostics:\n UnusedIncludes: Strict\n MissingIncludes: Strict' >> %t/clangd/config.yaml
+# RUN: echo 'Diagnostics:' > %t/clangd/config.yaml
+# RUN: echo ' UnusedIncludes: Strict' >> %t/clangd/config.yaml
+# RUN: echo ' MissingIncludes: Strict' >> %t/clangd/config.yaml
# RUN: env XDG_CONFIG_HOME=%t clangd -lit-test -enable-config --compile-commands-dir=%t < %s | FileCheck -strict-whitespace %s
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true, "changeAnnotationSupport":{"groupsOnLabel":true}}}},"trace":"off"}}
diff --git a/clang-tools-extra/clangd/test/index-tools.test b/clang-tools-extra/clangd/test/index-tools.test
index 93cf56fea371a..04bba68fea7fe 100644
--- a/clang-tools-extra/clangd/test/index-tools.test
+++ b/clang-tools-extra/clangd/test/index-tools.test
@@ -1,6 +1,4 @@
# RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
-# FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# REQUIRES: shell
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
+# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01
# Pass invalid JSON file and check that IndexBenchmark fails to parse it.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then not %clangd-benchmark-dir/IndexBenchmark %t.index %t --benchmark_min_time=0.01 ; fi
+# RUN: not %clangd-benchmark-dir/IndexBenchmark %t.index %t --benchmark_min_time=0.01
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test b/clang-tools-extra/clangd/test/system-include-extractor.test
index 83a8c28bf7d56..3314be806a801 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,7 +5,8 @@
# Create a bin directory to store the mock-driver and add it to the path
# RUN: mkdir -p %t.dir/bin
-# RUN: export PATH=%t.dir/bin:$PATH
+# RUN: %python -c "print(__import__('os').environ['PATH'])" > %t.path
+# RUN: export PATH=%t.dir/bin:%{readfile:%t.path}
# Generate a mock-driver that will print %temp_dir%/my/dir and
# %temp_dir%/my/dir2 as include search paths.
# RUN: echo '#!/bin/sh' >> %t.dir/bin/my_driver.sh
|
This makes all of the clangd tests work with the internal shell. Modifications needed for each test are as follows: 1. system-include-extractor.test was using variable expansion which is not supported in the internal shell. This patch rewrites it to use the readfile mechanism along with python. This isn't super pretty but is readily understandable and there are only two tests across the monorepo that use this construction, so making it prettier is hard to justify. 2. include-cleaner-batch-fix.test - Was using $'' construction to create new lines in a string. Simply replace it with multiple echo commands to be canonical with the rest of the repository. 3. index-tools.test - Just add IndexBenchmark to the clangd test depends, so the test now just works unconditionally. This should significantly increase test coverage at little cost. Pull Request: llvm#169539
| @@ -1,6 +1,6 @@ | |||
| # Paths are not constructed correctly for the test to run on Windows. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is also a current limitation, so we're not accidentally losing coverage on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. REQUIRES: shell implies that it does not run on Windows.
This patch actually increases coverage by quite a bit because previously the test would only do anything if you manually built the benchmark target due to the lack of build system dependencies and how the test was constructed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, missed it under the FIXME. thanks.
This makes all of the clangd tests work with the internal shell. Modifications needed for each test are as follows: 1. system-include-extractor.test was using variable expansion which is not supported in the internal shell. This patch rewrites it to use the readfile mechanism along with python. This isn't super pretty but is readily understandable and there are only two tests across the monorepo that use this construction, so making it prettier is hard to justify. 2. include-cleaner-batch-fix.test - Was using $'' construction to create new lines in a string. Simply replace it with multiple echo commands to be canonical with the rest of the repository. 3. index-tools.test - Just add IndexBenchmark to the clangd test depends, so the test now just works unconditionally. This should significantly increase test coverage at little cost. Reviewers: ilovepi, HighCommander4, petrhosek, kadircet Reviewed By: ilovepi Pull Request: llvm/llvm-project#169539
This makes all of the clangd tests work with the internal shell.
Modifications needed for each test are as follows:
not supported in the internal shell. This patch rewrites it to use
the readfile mechanism along with python. This isn't super pretty but
is readily understandable and there are only two tests across the
monorepo that use this construction, so making it prettier is hard to
justify.
new lines in a string. Simply replace it with multiple echo commands
to be canonical with the rest of the repository.
depends, so the test now just works unconditionally. This should
significantly increase test coverage at little cost.