[PATCH] D84535: [clangd] Switch from EXPECT_TRUE to ASSERT_TRUE in remote marshalling tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks LGTM!



================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:64
   *Serialized->mutable_location()->mutable_file_path() = std::string();
   Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_FALSE(Deserialized);
----------------
nit: can be directly asserted




================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:70
   WithInvalidURI.Location.FileURI = "This is not a URI";
   Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
   EXPECT_FALSE(Serialized);
----------------
nit: can be directly asserted




================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:79
       Strings.save(UnittestURI->toString()).begin();
   Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
   EXPECT_FALSE(Serialized);
----------------
nit: can be directly asserted


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:87
       "/usr/local/user/home/HelloWorld.cpp";
   Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
   EXPECT_FALSE(Deserialized);
----------------
nit: again can be directly asserted


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:159
   Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_TRUE(Deserialized);
 
----------------
maybe make this an assert too (and also consider issuing ASSERT_FALSE directly on the expression instead of an extra assingment)


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:165
   Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
   EXPECT_FALSE(Deserialized);
 
----------------
same as the previous one


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:171
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
   EXPECT_FALSE(Serialized);
 
----------------
same as the previous one


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:193
   Marshaller WrongMarshaller(testPath("nothome/"), testPath("home/"));
   Serialized = WrongMarshaller.toProtobuf(Sym);
   EXPECT_FALSE(Serialized);
----------------
nit: maybe directly issue `EXPECT_FALSE(WrongMarshaller.toProtobuf(Sym))` instead of going through an extra variable.


================
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:275
             ValidHeaders.size());
-  EXPECT_TRUE(Serialized);
+  ASSERT_TRUE(Serialized);
   auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
----------------
this should be done before the EXPECT_EQ above (which accesses `headers_size()`)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84535/new/

https://reviews.llvm.org/D84535