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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to