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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:36
+  ],
+                                          stderr=subprocess.PIPE)
+
----------------
kadircet wrote:
> nit: formatting looks off, is this really what yapf offers?
Yep, I'm formatting with it :(


================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:39
+  # Wait for the server to warm-up.
+  if not b'Server listening' in index_server_process.stderr.readline():
+    sys.exit(1)
----------------
kadircet wrote:
> i would rather scan through all lines and look for `Server listening on $server_addres`. We should be able to either hit the end of stream (i.e. server encounters a failure and shuts down) or find the log message we are looking for.
> 
> just to make test less reliant on log ordering.
Unfortunately, it's not as easy :( Then I would have to set a time to wait for the server to warm up and read lines then only. The problem here is that the server runs, doesn't shut down but also doesn't print the message. We'll hang the buildbots then.

But I'll go with the timeout option. I don't think it's better than checking the first line as the initialize one but still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291