[PATCH] D72914: [FileCheck] Strengthen error checks in unit tests

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:206
+  handleAllErrors(std::move(Err), [&](const ErrorT &E) {
+    EXPECT_NE(E.message().find(ExpectedMsg.str()), std::string::npos);
+  });
----------------
jhenderson wrote:
> thopre wrote:
> > jhenderson wrote:
> > > Would `EXPECT_EQ(ExpectedMsg, E.message())`  be more precise?
> > A lot of the diagnostics span several lines because they come with a location information. Matching the exact message means you'd have to provide all this in the string to check against. I felt that the error message itself was enough to check which requires a substring match.
> > 
> > What do you think?
> Okay, that's fine. No need to add the additional context in most cases (though you might want a single test that shows location information too).
Is the intent of a single location check to verify that a given type of error provide location information or to check for a given error that the location is correct?

If the former I can perhaps use some regex check in expectError and check for a location info for all messages. I'm not sure what we gain if the intent is the latter. Besides, location information in error message is generally already tested in the standard LIT testsuite.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72914