Skip to content

Commit 0ece2dc

Browse files
ravenblackxravenblackx
authored
Improve matcher tests (#39603)
Commit Message: Improve matcher tests Additional Description: This is how gtests are supposed to work. NOTE: please do not merge until #38726 is in, to avoid provoking a merge conflict on a PR that's already been embattled. Before: ``` ./test/common/matcher/test_utility.h:290: Failure Expected equality of these values: result.on_match_->action_cb_().get()->getTyped<StringAction>() Which is: 40-byte object <80-BA 85-24 1A-5C 00-00 58-97 EB-3F B2-72 00-00 08-00 00-00 00-00 00-00 6E-6F 5F-6D 61-74 63-68 00-00 00-00 00-00 00-00> *stringValue(expected_value) Which is: 40-byte object <80-BA 85-24 1A-5C 00-00 18-97 EB-3F B2-72 00-00 0A-00 00-00 00-00 00-00 78-78 6E-6F 5F-6D 61-74 63-68 00-00 00-00 00-00> ``` After: ``` test/common/matcher/prefix_map_matcher_test.cc:44: Failure Value of: result Expected: has string action (matcher: "xxno_match") Actual: {match_state_=MatchComplete, on_match_={action_cb_={string_value="no_match"}}} ``` (Note, both failures actually are on line 44 of prefix_map_matcher_test, but only the "after" version says so.) Risk Level: None, test-only. Testing: Yes it is. Docs Changes: n/a Release Notes: n/a (unless we need a release note in case someone was using these test helpers in an external extension?) Platform Specific Features: n/a --------- Signed-off-by: Raven Black <[email protected]>
1 parent e17a126 commit 0ece2dc

File tree

3 files changed

+95
-79
lines changed

3 files changed

+95
-79
lines changed

‎test/common/matcher/exact_map_matcher_test.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ TEST(ExactMapMatcherTest, NoMatch) {
2121

2222
TestData data;
2323
const auto result = matcher->match(data);
24-
verifyNoMatch(result);
24+
EXPECT_THAT(result, HasNoMatch());
2525
}
2626

2727
TEST(ExactMapMatcherTest, NoMatchDueToNoData) {
@@ -32,7 +32,7 @@ TEST(ExactMapMatcherTest, NoMatchDueToNoData) {
3232

3333
TestData data;
3434
const auto result = matcher->match(data);
35-
verifyNoMatch(result);
35+
EXPECT_THAT(result, HasNoMatch());
3636
}
3737

3838
TEST(ExactMapMatcherTest, NoMatchWithFallback) {
@@ -43,7 +43,7 @@ TEST(ExactMapMatcherTest, NoMatchWithFallback) {
4343

4444
TestData data;
4545
const auto result = matcher->match(data);
46-
verifyImmediateMatch(result, "no_match");
46+
EXPECT_THAT(result, HasStringAction("no_match"));
4747
}
4848

4949
TEST(ExactMapMatcherTest, Match) {
@@ -56,7 +56,7 @@ TEST(ExactMapMatcherTest, Match) {
5656

5757
TestData data;
5858
const auto result = matcher->match(data);
59-
verifyImmediateMatch(result, "match");
59+
EXPECT_THAT(result, HasStringAction("match"));
6060
}
6161

6262
TEST(ExactMapMatcherTest, DataNotAvailable) {
@@ -69,7 +69,7 @@ TEST(ExactMapMatcherTest, DataNotAvailable) {
6969

7070
TestData data;
7171
const auto result = matcher->match(data);
72-
verifyNotEnoughDataForMatch(result);
72+
EXPECT_THAT(result, HasNotEnoughData());
7373
}
7474

7575
TEST(ExactMapMatcherTest, MoreDataMightBeAvailableNoMatch) {
@@ -82,7 +82,7 @@ TEST(ExactMapMatcherTest, MoreDataMightBeAvailableNoMatch) {
8282

8383
TestData data;
8484
const auto result = matcher->match(data);
85-
verifyNotEnoughDataForMatch(result);
85+
EXPECT_THAT(result, HasNotEnoughData());
8686
}
8787

8888
TEST(ExactMapMatcherTest, MoreDataMightBeAvailableMatch) {
@@ -95,7 +95,7 @@ TEST(ExactMapMatcherTest, MoreDataMightBeAvailableMatch) {
9595

9696
TestData data;
9797
const auto result = matcher->match(data);
98-
verifyImmediateMatch(result, "match");
98+
EXPECT_THAT(result, HasStringAction("match"));
9999
}
100100

101101
TEST(ExactMapMatcherTest, RecursiveMatching) {
@@ -114,7 +114,7 @@ TEST(ExactMapMatcherTest, RecursiveMatching) {
114114

115115
TestData data;
116116
const auto result = matcher->match(data);
117-
verifyImmediateMatch(result, "match");
117+
EXPECT_THAT(result, HasStringAction("match"));
118118
}
119119

120120
TEST(ExactMapMatcherTest, RecursiveMatchingOnNoMatch) {
@@ -133,7 +133,7 @@ TEST(ExactMapMatcherTest, RecursiveMatchingOnNoMatch) {
133133

134134
TestData data;
135135
const auto result = matcher->match(data);
136-
verifyImmediateMatch(result, "nested_match");
136+
EXPECT_THAT(result, HasStringAction("nested_match"));
137137
}
138138

139139
TEST(ExactMapMatcherTest, RecursiveMatchingWithKeepMatching) {

‎test/common/matcher/prefix_map_matcher_test.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ TEST(PrefixMapMatcherTest, NoMatch) {
1919

2020
TestData data;
2121
const auto result = matcher->match(data);
22-
verifyNoMatch(result);
22+
EXPECT_THAT(result, HasNoMatch());
2323
}
2424

2525
TEST(PrefixMapMatcherTest, NoMatchDueToNoData) {
@@ -30,7 +30,7 @@ TEST(PrefixMapMatcherTest, NoMatchDueToNoData) {
3030

3131
TestData data;
3232
const auto result = matcher->match(data);
33-
verifyNoMatch(result);
33+
EXPECT_THAT(result, HasNoMatch());
3434
}
3535

3636
TEST(PrefixMapMatcherTest, NoMatchWithFallback) {
@@ -41,7 +41,7 @@ TEST(PrefixMapMatcherTest, NoMatchWithFallback) {
4141

4242
TestData data;
4343
const auto result = matcher->match(data);
44-
verifyImmediateMatch(result, "no_match");
44+
EXPECT_THAT(result, HasStringAction("no_match"));
4545
}
4646

4747
TEST(PrefixMapMatcherTest, Match) {
@@ -54,7 +54,7 @@ TEST(PrefixMapMatcherTest, Match) {
5454

5555
TestData data;
5656
const auto result = matcher->match(data);
57-
verifyImmediateMatch(result, "match");
57+
EXPECT_THAT(result, HasStringAction("match"));
5858
}
5959

6060
TEST(PrefixMapMatcherTest, PrefixMatch) {
@@ -67,7 +67,7 @@ TEST(PrefixMapMatcherTest, PrefixMatch) {
6767

6868
TestData data;
6969
const auto result = matcher->match(data);
70-
verifyImmediateMatch(result, "mat");
70+
EXPECT_THAT(result, HasStringAction("mat"));
7171
}
7272

7373
TEST(PrefixMapMatcherTest, LongestPrefixMatch) {
@@ -82,7 +82,7 @@ TEST(PrefixMapMatcherTest, LongestPrefixMatch) {
8282

8383
TestData data;
8484
const auto result = matcher->match(data);
85-
verifyImmediateMatch(result, "match");
85+
EXPECT_THAT(result, HasStringAction("match"));
8686
}
8787

8888
TEST(PrefixMapMatcherTest, DataNotAvailable) {
@@ -95,7 +95,7 @@ TEST(PrefixMapMatcherTest, DataNotAvailable) {
9595

9696
TestData data;
9797
const auto result = matcher->match(data);
98-
verifyNotEnoughDataForMatch(result);
98+
EXPECT_THAT(result, HasNotEnoughData());
9999
}
100100

101101
TEST(PrefixMapMatcherTest, MoreDataMightBeAvailableNoMatch) {
@@ -108,7 +108,7 @@ TEST(PrefixMapMatcherTest, MoreDataMightBeAvailableNoMatch) {
108108

109109
TestData data;
110110
const auto result = matcher->match(data);
111-
verifyNotEnoughDataForMatch(result);
111+
EXPECT_THAT(result, HasNotEnoughData());
112112
}
113113

114114
TEST(PrefixMapMatcherTest, MoreDataMightBeAvailableMatch) {
@@ -121,7 +121,7 @@ TEST(PrefixMapMatcherTest, MoreDataMightBeAvailableMatch) {
121121

122122
TestData data;
123123
const auto result = matcher->match(data);
124-
verifyImmediateMatch(result, "match");
124+
EXPECT_THAT(result, HasStringAction("match"));
125125
}
126126

127127
TEST(PrefixMapMatcherTest, MoreDataMightBeAvailableNoMatchThenMatchDoesNotPerformSecondMatch) {

‎test/common/matcher/test_utility.h‎

Lines changed: 77 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -271,90 +271,106 @@ template <class T> OnMatch<T> stringOnMatch(absl::string_view value, bool keep_m
271271
return OnMatch<T>{[s = std::string(value)]() { return stringValue(s); }, nullptr, keep_matching};
272272
}
273273

274-
// Verifies the match tree completes the matching with an not match result.
275-
void verifyNoMatch(const MatchTree<TestData>::MatchResult& result) {
276-
EXPECT_EQ(MatchState::MatchComplete, result.match_state_);
277-
EXPECT_FALSE(result.on_match_.has_value());
274+
inline void PrintTo(const Action& action, std::ostream* os) {
275+
if (action.typeUrl() == "google.protobuf.StringValue") {
276+
*os << "{string_value=\"" << action.getTyped<StringAction>().string_ << "\"}";
277+
return;
278+
}
279+
*os << "{type=" << action.typeUrl() << "}";
278280
}
279281

280-
void verifyOnMatch(const OnMatch<TestData>& on_match, absl::string_view expected_value) {
281-
EXPECT_NE(on_match.action_cb_, nullptr);
282-
EXPECT_EQ(on_match.action_cb_().get()->getTyped<StringAction>(), *stringValue(expected_value))
283-
<< " Actual: " << on_match.action_cb_().get()->getTyped<StringAction>().string_
284-
<< ". Expected: " << expected_value;
282+
inline void PrintTo(const ActionFactoryCb& action_cb, std::ostream* os) {
283+
if (action_cb == nullptr) {
284+
*os << "nullptr";
285+
return;
286+
}
287+
ActionPtr action = action_cb();
288+
PrintTo(*action, os);
285289
}
286290

287-
// Verifies the match tree completes the matching with the expected value.
288-
void verifyImmediateMatch(const MatchTree<TestData>::MatchResult& result,
289-
absl::string_view expected_value) {
290-
EXPECT_EQ(MatchState::MatchComplete, result.match_state_);
291-
EXPECT_TRUE(result.on_match_.has_value());
291+
inline void PrintTo(const MatchState state, std::ostream* os) {
292+
switch (state) {
293+
case MatchState::UnableToMatch:
294+
*os << "UnableToMatch";
295+
break;
296+
case MatchState::MatchComplete:
297+
*os << "MatchComplete";
298+
break;
299+
}
300+
}
292301

293-
EXPECT_EQ(nullptr, result.on_match_->matcher_);
294-
verifyOnMatch(*result.on_match_, expected_value);
302+
inline void PrintTo(const MatchTree<TestData>& matcher, std::ostream* os) {
303+
*os << "{type=" << typeid(matcher).name() << "}";
295304
}
296305

297-
// Verifies the match tree fails to match since the data are not enough.
298-
void verifyNotEnoughDataForMatch(const MatchTree<TestData>::MatchResult& result) {
299-
EXPECT_EQ(MatchState::UnableToMatch, result.match_state_);
300-
EXPECT_FALSE(result.on_match_.has_value());
306+
inline void PrintTo(const OnMatch<TestData>& on_match, std::ostream* os) {
307+
if (on_match.action_cb_) {
308+
*os << "{action_cb_=";
309+
PrintTo(on_match.action_cb_, os);
310+
*os << "}";
311+
} else if (on_match.matcher_) {
312+
*os << "{matcher_=";
313+
PrintTo(*on_match.matcher_, os);
314+
*os << "}";
315+
} else {
316+
*os << "{invalid, no value set}";
317+
}
301318
}
302319

303-
MATCHER_P(IsStringAction, m, "") {
304-
// Accepts an ActionFactoryCb argument.
305-
if (arg == nullptr) {
306-
*result_listener << "action callback is nullptr";
307-
return false;
320+
inline void PrintTo(const MatchTree<TestData>::MatchResult& result, std::ostream* os) {
321+
*os << "{match_state_=";
322+
PrintTo(result.match_state_, os);
323+
*os << ", on_match_=";
324+
if (result.on_match_.has_value()) {
325+
PrintTo(result.on_match_.value(), os);
326+
} else {
327+
*os << "nullopt";
308328
}
309-
ActionPtr action = arg();
310-
StringAction string_action = action->getTyped<StringAction>();
311-
return ::testing::ExplainMatchResult(m, string_action.string_, result_listener);
329+
*os << "}";
312330
}
313331

314-
MATCHER_P(HasStringAction, m, "") {
315-
// Accepts a MatchResult argument.
316-
if (arg.match_state_ != MatchState::MatchComplete) {
317-
*result_listener << "match_state_ is not MatchComplete";
332+
MATCHER(HasNotEnoughData, "") {
333+
// Takes a MatchTree<TestData>::MatchResult& and validates that it
334+
// is in the UnableToMatch state.
335+
return arg.match_state_ == MatchState::UnableToMatch && !arg.on_match_.has_value();
336+
}
337+
338+
MATCHER_P(IsStringAction, matcher, "") {
339+
// Takes an ActionFactoryCb argument, and compares its StringAction's string against matcher.
340+
if (arg == nullptr) {
318341
return false;
319342
}
320-
if (arg.on_match_ == absl::nullopt) {
321-
*result_listener << "on_match_ is nullopt";
343+
ActionPtr action = arg();
344+
if (action->typeUrl() != "google.protobuf.StringValue") {
322345
return false;
323346
}
324-
return ExplainMatchResult(IsStringAction(m), arg.on_match_->action_cb_, result_listener);
347+
return ::testing::ExplainMatchResult(testing::Matcher<std::string>(matcher),
348+
action->template getTyped<StringAction>().string_,
349+
result_listener);
325350
}
326351

327-
MATCHER(HasNoMatch, "") {
328-
// Accepts a MatchResult argument.
329-
if (arg.match_state_ != MatchState::MatchComplete) {
330-
*result_listener << "match_state_ is not MatchComplete";
352+
MATCHER_P(HasStringAction, matcher, "") {
353+
// Takes a MatchTree<TestData>::MatchResult& and validates that it
354+
// is a StringAction with contents matching matcher.
355+
if (arg.match_state_ != MatchState::MatchComplete || !arg.on_match_.has_value() ||
356+
arg.on_match_->matcher_ != nullptr) {
331357
return false;
332358
}
333-
if (arg.on_match_ != absl::nullopt) {
334-
*result_listener << "on_match_ was not nullopt";
335-
return false;
336-
}
337-
return true;
359+
return ::testing::ExplainMatchResult(IsStringAction(matcher), arg.on_match_->action_cb_,
360+
result_listener);
361+
}
362+
363+
MATCHER(HasNoMatch, "") {
364+
// Takes a MatchTree<TestData>::MatchResult& and validates that it
365+
// is MatchComplete with no on_match_.
366+
return arg.match_state_ == MatchState::MatchComplete && !arg.on_match_.has_value();
338367
}
339368

340369
MATCHER(HasSubMatcher, "") {
341-
// Accepts a MatchResult argument.
342-
if (arg.match_state_ != MatchState::MatchComplete) {
343-
*result_listener << "match_state_ is not MatchComplete";
344-
return false;
345-
}
346-
if (arg.on_match_ == absl::nullopt) {
347-
*result_listener << "on_match_ is nullopt";
348-
return false;
349-
}
350-
if (arg.on_match_->matcher_ == nullptr) {
351-
*result_listener << "on_match_->matcher_ is nullptr, expected it to not be.";
352-
if (arg.on_match_->action_cb_ != nullptr) {
353-
*result_listener << "\non_match_->action_cb_ is not nullptr.";
354-
}
355-
return false;
356-
}
357-
return true;
370+
// Takes a MatchTree<TestData>::MatchResult& and validates that it
371+
// has a matcher_ value in on_match_.
372+
return arg.match_state_ == MatchState::MatchComplete && arg.on_match_.has_value() &&
373+
arg.on_match_->matcher_ != nullptr;
358374
}
359375

360376
MATCHER_P(HasResult, m, "") {

0 commit comments

Comments
 (0)