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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a subscriber: sammccall.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm! but I would wait for a while to see if someone(that remembers why this test was specifically asserting for built-in headers) will object.



================
Comment at: clang-tools-extra/clangd/test/document-link.test:1
-# for %resource_dir: REQUIRES: clang
-# %resource_dir actually points at builtin_include_dir, go up one directory.
-# RUN: clangd -lit-test -resource-dir=%resource_dir/.. < %s | FileCheck -strict-whitespace %s
+
+# create a fake resource_dir so that the test can find the headers.
----------------
nit: drop empty line


================
Comment at: clang-tools-extra/clangd/test/document-link.test:2
+
+# create a fake resource_dir so that the test can find the headers.
+# RUN: mkdir -p %t/include/ && touch %t/include/foo.h
----------------
s/create/Create/


================
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
----------------
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`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95670