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