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

[Philip Craig]

================
Comment at: unittests/AST/SourceLocationTest.cpp:95
@@ +94,3 @@
+  if (!Node) {
+    setResult("Could not find node with id \"" + ExpectId + "\"");
+    Verified = false;
----------------
Manuel Klimek wrote:
> Manuel Klimek wrote:
> > Philip Craig wrote:
> > > 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.
> > Oh, I thought gtest prints the full stack trace at the point of the exception failure. .. If it doesn't, I'm happy with your solution.
> And when I say "exception" I mean "expectation" :)
It doesn't have the full stack trace. And actually it's more than just the line number that is wrong; it doesn't include the code being tested either. So I have left it as is (with a small change to how the result is set).


http://llvm-reviews.chandlerc.com/D72