[cfe-commits] [PATCH] Add unit tests for source locations of AST nodes

Manuel Klimek klimek at google.com
Wed Oct 24 00:46:40 PDT 2012

Comment at: unittests/AST/SourceLocationTest.cpp:29
@@ +28,3 @@
+  MatchVerifier() : ExpectId("") {}
Philip Craig wrote:
> Manuel Klimek wrote:
> > ExpectId seems completely unused now...
> Yes, I left it there in case a test needs to specify its own bind within the matcher. I can delete it for now if you think that's better.
I would delete it. I dislike unused things that "might" be used in the future, as more often than not they end up staying unused.

Comment at: unittests/AST/SourceLocationTest.cpp:95
@@ +94,3 @@
+  if (!Node) {
+    setResult("Could not find node with id \"" + ExpectId + "\"");
+    Verified = false;
Philip Craig wrote:
> Manuel Klimek wrote:
> > Can we instead use EXPECT(...) to fail the test? That way googletest would take care of keeping track of errors etc.
> This result is returned at the end of match(), which is used in an EXPECT(), so it will keep track of the error.
I'm aware - my point is that all the result tracking could be deleted and replaced with one EXPECT here, which would lead to simpler code overall. Instead of EXPECT_TRUE(match(...)), the test could call Verifier.expectMatch(...).


More information about the cfe-commits mailing list