[PATCH] D90291: [clangd] Add lit tests for remote index

Thu Oct 29 07:39:25 PDT 2020

kbobyrev added inline comments.

================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline.test:3
+# RUN: clangd-indexer %/S/../Inputs/remote-index/Source.cpp > %t.in.dex
+# RUN: python %/S/pipeline_helper.py --input-file-name=%s --server-address=0.0.0.0:50051 --test-directory=%/S --index-file=%t.in.dex | FileCheck %s
+# REQUIRES: clangd-remote-index
----------------
ArcsinX wrote:
> kadircet wrote:
> > use `%python` instead to enable lit substitution, just in case there are weird build bots without python on the PATH or the default one pointing at a wrong version.
> > 
> > can we use 127.0.0.1:50051 instead? as starting a server on 0.0.0.0 might be blocked on some bots again. And also I would rather use a random port number (greater than 1024, possibly by letting the python script choose the number, and getting rid of the argument completely), to lower the chances of failing accidentally when 50051 is in use. (it might be because bot is running multiple tests in parallel, or bot itself has a grpc server running on default port for whatever reason).
> > can we use 127.0.0.1:50051 instead?
> I think we should do this, because we also pass this address into `--remote-index-address=` clangd parameter, which is illegal. Seems Linux and Mac allow to call connect with 0.0.0.0 as a destination address, but Windows does not and this test fails.
Thanks for checking! Can you try run this test now and see if it works?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291