[clangd] Test benchmark if we can build it

[kbobyrev]

Looks good, thanks!

While we're here: I'm wondering whether we should also introduce very basic test which would just run clangd-indexer since it doesn't depend on benchmarks and would be run on all buildbots.

Also, I was thinking whether we should ask some buildbot owners to enable LLVM_BUILD_BENCHMARKS, but it probably makes sense when we have more benchmarks (ideally, across different LLVM parts).

[sammccall]

In D52030#1233323, @kbobyrev wrote:

Looks good, thanks!

While we're here: I'm wondering whether we should also introduce very basic test which would just run clangd-indexer since it doesn't depend on benchmarks and would be run on all buildbots.

Isn't that this test?

Also, I was thinking whether we should ask some buildbot owners to enable LLVM_BUILD_BENCHMARKS, but it probably makes sense when we have more benchmarks (ideally, across different LLVM parts).

Hmm, I thought that was already the case. Isn't the benchmark stuff only not-built on the windows bots?

[kbobyrev]

In D52030#1233349, @sammccall wrote:

In D52030#1233323, @kbobyrev wrote:

Looks good, thanks!

While we're here: I'm wondering whether we should also introduce very basic test which would just run clangd-indexer since it doesn't depend on benchmarks and would be run on all buildbots.

Isn't that this test?

Yes, but this test checks both. As mentioned, buildbots don't build benchmarks by default and hence this testcase is not actually run anywhere at the moment.

Also, I was thinking whether we should ask some buildbot owners to enable LLVM_BUILD_BENCHMARKS, but it probably makes sense when we have more benchmarks (ideally, across different LLVM parts).

Hmm, I thought that was already the case. Isn't the benchmark stuff only not-built on the windows bots?

LLVM_BUILD_BENCHMARKS (which adds benchmarks to the list of default targets) is OFF on all platforms by default (although LLVM_INCLUDE_BENCHMARKS which produces build targets in the first place is enabled on Windows now, thanks to @rnk; it seems that the stage 2 bug we encountered was fixed). Hence, the benchmark library itself is built on all platforms (same as gtest library), but assuming that buildbots run ninja/make this only compiles default targets (which exclude IndexBenchmark due to LLVM_BUILD_BENCHMARKS being OFF by default). Hence, we should either ask buildbot owners to set LLVM_BUILD_BENCHMARKS to ON or don't exclude benchmarks from the list of default targets (which might result in some noise due to the full build time increase and is probably not what we want to do.

[kbobyrev]

In D52030#1233395, @kbobyrev wrote:

In D52030#1233349, @sammccall wrote:

In D52030#1233323, @kbobyrev wrote:

Looks good, thanks!

While we're here: I'm wondering whether we should also introduce very basic test which would just run clangd-indexer since it doesn't depend on benchmarks and would be run on all buildbots.

Isn't that this test?

Yes, but this test checks both. As mentioned, buildbots don't build benchmarks by default and hence this testcase is not actually run anywhere at the moment.

Sorry, that's wrong. It *is* run on every buildbot at the moment, but IndexBenchmark binary doesn't exist on any buildbot IIUC, so the second part of the test is not being run. If you add the dependency on LLVM_BUILD_BENCHMARKS, then the whole test would be disabled.+

test/CMakeLists.txt

list(APPEND CLANG_TOOLS_TEST_DEPS
     c-index-test
     # clang-tidy tests require it.
     clang-headers
 
     clang-tidy
     )
 endif()
 
+# clangd's benchmark has an optional test.
+if(LLVM_BUILD_BENCHMARKS)
+  list(APPEND CLANG_TOOLS_TEST_DEPS
+    IndexBenchmark
+    )
+endif()
+
 set(llvm_utils
   FileCheck count not
   )
 
 foreach(t ${llvm_utils})
   if(TARGET ${t})
     list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
   endif()

test/clangd/index-tools.test

 # 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.
+# FIXME: This is fragile. Can we express the LLVM_BUILD_BENCHMARKS dep directly?
 # RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi