Use matcher's description in AllOf if matcher has no explanation.
PiperOrigin-RevId: 652798234
Change-Id: I8e92248a2d9faf2a5719fe220145ea563acc14ff
diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h
index 3daf617..063ee6c 100644
--- a/googlemock/include/gmock/gmock-matchers.h
+++ b/googlemock/include/gmock/gmock-matchers.h
@@ -1300,48 +1300,34 @@
bool MatchAndExplain(const T& x,
MatchResultListener* listener) const override {
- // This method uses matcher's explanation when explaining the result.
- // However, if matcher doesn't provide one, this method uses matcher's
- // description.
+ // If either matcher1_ or matcher2_ doesn't match x, we only need
+ // to explain why one of them fails.
std::string all_match_result;
- for (const Matcher<T>& matcher : matchers_) {
+
+ for (size_t i = 0; i < matchers_.size(); ++i) {
StringMatchResultListener slistener;
- // Return explanation for first failed matcher.
- if (!matcher.MatchAndExplain(x, &slistener)) {
- const std::string explanation = slistener.str();
- if (!explanation.empty()) {
- *listener << explanation;
+ if (matchers_[i].MatchAndExplain(x, &slistener)) {
+ if (all_match_result.empty()) {
+ all_match_result = slistener.str();
} else {
- *listener << "which doesn't match (" << Describe(matcher) << ")";
+ std::string result = slistener.str();
+ if (!result.empty()) {
+ all_match_result += ", and ";
+ all_match_result += result;
+ }
}
- return false;
- }
- // Keep track of explanations in case all matchers succeed.
- std::string explanation = slistener.str();
- if (explanation.empty()) {
- explanation = Describe(matcher);
- }
- if (all_match_result.empty()) {
- all_match_result = explanation;
} else {
- if (!explanation.empty()) {
- all_match_result += ", and ";
- all_match_result += explanation;
- }
+ *listener << slistener.str();
+ return false;
}
}
+ // Otherwise we need to explain why *both* of them match.
*listener << all_match_result;
return true;
}
private:
- // Returns matcher description as a string.
- std::string Describe(const Matcher<T>& matcher) const {
- StringMatchResultListener listener;
- matcher.DescribeTo(listener.stream());
- return listener.str();
- }
const std::vector<Matcher<T>> matchers_;
};
diff --git a/googlemock/test/gmock-matchers-arithmetic_test.cc b/googlemock/test/gmock-matchers-arithmetic_test.cc
index 7521c0d..f176962 100644
--- a/googlemock/test/gmock-matchers-arithmetic_test.cc
+++ b/googlemock/test/gmock-matchers-arithmetic_test.cc
@@ -559,9 +559,10 @@
Matcher<int> m;
// Successful match. Both matchers need to explain. The second
- // matcher doesn't give an explanation, so the matcher description is used.
+ // matcher doesn't give an explanation, so only the first matcher's
+ // explanation is printed.
m = AllOf(GreaterThan(10), Lt(30));
- EXPECT_EQ("which is 15 more than 10, and is < 30", Explain(m, 25));
+ EXPECT_EQ("which is 15 more than 10", Explain(m, 25));
// Successful match. Both matchers need to explain.
m = AllOf(GreaterThan(10), GreaterThan(20));
@@ -571,9 +572,8 @@
// Successful match. All matchers need to explain. The second
// matcher doesn't given an explanation.
m = AllOf(GreaterThan(10), Lt(30), GreaterThan(20));
- EXPECT_EQ(
- "which is 15 more than 10, and is < 30, and which is 5 more than 20",
- Explain(m, 25));
+ EXPECT_EQ("which is 15 more than 10, and which is 5 more than 20",
+ Explain(m, 25));
// Successful match. All matchers need to explain.
m = AllOf(GreaterThan(10), GreaterThan(20), GreaterThan(30));
@@ -588,10 +588,10 @@
EXPECT_EQ("which is 5 less than 10", Explain(m, 5));
// Failed match. The second matcher, which failed, needs to
- // explain. Since it doesn't given an explanation, the matcher text is
+ // explain. Since it doesn't given an explanation, nothing is
// printed.
m = AllOf(GreaterThan(10), Lt(30));
- EXPECT_EQ("which doesn't match (is < 30)", Explain(m, 40));
+ EXPECT_EQ("", Explain(m, 40));
// Failed match. The second matcher, which failed, needs to
// explain.
diff --git a/googlemock/test/gmock-matchers-comparisons_test.cc b/googlemock/test/gmock-matchers-comparisons_test.cc
index 9881155..5b75b45 100644
--- a/googlemock/test/gmock-matchers-comparisons_test.cc
+++ b/googlemock/test/gmock-matchers-comparisons_test.cc
@@ -2334,11 +2334,9 @@
EXPECT_EQ("which is 0 modulo 2, and which is 0 modulo 3", Explain(m, 6));
}
-// Tests that when AllOf() succeeds, but matchers have no explanation,
-// the matcher description is used.
TEST(ExplainMatchResultTest, AllOf_True_True_2) {
const Matcher<int> m = AllOf(Ge(2), Le(3));
- EXPECT_EQ("is >= 2, and is <= 3", Explain(m, 2));
+ EXPECT_EQ("", Explain(m, 2));
}
INSTANTIATE_GTEST_MATCHER_TEST_P(ExplainmatcherResultTest);
