[PATCH] D50641: [clangd][test] Fix exit messages in tests

Jan Korous via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 14 03:37:49 PDT 2018


jkorous added a comment.

I think that by introducing different codepath for testing purposes we might end up with fewer bugs in tests but the actual production code could become less tested. Actually, even the -lit-test itself might be not the theoretically most correct approach but I do see the practical reason for it. In general I'd rather keep the testing specific code to a minimum though.

What we might be able to do specifically for this case is to return false iff we hit unexpected EOF in clangd::runLanguageServerLoop() (currently void) and || it with ShutdownRequestReceived in ClangdLSPServer::run().

I still see some value in monitoring clangd's stderr in tests in general though. I can imagine something simple like redirection of clangd's stderr to a file and just grepping the log for lines starting with "E[" and in case of some set of errors being expected/allowed for specific test case then grepping for negation (grep -v expected_error). There might be some work necessary to either recalibrate log levels or add allowed errors to large part of tests. For example clangd currently logs this error for most tests:

  E[11:24:49.910] Failed to get status for RootPath clangd: No such file or directory


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50641





More information about the cfe-commits mailing list