Remove short-circuiting from AllOf, for better failure messages
For `EXPECT_THAT` matcher usage, showing only the first failure meant that users would sometimes have to make a fix and run the test again only to notice that there's another failure. It's better to show more failures so that the user can fix several issues in one go. In practice, very little code actually wants the short circuiting here, only a handful of cases with custom matchers used like `AllOf(BoundsCheck(), UncheckedAccess())`. These cases are fixable by refactoring `UncheckedAccess()` to instead also apply a bounds check to fail the matcher rather than crash. Notably, this change doesn't affect `AnyOf`, so another workaround is to change `AllOf(m1, m2, ...)` into `Not(AnyOf(Not(m1), Not(m2), ...))`. PiperOrigin-RevId: 826316273 Change-Id: Ie8186f75c10443d8da35b5d07b6a8cd9ae85b451
This commit is contained in:
committed by
Copybara-Service
parent
4fe3307fb2
commit
17d335d7c7
@@ -1329,17 +1329,23 @@ class AllOfMatcherImpl : public MatcherInterface<const T&> {
|
||||
// However, if matcher doesn't provide one, this method uses matcher's
|
||||
// description.
|
||||
std::string all_match_result;
|
||||
bool success = true;
|
||||
for (const Matcher<T>& matcher : matchers_) {
|
||||
StringMatchResultListener slistener;
|
||||
// Return explanation for first failed matcher.
|
||||
if (!matcher.MatchAndExplain(x, &slistener)) {
|
||||
const std::string explanation = slistener.str();
|
||||
if (!success) {
|
||||
// Already had a failure.
|
||||
*listener << ", and ";
|
||||
}
|
||||
if (!explanation.empty()) {
|
||||
*listener << explanation;
|
||||
} else {
|
||||
*listener << "which doesn't match (" << Describe(matcher) << ")";
|
||||
}
|
||||
return false;
|
||||
success = false;
|
||||
continue;
|
||||
}
|
||||
// Keep track of explanations in case all matchers succeed.
|
||||
std::string explanation = slistener.str();
|
||||
@@ -1356,8 +1362,10 @@ class AllOfMatcherImpl : public MatcherInterface<const T&> {
|
||||
}
|
||||
}
|
||||
|
||||
*listener << all_match_result;
|
||||
return true;
|
||||
if (success) {
|
||||
*listener << all_match_result;
|
||||
}
|
||||
return success;
|
||||
}
|
||||
|
||||
private:
|
||||
|
||||
Reference in New Issue
Block a user