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

Jan Korous via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 13 09:06:29 PDT 2018


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 crashing it instead with the last message not being valid JSON and clangd hitting unexpected EOF in runLanguageServerLoop() right after. Seems like this bug even managed to spawn some offsprings via copy-pasting. I just found it by accident since I am not closing stdin after the last message when using XPC adapter and my test was hanging - waiting for clangd to exit.

It's not a big deal but I got two ideas.

1. Maybe we should somehow check stderr from clangd - in this case there was JSON parse error present but tests didn't notice.
2. When fixing that I got surprised by shutdown-without-exit.test implementation. It seems more like exit-without-shutdown to me and makes sense that we still want to exit (with non-success return value) in such case. Was this the intention?

What do you think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50641

Files:
  clangd/extra-flags.test
  clangd/formatting.test
  clangd/initialize-params.test
  clangd/shutdown-with-exit.test
  clangd/shutdown-without-exit.test
  clangd/unsupported-method.test


Index: clangd/unsupported-method.test
===================================================================
--- clangd/unsupported-method.test
+++ clangd/unsupported-method.test
@@ -13,4 +13,4 @@
 ---
 {"jsonrpc":"2.0","id":2,"method":"shutdown"}
 ---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-without-exit.test
===================================================================
--- clangd/shutdown-without-exit.test
+++ clangd/shutdown-without-exit.test
@@ -1,2 +1,2 @@
 # RUN: not clangd -lit-test < %s
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-with-exit.test
===================================================================
--- clangd/shutdown-with-exit.test
+++ clangd/shutdown-with-exit.test
@@ -1,4 +1,4 @@
 # RUN: clangd -lit-test < %s
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 ---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/initialize-params.test
===================================================================
--- clangd/initialize-params.test
+++ clangd/initialize-params.test
@@ -46,4 +46,4 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": null
 ---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/formatting.test
===================================================================
--- clangd/formatting.test
+++ clangd/formatting.test
@@ -184,4 +184,4 @@
 ---
 {"jsonrpc":"2.0","id":6,"method":"shutdown"}
 ---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/extra-flags.test
===================================================================
--- clangd/extra-flags.test
+++ clangd/extra-flags.test
@@ -47,6 +47,6 @@
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
 
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50641.160367.patch
Type: text/x-patch
Size: 1900 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180813/6eda612c/attachment.bin>


More information about the cfe-commits mailing list