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

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 1 23:38:27 PST 2020

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list