This revision was automatically updated to reflect the committed changes.
Closed by commit rL339781: [clangd][tests] Fix typo in tests - invalid LSP exit
message (authored by jkorous, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D506
jkorous added a comment.
I can see the value of getting more information in case of unexpected test
behaviour but I still don't really like to have separate codepath for testing
purposes.
Anyway, it's not a big deal and it looks like you guys are all in agreement
about this.
I created a patch
ilya-biryukov added a comment.
Haven't noticed Alex's comments, sorry for a duplicate suggestion about exiting
with failure error code
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50641
___
cfe-commits mailing list
cfe-commits@li
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM. Many thanks for fixing this.
Adding some failure monitoring seems like a nice idea. On the other hand,
polluting every test with stderr redirection doesn't look like a nice
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 th
arphaman added a comment.
Thanks for fixing this!
You're right we should try to fix it properly to avoid such mistakes in the
future. Checking for stderr from Clangd might work, but I don't think it's the
most optimal solution. What do you think about Clangd exiting with failure on
malformed J
jkorous created this revision.
jkorous added reviewers: sammccall, ilya-biryukov.
jkorous added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, arphaman, dexonsmith, MaskRay, ioeric.
There's a small typo in tests - causing that we aren't sending exit LSP message
to clangd but
