Skip to content

Conversation

@boomanaiden154
Copy link
Contributor

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.

Created using spr 1.3.7
@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2025

@llvm/pr-subscribers-clangd

@llvm/pr-subscribers-clang-tools-extra

Author: Aiden Grossman (boomanaiden154)

Changes

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.

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

4 Files Affected:

  • (modified) clang-tools-extra/clangd/test/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clangd/test/include-cleaner-batch-fix.test (+3-1)
  • (modified) clang-tools-extra/clangd/test/index-tools.test (+2-4)
  • (modified) clang-tools-extra/clangd/test/system-include-extractor.test (+2-1)
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

boomanaiden154 added a commit to boomanaiden154/llvm-project that referenced this pull request Nov 25, 2025
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
Created using spr 1.3.7
@@ -1,6 +1,6 @@
# Paths are not constructed correctly for the test to run on Windows.
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@boomanaiden154 boomanaiden154 merged commit 9c414c4 into main Nov 26, 2025
8 of 9 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/clangd-make-lit-tests-work-with-the-internal-shell branch November 26, 2025 00:46
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Nov 26, 2025
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
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.

4 participants