diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 063ee6ca25..3daf617307 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -1300,34 +1300,48 @@ class AllOfMatcherImpl : public MatcherInterface { bool MatchAndExplain(const T& x, MatchResultListener* listener) const override { - // If either matcher1_ or matcher2_ doesn't match x, we only need - // to explain why one of them fails. + // This method uses matcher's explanation when explaining the result. + // However, if matcher doesn't provide one, this method uses matcher's + // description. std::string all_match_result; - - for (size_t i = 0; i < matchers_.size(); ++i) { + for (const Matcher& matcher : matchers_) { StringMatchResultListener slistener; - if (matchers_[i].MatchAndExplain(x, &slistener)) { - if (all_match_result.empty()) { - all_match_result = slistener.str(); + // Return explanation for first failed matcher. + if (!matcher.MatchAndExplain(x, &slistener)) { + const std::string explanation = slistener.str(); + if (!explanation.empty()) { + *listener << explanation; } else { - std::string result = slistener.str(); - if (!result.empty()) { - all_match_result += ", and "; - all_match_result += result; - } + *listener << "which doesn't match (" << Describe(matcher) << ")"; } - } else { - *listener << slistener.str(); 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; + } + } } - // 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& matcher) const { + StringMatchResultListener listener; + matcher.DescribeTo(listener.stream()); + return listener.str(); + } const std::vector> matchers_; }; diff --git a/googlemock/test/gmock-matchers-arithmetic_test.cc b/googlemock/test/gmock-matchers-arithmetic_test.cc index f176962855..7521c0df08 100644 --- a/googlemock/test/gmock-matchers-arithmetic_test.cc +++ b/googlemock/test/gmock-matchers-arithmetic_test.cc @@ -559,10 +559,9 @@ TEST_P(AllOfTestP, ExplainsResult) { Matcher m; // Successful match. Both matchers need to explain. The second - // matcher doesn't give an explanation, so only the first matcher's - // explanation is printed. + // matcher doesn't give an explanation, so the matcher description is used. m = AllOf(GreaterThan(10), Lt(30)); - EXPECT_EQ("which is 15 more than 10", Explain(m, 25)); + EXPECT_EQ("which is 15 more than 10, and is < 30", Explain(m, 25)); // Successful match. Both matchers need to explain. m = AllOf(GreaterThan(10), GreaterThan(20)); @@ -572,8 +571,9 @@ TEST_P(AllOfTestP, ExplainsResult) { // 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 which is 5 more than 20", - Explain(m, 25)); + EXPECT_EQ( + "which is 15 more than 10, and is < 30, 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 @@ TEST_P(AllOfTestP, ExplainsResult) { 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, nothing is + // explain. Since it doesn't given an explanation, the matcher text is // printed. m = AllOf(GreaterThan(10), Lt(30)); - EXPECT_EQ("", Explain(m, 40)); + EXPECT_EQ("which doesn't match (is < 30)", 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 769dc99e86..57c675fa4e 100644 --- a/googlemock/test/gmock-matchers-comparisons_test.cc +++ b/googlemock/test/gmock-matchers-comparisons_test.cc @@ -2333,9 +2333,11 @@ TEST(ExplainMatchResultTest, AllOf_True_True) { 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 m = AllOf(Ge(2), Le(3)); - EXPECT_EQ("", Explain(m, 2)); + EXPECT_EQ("is >= 2, and is <= 3", Explain(m, 2)); } INSTANTIATE_GTEST_MATCHER_TEST_P(ExplainmatcherResultTest);