Skip to content

Commit 74016d4

Browse files
authored
Rework the IsSuccess matcher to be fully polymorphic (#5981)
Previously, this matcher mostly worked, but the `DescribeTo` functions wouldn't compile when another polymorphic matcher was nested to match the value. The updated code uses the same polymorphic matcher design as used by `Not` and others in Google Test itself. I've added a test that uses `VariantWith` to nest matchers more deeply with `IsSuccess`. This test doesn't compile prior to this change.
1 parent d49cb3e commit 74016d4

File tree

2 files changed

+53
-16
lines changed

2 files changed

+53
-16
lines changed

common/error_test.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ using ::Carbon::Testing::IsError;
1919
using ::Carbon::Testing::IsSuccess;
2020
using ::testing::_;
2121
using ::testing::Eq;
22+
using ::testing::VariantWith;
2223

2324
TEST(ErrorTest, Error) {
2425
Error err("test");
@@ -158,6 +159,17 @@ TYPED_TEST(ErrorOrTest, UnprintableValue) {
158159
EXPECT_THAT(error, IsError(this->ErrorStr()));
159160
}
160161

162+
// Note that this is more of a test of `IsSuccess` than `ErrorOr` itself.
163+
TYPED_TEST(ErrorOrTest, NestedMatching) {
164+
using TestErrorOr = ErrorOr<std::variant<int, float>, TypeParam>;
165+
166+
TestErrorOr i(42);
167+
EXPECT_THAT(i, IsSuccess(VariantWith<int>(Eq(42))));
168+
169+
TestErrorOr f(0.42F);
170+
EXPECT_THAT(f, IsSuccess(VariantWith<float>(Eq(0.42F))));
171+
}
172+
161173
TYPED_TEST(ErrorOrTest, ReturnIfErrorNoError) {
162174
using TestErrorOr = ErrorOr<Success, TypeParam>;
163175
auto result = []() -> TestErrorOr {

common/error_test_helpers.h

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,43 +48,68 @@ class IsError {
4848
::testing::Matcher<std::string> matcher_;
4949
};
5050

51-
// Matches the value for a non-error state of `ErrorOr<T>`. For example:
52-
// EXPECT_THAT(my_result, IsSuccess(Eq(3)));
53-
template <typename InnerMatcher>
54-
class IsSuccessMatcher {
51+
// Implementation of a success matcher for a specific `T` and `ErrorT` in an
52+
// `ErrorOr`. Supports a nested matcher for the `T` value.
53+
template <typename T, typename ErrorT>
54+
class IsSuccessMatcherImpl
55+
: public ::testing::MatcherInterface<const ErrorOr<T, ErrorT>&> {
5556
public:
56-
// NOLINTNEXTLINE(readability-identifier-naming)
57-
using is_gtest_matcher = void;
58-
59-
explicit IsSuccessMatcher(InnerMatcher matcher)
60-
: matcher_(std::move(matcher)) {}
57+
explicit IsSuccessMatcherImpl(const ::testing::Matcher<T>& matcher)
58+
: matcher_(matcher) {}
6159

62-
template <typename T, typename ErrorT>
6360
auto MatchAndExplain(const ErrorOr<T, ErrorT>& result,
64-
::testing::MatchResultListener* listener) const -> bool {
61+
::testing::MatchResultListener* listener) const
62+
-> bool override {
6563
if (result.ok()) {
66-
return ::testing::Matcher<T>(matcher_).MatchAndExplain(*result, listener);
64+
return matcher_.MatchAndExplain(*result, listener);
6765
} else {
6866
*listener << "is an error with `" << result.error() << "`";
6967
return false;
7068
}
7169
}
7270

73-
auto DescribeTo(std::ostream* os) const -> void {
71+
auto DescribeTo(std::ostream* os) const -> void override {
7472
*os << "is a success and matches ";
7573
matcher_.DescribeTo(os);
7674
}
7775

78-
auto DescribeNegationTo(std::ostream* os) const -> void {
76+
auto DescribeNegationTo(std::ostream* os) const -> void override {
7977
*os << "is an error or does not match ";
80-
matcher_.DescribeTo(os);
78+
matcher_.DescribeNegationTo(os);
79+
}
80+
81+
private:
82+
::testing::Matcher<T> matcher_;
83+
};
84+
85+
// Polymorphic match implementation for GoogleTest.
86+
//
87+
// To support matching arbitrary types that `InnerMatcher` can also match, this
88+
// itself must match arbitrary types. This is accomplished by not being a
89+
// matcher itself, but by being convertible into matchers for any particular
90+
// `ErrorOr`.
91+
template <typename InnerMatcher>
92+
class IsSuccessMatcher {
93+
public:
94+
explicit IsSuccessMatcher(InnerMatcher matcher)
95+
: matcher_(std::move(matcher)) {}
96+
97+
template <typename T, typename ErrorT>
98+
// NOLINTNEXTLINE(google-explicit-constructor): Required for matcher APIs.
99+
operator ::testing::Matcher<const ErrorOr<T, ErrorT>&>() const {
100+
return ::testing::Matcher<const ErrorOr<T, ErrorT>&>(
101+
new IsSuccessMatcherImpl<T, ErrorT>(
102+
::testing::SafeMatcherCast<T>(matcher_)));
81103
}
82104

83105
private:
84106
InnerMatcher matcher_;
85107
};
86108

87-
// Wraps `IsSuccessMatcher` for the inner matcher deduction.
109+
// Returns a matcher the value for a non-error state of `ErrorOr<T>`.
110+
//
111+
// For example:
112+
// EXPECT_THAT(my_result, IsSuccess(Eq(3)));
88113
template <typename InnerMatcher>
89114
auto IsSuccess(InnerMatcher matcher) -> IsSuccessMatcher<InnerMatcher> {
90115
return IsSuccessMatcher<InnerMatcher>(matcher);

0 commit comments

Comments
 (0)