[PATCH] D91330: [clangd] Ensure we test for compatibility of serialized index format

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Very nice, thanks for doing this!



================
Comment at: clang-tools-extra/clangd/test/index-serialization/version-is-correct.test:1
+# If this test fails it means there has been a backward incompatilbe change to
+# serialization format. Please bump the version number in
----------------
I think this test is REQUIRES: zlib
(see windows failures)


================
Comment at: clang-tools-extra/clangd/test/index-serialization/version-is-correct.test:1
+# If this test fails it means there has been a backward incompatilbe change to
+# serialization format. Please bump the version number in
----------------
sammccall wrote:
> I think this test is REQUIRES: zlib
> (see windows failures)
nit: maybe say a bit more about what the test does? And separate what the test does vs what failure means vs what action to take a little.

This test tries to parse a checked-in binary index.
If it fails, there has been...
Please bump the version number...


================
Comment at: clang-tools-extra/clangd/test/index-serialization/version-is-correct.test:3
+# serialization format. Please bump the version number in
+# clang-tools-extra/clangd/index/Serialization.cpp and regenarate the sample.idx
+# with
----------------
nit: regenerate sample.idx


================
Comment at: clang-tools-extra/clangd/test/index-serialization/version-is-correct.test:5
+# with
+# clangd-indexer \
+# clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp > \
----------------
this isn't easy to copy/paste because of the comment markers.
This file isn't interpreted so there's no need for them, I think...

It could also be formatted so it's easier to recognize the command (surround by blank lines, indent by a couple of spaces?)


================
Comment at: clang-tools-extra/clangd/test/index-serialization/version-is-correct.test:10
+# indexing sample.cpp would yield non-trivial values for those.
+# RUN: dexp %/S/Inputs/sample.idx
+
----------------
`dexp ... -c="find B" | grep Bar`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91330