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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
