From 9ac4cd0f4958608961f9b5ca27b99fe696e1dd27 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Sat, 20 Jun 2020 11:41:25 +0300 Subject: [PATCH 01/10] Add matchers for testing exception properties This PR adds matchers that accept a callable and verify that when invoked, it throws an exception with the given type and properties. Fixes #952 --- googlemock/include/gmock/gmock-matchers.h | 134 ++++++++++++++++ googlemock/test/gmock-matchers_test.cc | 182 ++++++++++++++++++++++ 2 files changed, 316 insertions(+) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 0bc9b3649f..43456c253b 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4725,6 +4725,140 @@ PolymorphicMatcher > VariantWith( internal::variant_matcher::VariantMatcher(matcher)); } + +#if GTEST_HAS_EXCEPTIONS + +// Anything inside the 'internal' namespace IS INTERNAL IMPLEMENTATION +// and MUST NOT BE USED IN USER CODE!!! +namespace internal { + +template +class ExceptionMatcherImpl { + public: + ExceptionMatcherImpl(Matcher matcher) + : matcher_(std::move(matcher)) {} + + public: + void DescribeTo(::std::ostream* os) const { + *os << "throws an exception of type " << GetTypeName(); + if (matcher_.GetDescriber() != nullptr) { + *os << " which "; + matcher_.DescribeTo(os); + } + } + + void DescribeNegationTo(::std::ostream* os) const { + *os << "not ("; + DescribeTo(os); + *os << ")"; + } + + template + bool MatchAndExplain(T&& x, MatchResultListener* listener) const { + try { + (void)(std::forward(x)()); + } catch (const Err& err) { + *listener << "throws an exception of type " << GetTypeName(); + if (matcher_.GetDescriber() != nullptr) { + *listener << " "; + return matcher_.MatchAndExplain(err, listener); + } else { + return true; + } + } catch (const std::exception& err) { +#if GTEST_HAS_RTTI + *listener << "throws an exception of type " + << GetTypeName(typeid(err)) << " "; +#else + *listener << "throws an std::exception-derived error "; +#endif + *listener << "with description \"" << err.what() << "\""; + return false; + } catch (...) { + *listener << "throws an exception of some other type"; + return false; + } + *listener << "does not throw any exception"; + return false; + } + + private: + const Matcher matcher_; +}; + +} // namespace internal + +// Throws() +// Throws(exceptionMatcher) +// ThrowsMessage(messageMatcher) +// ThrowsMessageHasSubstr(message) +// +// This matcher accepts a callable and verifies that when invoked, it throws +// an exception with the given type and properties. +// +// Examples: +// +// EXPECT_THAT( +// []() { throw std::runtime_error("message"); }, +// Throws()); +// +// EXPECT_THAT( +// []() { throw std::runtime_error("message"); }, +// ThrowsMessage(HasSubstr("message"))); +// +// EXPECT_THAT( +// []() { throw std::runtime_error("message"); }, +// ThrowsMessageHasSubstr("message")); +// +// EXPECT_THAT( +// []() { throw std::runtime_error("message"); }, +// Throws + +template +PolymorphicMatcher> +Throws() { + return MakePolymorphicMatcher( + internal::ExceptionMatcherImpl{ + Matcher{}}); +} +template +PolymorphicMatcher> +Throws(const ExceptionMatcher& exceptionMatcher) { + // Using matcher cast allows users to pass a matcher of a more broad type. + // For example user may want to pass Matcher + // to Throws, or Matcher to Throws. + return MakePolymorphicMatcher( + internal::ExceptionMatcherImpl{ + SafeMatcherCast(exceptionMatcher)}); +} +template +PolymorphicMatcher> +ThrowsMessage(const MessageMatcher& messageMatcher) { + static_assert( + std::is_base_of::value, + "expected an std::exception-derived class"); + // We cast matcher to std::string so that we have string semantics instead of + // pointer semantics. With this cast, we can accept matchers that match types + // that're constructible from strings. Also, we can accept raw string + // literals, e.g. ThrowsMessage("message"). + return MakePolymorphicMatcher( + internal::ExceptionMatcherImpl{ + Property("description", &std::exception::what, + MatcherCast(messageMatcher))}); +} +template +PolymorphicMatcher> +ThrowsMessageHasSubstr(const internal::StringLike& message) { + static_assert( + std::is_base_of::value, + "expected an std::exception-derived class"); + return MakePolymorphicMatcher( + internal::ExceptionMatcherImpl{ + Property("description", &std::exception::what, HasSubstr(message))}); +} + +#endif // GTEST_HAS_EXCEPTIONS + // These macros allow using matchers to check values in Google Test // tests. ASSERT_THAT(value, matcher) and EXPECT_THAT(value, matcher) // succeed if and only if the value matches the matcher. If the assertion diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index 015bb09a85..db2e043043 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8117,6 +8117,188 @@ TEST(MatcherPMacroTest, WorksOnMoveOnlyType) { EXPECT_THAT(p, Not(UniquePointee(2))); } +#if GTEST_HAS_EXCEPTIONS + +TEST(ThrowsTest, Describe) { + Matcher matcher = Throws(); + std::stringstream ss; + matcher.DescribeTo(&ss); + auto explanation = ss.str(); + EXPECT_THAT(explanation, testing::HasSubstr("std::runtime_error")); +} + +TEST(ThrowsTest, Success) { + Matcher matcher = Throws(); + StringMatchResultListener listener; + EXPECT_TRUE( + matcher.MatchAndExplain( + []() { throw std::runtime_error("error message"); }, &listener)); + EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); +} + +TEST(ThrowsTest, FailWrongType) { + Matcher matcher = Throws(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { throw std::logic_error("error message"); }, &listener)); + EXPECT_THAT(listener.str(), testing::HasSubstr("std::logic_error")); + EXPECT_THAT(listener.str(), testing::HasSubstr("\"error message\"")); +} + +TEST(ThrowsTest, FailWrongTypeNonStd) { + Matcher matcher = Throws(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { throw 10; }, &listener)); + EXPECT_THAT( + listener.str(), + testing::HasSubstr("throws an exception of some other type")); +} + +TEST(ThrowsTest, FailNoThrow) { + Matcher matcher = Throws(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { (void)0; }, &listener)); + EXPECT_THAT( + listener.str(), + testing::HasSubstr("does not throw any exception")); +} + +class ThrowsPredicateTest: public TestWithParam> {}; + +TEST_P(ThrowsPredicateTest, Describe) { + Matcher matcher = GetParam(); + std::stringstream ss; + matcher.DescribeTo(&ss); + auto explanation = ss.str(); + EXPECT_THAT(explanation, testing::HasSubstr("std::runtime_error")); + EXPECT_THAT(explanation, testing::HasSubstr("error message")); +} + +TEST_P(ThrowsPredicateTest, Success) { + Matcher matcher = GetParam(); + StringMatchResultListener listener; + EXPECT_TRUE( + matcher.MatchAndExplain( + []() { throw std::runtime_error("error message"); }, &listener)); + EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); +} + +TEST_P(ThrowsPredicateTest, FailWrongType) { + Matcher matcher = GetParam(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { throw std::logic_error("error message"); }, &listener)); + EXPECT_THAT(listener.str(), testing::HasSubstr("std::logic_error")); + EXPECT_THAT(listener.str(), testing::HasSubstr("\"error message\"")); +} + +TEST_P(ThrowsPredicateTest, FailWrongTypeNonStd) { + Matcher matcher = GetParam(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { throw 10; }, &listener)); + EXPECT_THAT( + listener.str(), + testing::HasSubstr("throws an exception of some other type")); +} + +TEST_P(ThrowsPredicateTest, FailWrongMessage) { + Matcher matcher = GetParam(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { throw std::runtime_error("wrong message"); }, &listener)); + EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); + EXPECT_THAT(listener.str(), testing::HasSubstr("wrong message")); +} + +TEST_P(ThrowsPredicateTest, FailNoThrow) { + Matcher matcher = GetParam(); + StringMatchResultListener listener; + EXPECT_FALSE( + matcher.MatchAndExplain( + []() { (void)0; }, &listener)); + EXPECT_THAT( + listener.str(), + testing::HasSubstr("does not throw any exception")); +} + +INSTANTIATE_TEST_SUITE_P(AllMessagePredicates, ThrowsPredicateTest, + ::testing::Values( + static_cast>( + Throws( + Property(&std::exception::what, HasSubstr("error message")))), + static_cast>( + ThrowsMessage(HasSubstr("error message"))), + static_cast>( + ThrowsMessageHasSubstr("error message")))); + +// Tests that Throws(Matcher{}) compiles even when E2 != const E1&. +TEST(ThrowsPredicateCompilesTest, ExceptionMatcherAcceptsBroadType) { + { + Matcher inner = + Property(&std::exception::what, HasSubstr("error message")); + Matcher matcher = Throws(inner); + EXPECT_TRUE( + matcher.Matches([]() { throw std::runtime_error("error message"); })); + EXPECT_FALSE( + matcher.Matches([]() { throw std::runtime_error("wrong message"); })); + } + + { + Matcher inner = Eq(10); + Matcher matcher = Throws(inner); + EXPECT_TRUE( + matcher.Matches([]() { throw (uint32_t)10; })); + EXPECT_FALSE( + matcher.Matches([]() { throw (uint32_t)11; })); + } +} + +// Tests that ThrowsMessage("message") is equivalent +// to ThrowsMessage(Eq("message")). +TEST(ThrowsPredicateCompilesTest, MessageMatcherAcceptsNonMatcher) { + Matcher matcher = ThrowsMessage( + "error message"); + EXPECT_TRUE( + matcher.Matches( + []() { throw std::runtime_error("error message"); })); + EXPECT_FALSE( + matcher.Matches( + []() { throw std::runtime_error("wrong error message"); })); +} + +// Tests that ThrowsMessageHasSubstr accepts types that're +// explicitly-convertible to std::string. +TEST(ThrowsPredicateCompilesTest, StringLikeMessage) { + struct SomeCustomString { + std::string inner; + + // Note: explicit conversion. + explicit operator std::string() const { + return inner; + } +}; + + Matcher matcher = ThrowsMessageHasSubstr( + SomeCustomString{"error message"}); + EXPECT_TRUE( + matcher.Matches( + []() { throw std::runtime_error("error message"); })); + EXPECT_FALSE( + matcher.Matches( + []() { throw std::runtime_error("wrong message"); })); +} + +#endif // GTEST_HAS_EXCEPTIONS + } // namespace } // namespace gmock_matchers_test } // namespace testing From 46734d9a66e858e2b419729b52d6aa21faa614c7 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Sat, 20 Jun 2020 16:38:55 +0300 Subject: [PATCH 02/10] Small improvements: code style and property name --- googlemock/include/gmock/gmock-matchers.h | 4 ++-- googlemock/test/gmock-matchers_test.cc | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 43456c253b..cc045648cc 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4843,7 +4843,7 @@ ThrowsMessage(const MessageMatcher& messageMatcher) { // literals, e.g. ThrowsMessage("message"). return MakePolymorphicMatcher( internal::ExceptionMatcherImpl{ - Property("description", &std::exception::what, + Property("what", &std::exception::what, MatcherCast(messageMatcher))}); } template @@ -4854,7 +4854,7 @@ ThrowsMessageHasSubstr(const internal::StringLike& message) { "expected an std::exception-derived class"); return MakePolymorphicMatcher( internal::ExceptionMatcherImpl{ - Property("description", &std::exception::what, HasSubstr(message))}); + Property("what", &std::exception::what, HasSubstr(message))}); } #endif // GTEST_HAS_EXCEPTIONS diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index db2e043043..dd61a53918 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8282,10 +8282,8 @@ TEST(ThrowsPredicateCompilesTest, StringLikeMessage) { std::string inner; // Note: explicit conversion. - explicit operator std::string() const { - return inner; - } -}; + explicit operator std::string() const { return inner; } + }; Matcher matcher = ThrowsMessageHasSubstr( SomeCustomString{"error message"}); From 49d1201a7ec83987deaae161958577deeba1a9ed Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Fri, 3 Jul 2020 20:50:06 +0300 Subject: [PATCH 03/10] Add missing documentation piece --- googlemock/include/gmock/gmock-matchers.h | 3 ++- googlemock/test/gmock-matchers_test.cc | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index cc045648cc..708c7c8700 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4812,7 +4812,8 @@ class ExceptionMatcherImpl { // // EXPECT_THAT( // []() { throw std::runtime_error("message"); }, -// Throws +// Throws( +// Property(&std::runtime_error::what, HasSubstr("message")))); template PolymorphicMatcher> diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index dd61a53918..0741187389 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8119,6 +8119,26 @@ TEST(MatcherPMacroTest, WorksOnMoveOnlyType) { #if GTEST_HAS_EXCEPTIONS +// Test that examples from documentation compile +TEST(ThrowsTest, Examples) { + EXPECT_THAT( + []() { throw std::runtime_error("message"); }, + Throws()); + + EXPECT_THAT( + []() { throw std::runtime_error("message"); }, + ThrowsMessage(HasSubstr("message"))); + + EXPECT_THAT( + []() { throw std::runtime_error("message"); }, + ThrowsMessageHasSubstr("message")); + + EXPECT_THAT( + []() { throw std::runtime_error("message"); }, + Throws( + Property(&std::runtime_error::what, HasSubstr("message")))); +} + TEST(ThrowsTest, Describe) { Matcher matcher = Throws(); std::stringstream ss; From 69c510fb5137160b532e1c7e9bec9e660c14b193 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Tue, 7 Jul 2020 19:01:58 +0300 Subject: [PATCH 04/10] Add a test for duplicate catch clauses in throw matchers, fix a couple of nitpicks. --- googlemock/include/gmock/gmock-matchers.h | 3 +-- googlemock/test/gmock-matchers_test.cc | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 708c7c8700..36625209ec 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4738,7 +4738,6 @@ class ExceptionMatcherImpl { ExceptionMatcherImpl(Matcher matcher) : matcher_(std::move(matcher)) {} - public: void DescribeTo(::std::ostream* os) const { *os << "throws an exception of type " << GetTypeName(); if (matcher_.GetDescriber() != nullptr) { @@ -4775,7 +4774,7 @@ class ExceptionMatcherImpl { *listener << "with description \"" << err.what() << "\""; return false; } catch (...) { - *listener << "throws an exception of some other type"; + *listener << "throws an exception of an unknown type"; return false; } *listener << "does not throw any exception"; diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index 0741187389..cd89d81e02 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8139,6 +8139,12 @@ TEST(ThrowsTest, Examples) { Property(&std::runtime_error::what, HasSubstr("message")))); } +TEST(ThrowsTest, DoesNotGenerateDuplicateCatchClauseWarning) { + EXPECT_THAT( + []() { throw std::exception(); }, + Throws()); +} + TEST(ThrowsTest, Describe) { Matcher matcher = Throws(); std::stringstream ss; From 92d0a6f7e2dafe49377669884e665026fc1aa376 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Tue, 7 Jul 2020 19:09:53 +0300 Subject: [PATCH 05/10] Add a test to ensure that the `Throws` matcher only invokes its argument once. --- googlemock/test/gmock-matchers_test.cc | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index cd89d81e02..21ade890fe 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8145,6 +8145,36 @@ TEST(ThrowsTest, DoesNotGenerateDuplicateCatchClauseWarning) { Throws()); } +TEST(ThrowsTest, CallableExecutedExactlyOnce) { + size_t a = 0; + + EXPECT_THAT( + [&a]() { a++; throw 10; }, + Throws()); + EXPECT_EQ(a, 1); + + EXPECT_THAT( + [&a]() { a++; throw std::runtime_error("message"); }, + Throws()); + EXPECT_EQ(a, 2); + + EXPECT_THAT( + [&a]() { a++; throw std::runtime_error("message"); }, + ThrowsMessage(HasSubstr("message"))); + EXPECT_EQ(a, 3); + + EXPECT_THAT( + [&a]() { a++; throw std::runtime_error("message"); }, + ThrowsMessageHasSubstr("message")); + EXPECT_EQ(a, 4); + + EXPECT_THAT( + [&a]() { a++; throw std::runtime_error("message"); }, + Throws( + Property(&std::runtime_error::what, HasSubstr("message")))); + EXPECT_EQ(a, 5); +} + TEST(ThrowsTest, Describe) { Matcher matcher = Throws(); std::stringstream ss; From 0a80845e73d5b6c87c87eec6b5777224542d8bd9 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Tue, 7 Jul 2020 19:28:24 +0300 Subject: [PATCH 06/10] Fix build under msvc --- googlemock/include/gmock/gmock-matchers.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 36625209ec..f2e47efa47 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4734,6 +4734,13 @@ namespace internal { template class ExceptionMatcherImpl { + class NeverThrown { + public: + const char* what() const noexcept { + return "this exception should never be thrown"; + } + }; + public: ExceptionMatcherImpl(Matcher matcher) : matcher_(std::move(matcher)) {} @@ -4764,7 +4771,14 @@ class ExceptionMatcherImpl { } else { return true; } - } catch (const std::exception& err) { + } catch ( + typename std::conditional< + std::is_same< + typename std::remove_cv< + typename std::remove_reference::type>::type, + std::exception>::value, + const NeverThrown&, + const std::exception&>::type const& err) { #if GTEST_HAS_RTTI *listener << "throws an exception of type " << GetTypeName(typeid(err)) << " "; From c46bdea43ace31b3f62ccffc60387e7e329dc190 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Tue, 7 Jul 2020 19:30:37 +0300 Subject: [PATCH 07/10] Update tests after changing an error message --- googlemock/test/gmock-matchers_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index 21ade890fe..63cd99b9a6 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8210,7 +8210,7 @@ TEST(ThrowsTest, FailWrongTypeNonStd) { []() { throw 10; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("throws an exception of some other type")); + testing::HasSubstr("throws an exception of an unknown type")); } TEST(ThrowsTest, FailNoThrow) { @@ -8262,7 +8262,7 @@ TEST_P(ThrowsPredicateTest, FailWrongTypeNonStd) { []() { throw 10; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("throws an exception of some other type")); + testing::HasSubstr("throws an exception of an unknown type")); } TEST_P(ThrowsPredicateTest, FailWrongMessage) { From 4ebbfea623212704fccfe2cbf79bba0bc8fb6770 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Tue, 7 Jul 2020 20:00:09 +0300 Subject: [PATCH 08/10] Fix build under msvc --- googlemock/test/gmock-matchers_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index 63cd99b9a6..a8b39b8f6b 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8151,28 +8151,28 @@ TEST(ThrowsTest, CallableExecutedExactlyOnce) { EXPECT_THAT( [&a]() { a++; throw 10; }, Throws()); - EXPECT_EQ(a, 1); + EXPECT_EQ(a, 1u); EXPECT_THAT( [&a]() { a++; throw std::runtime_error("message"); }, Throws()); - EXPECT_EQ(a, 2); + EXPECT_EQ(a, 2u); EXPECT_THAT( [&a]() { a++; throw std::runtime_error("message"); }, ThrowsMessage(HasSubstr("message"))); - EXPECT_EQ(a, 3); + EXPECT_EQ(a, 3u); EXPECT_THAT( [&a]() { a++; throw std::runtime_error("message"); }, ThrowsMessageHasSubstr("message")); - EXPECT_EQ(a, 4); + EXPECT_EQ(a, 4u); EXPECT_THAT( [&a]() { a++; throw std::runtime_error("message"); }, Throws( Property(&std::runtime_error::what, HasSubstr("message")))); - EXPECT_EQ(a, 5); + EXPECT_EQ(a, 5u); } TEST(ThrowsTest, Describe) { From a899cecb11d35a41393456be3bc5ed8a8c815da8 Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Mon, 13 Jul 2020 20:22:55 +0300 Subject: [PATCH 09/10] Cleanup a bulky expression, document implementation details --- googlemock/include/gmock/gmock-matchers.h | 38 ++++++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index f2e47efa47..969d9202cf 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4741,6 +4741,35 @@ class ExceptionMatcherImpl { } }; + // If the matchee raises an exception of a wrong type, we'd like to + // catch it and print its message and type. To do that, we add an additional + // catch clause: + // + // try { ... } + // catch (const Err&) { /* an expected exception */ } + // catch (const std::exception&) { /* exception of a wrong type */ } + // + // However, if the `Err` itself is `std::exception`, we'd end up with two + // identical `catch` clauses: + // + // try { ... } + // catch (const std::exception&) { /* an expected exception */ } + // catch (const std::exception&) { /* exception of a wrong type */ } + // + // This can cause a warning or an error in some compilers. To resolve + // the issue, we use a fake error type whenever `Err` is `std::exception`: + // + // try { ... } + // catch (const std::exception&) { /* an expected exception */ } + // catch (const NeverThrown&) { /* exception of a wrong type */ } + using DefaultExceptionType = typename std::conditional< + std::is_same< + typename std::remove_cv< + typename std::remove_reference::type>::type, + std::exception>::value, + const NeverThrown&, + const std::exception&>::type; + public: ExceptionMatcherImpl(Matcher matcher) : matcher_(std::move(matcher)) {} @@ -4771,14 +4800,7 @@ class ExceptionMatcherImpl { } else { return true; } - } catch ( - typename std::conditional< - std::is_same< - typename std::remove_cv< - typename std::remove_reference::type>::type, - std::exception>::value, - const NeverThrown&, - const std::exception&>::type const& err) { + } catch (DefaultExceptionType err) { #if GTEST_HAS_RTTI *listener << "throws an exception of type " << GetTypeName(typeid(err)) << " "; From 7f1c8bb44718d68cb96a3eea033856b8c0ad9a3d Mon Sep 17 00:00:00 2001 From: Vladimir Goncharov Date: Mon, 3 Aug 2020 23:44:27 +0300 Subject: [PATCH 10/10] Remove ThrowsMessageHasSubstr and fix some nits after review --- googlemock/include/gmock/gmock-matchers.h | 23 ++------ googlemock/test/gmock-matchers_test.cc | 67 ++++++----------------- 2 files changed, 22 insertions(+), 68 deletions(-) diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h index 969d9202cf..59ef2f3d56 100644 --- a/googlemock/include/gmock/gmock-matchers.h +++ b/googlemock/include/gmock/gmock-matchers.h @@ -4774,15 +4774,15 @@ class ExceptionMatcherImpl { ExceptionMatcherImpl(Matcher matcher) : matcher_(std::move(matcher)) {} - void DescribeTo(::std::ostream* os) const { - *os << "throws an exception of type " << GetTypeName(); + void DescribeTo(std::ostream* os) const { + *os << "throws an exception which is a " << GetTypeName(); if (matcher_.GetDescriber() != nullptr) { *os << " which "; matcher_.DescribeTo(os); } } - void DescribeNegationTo(::std::ostream* os) const { + void DescribeNegationTo(std::ostream* os) const { *os << "not ("; DescribeTo(os); *os << ")"; @@ -4793,7 +4793,7 @@ class ExceptionMatcherImpl { try { (void)(std::forward(x)()); } catch (const Err& err) { - *listener << "throws an exception of type " << GetTypeName(); + *listener << "throws an exception which is a " << GetTypeName(); if (matcher_.GetDescriber() != nullptr) { *listener << " "; return matcher_.MatchAndExplain(err, listener); @@ -4826,7 +4826,6 @@ class ExceptionMatcherImpl { // Throws() // Throws(exceptionMatcher) // ThrowsMessage(messageMatcher) -// ThrowsMessageHasSubstr(message) // // This matcher accepts a callable and verifies that when invoked, it throws // an exception with the given type and properties. @@ -4843,10 +4842,6 @@ class ExceptionMatcherImpl { // // EXPECT_THAT( // []() { throw std::runtime_error("message"); }, -// ThrowsMessageHasSubstr("message")); -// -// EXPECT_THAT( -// []() { throw std::runtime_error("message"); }, // Throws( // Property(&std::runtime_error::what, HasSubstr("message")))); @@ -4882,16 +4877,6 @@ ThrowsMessage(const MessageMatcher& messageMatcher) { Property("what", &std::exception::what, MatcherCast(messageMatcher))}); } -template -PolymorphicMatcher> -ThrowsMessageHasSubstr(const internal::StringLike& message) { - static_assert( - std::is_base_of::value, - "expected an std::exception-derived class"); - return MakePolymorphicMatcher( - internal::ExceptionMatcherImpl{ - Property("what", &std::exception::what, HasSubstr(message))}); -} #endif // GTEST_HAS_EXCEPTIONS diff --git a/googlemock/test/gmock-matchers_test.cc b/googlemock/test/gmock-matchers_test.cc index a8b39b8f6b..af3b02c178 100644 --- a/googlemock/test/gmock-matchers_test.cc +++ b/googlemock/test/gmock-matchers_test.cc @@ -8129,10 +8129,6 @@ TEST(ThrowsTest, Examples) { []() { throw std::runtime_error("message"); }, ThrowsMessage(HasSubstr("message"))); - EXPECT_THAT( - []() { throw std::runtime_error("message"); }, - ThrowsMessageHasSubstr("message")); - EXPECT_THAT( []() { throw std::runtime_error("message"); }, Throws( @@ -8163,16 +8159,11 @@ TEST(ThrowsTest, CallableExecutedExactlyOnce) { ThrowsMessage(HasSubstr("message"))); EXPECT_EQ(a, 3u); - EXPECT_THAT( - [&a]() { a++; throw std::runtime_error("message"); }, - ThrowsMessageHasSubstr("message")); - EXPECT_EQ(a, 4u); - EXPECT_THAT( [&a]() { a++; throw std::runtime_error("message"); }, Throws( Property(&std::runtime_error::what, HasSubstr("message")))); - EXPECT_EQ(a, 5u); + EXPECT_EQ(a, 4u); } TEST(ThrowsTest, Describe) { @@ -8180,7 +8171,7 @@ TEST(ThrowsTest, Describe) { std::stringstream ss; matcher.DescribeTo(&ss); auto explanation = ss.str(); - EXPECT_THAT(explanation, testing::HasSubstr("std::runtime_error")); + EXPECT_THAT(explanation, HasSubstr("std::runtime_error")); } TEST(ThrowsTest, Success) { @@ -8189,7 +8180,7 @@ TEST(ThrowsTest, Success) { EXPECT_TRUE( matcher.MatchAndExplain( []() { throw std::runtime_error("error message"); }, &listener)); - EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); + EXPECT_THAT(listener.str(), HasSubstr("std::runtime_error")); } TEST(ThrowsTest, FailWrongType) { @@ -8198,8 +8189,8 @@ TEST(ThrowsTest, FailWrongType) { EXPECT_FALSE( matcher.MatchAndExplain( []() { throw std::logic_error("error message"); }, &listener)); - EXPECT_THAT(listener.str(), testing::HasSubstr("std::logic_error")); - EXPECT_THAT(listener.str(), testing::HasSubstr("\"error message\"")); + EXPECT_THAT(listener.str(), HasSubstr("std::logic_error")); + EXPECT_THAT(listener.str(), HasSubstr("\"error message\"")); } TEST(ThrowsTest, FailWrongTypeNonStd) { @@ -8210,7 +8201,7 @@ TEST(ThrowsTest, FailWrongTypeNonStd) { []() { throw 10; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("throws an exception of an unknown type")); + HasSubstr("throws an exception of an unknown type")); } TEST(ThrowsTest, FailNoThrow) { @@ -8221,7 +8212,7 @@ TEST(ThrowsTest, FailNoThrow) { []() { (void)0; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("does not throw any exception")); + HasSubstr("does not throw any exception")); } class ThrowsPredicateTest: public TestWithParam> {}; @@ -8231,8 +8222,8 @@ TEST_P(ThrowsPredicateTest, Describe) { std::stringstream ss; matcher.DescribeTo(&ss); auto explanation = ss.str(); - EXPECT_THAT(explanation, testing::HasSubstr("std::runtime_error")); - EXPECT_THAT(explanation, testing::HasSubstr("error message")); + EXPECT_THAT(explanation, HasSubstr("std::runtime_error")); + EXPECT_THAT(explanation, HasSubstr("error message")); } TEST_P(ThrowsPredicateTest, Success) { @@ -8241,7 +8232,7 @@ TEST_P(ThrowsPredicateTest, Success) { EXPECT_TRUE( matcher.MatchAndExplain( []() { throw std::runtime_error("error message"); }, &listener)); - EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); + EXPECT_THAT(listener.str(), HasSubstr("std::runtime_error")); } TEST_P(ThrowsPredicateTest, FailWrongType) { @@ -8250,8 +8241,8 @@ TEST_P(ThrowsPredicateTest, FailWrongType) { EXPECT_FALSE( matcher.MatchAndExplain( []() { throw std::logic_error("error message"); }, &listener)); - EXPECT_THAT(listener.str(), testing::HasSubstr("std::logic_error")); - EXPECT_THAT(listener.str(), testing::HasSubstr("\"error message\"")); + EXPECT_THAT(listener.str(), HasSubstr("std::logic_error")); + EXPECT_THAT(listener.str(), HasSubstr("\"error message\"")); } TEST_P(ThrowsPredicateTest, FailWrongTypeNonStd) { @@ -8262,7 +8253,7 @@ TEST_P(ThrowsPredicateTest, FailWrongTypeNonStd) { []() { throw 10; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("throws an exception of an unknown type")); + HasSubstr("throws an exception of an unknown type")); } TEST_P(ThrowsPredicateTest, FailWrongMessage) { @@ -8271,8 +8262,8 @@ TEST_P(ThrowsPredicateTest, FailWrongMessage) { EXPECT_FALSE( matcher.MatchAndExplain( []() { throw std::runtime_error("wrong message"); }, &listener)); - EXPECT_THAT(listener.str(), testing::HasSubstr("std::runtime_error")); - EXPECT_THAT(listener.str(), testing::HasSubstr("wrong message")); + EXPECT_THAT(listener.str(), HasSubstr("std::runtime_error")); + EXPECT_THAT(listener.str(), HasSubstr("wrong message")); } TEST_P(ThrowsPredicateTest, FailNoThrow) { @@ -8283,18 +8274,16 @@ TEST_P(ThrowsPredicateTest, FailNoThrow) { []() { (void)0; }, &listener)); EXPECT_THAT( listener.str(), - testing::HasSubstr("does not throw any exception")); + HasSubstr("does not throw any exception")); } INSTANTIATE_TEST_SUITE_P(AllMessagePredicates, ThrowsPredicateTest, - ::testing::Values( + Values( static_cast>( Throws( Property(&std::exception::what, HasSubstr("error message")))), static_cast>( - ThrowsMessage(HasSubstr("error message"))), - static_cast>( - ThrowsMessageHasSubstr("error message")))); + ThrowsMessage(HasSubstr("error message"))))); // Tests that Throws(Matcher{}) compiles even when E2 != const E1&. TEST(ThrowsPredicateCompilesTest, ExceptionMatcherAcceptsBroadType) { @@ -8331,26 +8320,6 @@ TEST(ThrowsPredicateCompilesTest, MessageMatcherAcceptsNonMatcher) { []() { throw std::runtime_error("wrong error message"); })); } -// Tests that ThrowsMessageHasSubstr accepts types that're -// explicitly-convertible to std::string. -TEST(ThrowsPredicateCompilesTest, StringLikeMessage) { - struct SomeCustomString { - std::string inner; - - // Note: explicit conversion. - explicit operator std::string() const { return inner; } - }; - - Matcher matcher = ThrowsMessageHasSubstr( - SomeCustomString{"error message"}); - EXPECT_TRUE( - matcher.Matches( - []() { throw std::runtime_error("error message"); })); - EXPECT_FALSE( - matcher.Matches( - []() { throw std::runtime_error("wrong message"); })); -} - #endif // GTEST_HAS_EXCEPTIONS } // namespace