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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:42
+  found_init_message = False
+  for line in index_server_process.stderr:
+    if b'Server listening' in line:
----------------
so if we think `.readline()` is blocking but this is not, can't we just keep calling readline instead?


================
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)
----------------
kbobyrev wrote:
> 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.
> 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

Why? isn't the `readline()` on `stderr` blocking ? If not how was this code working without sleep before?

>  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.

If the argument above is correct, this was the case already, i.e. stderr.readline() would block until a line is available or server had shutdown.

To overcome this problem I would suggest having a separate thread that would kill the server if it is still running after N seconds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291