[PATCH] D95670: [clangd] Don't rely on builtin headers for document-link.test.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Thanks for fixing this, we don't want to depend on clang and I didn't realize we still did!



================
Comment at: clang-tools-extra/clangd/test/document-link.test:3
+# create a fake resource_dir so that the test can find the headers.
+# RUN: mkdir -p %t/include/ && touch %t/include/foo.h
+# RUN: clangd -lit-test -resource-dir=%t < %s | FileCheck -strict-whitespace %s
----------------
kadircet wrote:
> this is clever! I believe there was a reason for this test to be asserting built-in headers (I can't seem to remember, maybe @sammccall does), but asserting through a mock header should also be fine, I suppose.
> 
> one thing though, we need to clean up `%t`, e.g. `rm -rf %t`.
> 
> (and regrading the failure at head, resource_dir path has changed with release cut from `something/12.0.0` to `something/13.0.0` so it should go away once you build new clang via `ninja clang`)
> this is clever!
Indeed, I almost wish we had a less-clever way to do this but I can't think of one :-)

> I believe there was a reason for this test to be asserting built-in headers (I can't seem to remember, maybe @sammccall does), but asserting through a mock header should also be fine, I suppose.

Nah, I think it was just to avoid complex setup (and builtin over stdlib as it's a somewhat less crazy dep).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95670/new/

https://reviews.llvm.org/D95670