================
Comment at: unittests/AST/SourceLocationTest.cpp:29
@@ +28,3 @@
+public:
+ MatchVerifier() : ExpectId("") {}
+
----------------
Manuel Klimek wrote:
> 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.
Fixed.
================
Comment at: unittests/AST/SourceLocationTest.cpp:95
@@ +94,3 @@
+ if (!Node) {
+ setResult("Could not find node with id \"" + ExpectId + "\"");
+ Verified = false;
----------------
Manuel Klimek wrote:
> 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(...).
Doing that means that failures no longer print the line number of the test case
that is failing, but in most cases it will be the test case that needs fixing,
not the run() function. You still have the test name, but it's easier to
navigate to a line number. If that is acceptable, then this certainly
simplifies things a lot.
http://llvm-reviews.chandlerc.com/D72
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
