Skip to content

Commit b7a2edc

Browse files
bneradtbneradt
authored
Catch2: Use CAPTURE and GENERATE (#12731)
Add CAPTURE statements to parameterized tests to improve failure diagnostics. Convert many for-loop based parameterized tests to use GENERATE which provides automatic sectioning, continues running after failures, and produces clearer test output.
1 parent cc801ec commit b7a2edc

File tree

10 files changed

+153
-143
lines changed

10 files changed

+153
-143
lines changed

‎lib/swoc/unit_tests/test_TextView.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ TEST_CASE("TextView tokenizing", "[libswoc][TextView]") {
644644
TextView src = "alpha,bravo,,charlie";
645645
auto tokens = {"alpha", "bravo", "", "charlie"};
646646
for (auto token : tokens) {
647+
CAPTURE(token);
647648
REQUIRE(src.take_prefix_at(',') == token);
648649
}
649650
}

‎plugins/experimental/ja4_fingerprint/test_ja4.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ TEST_CASE("JA4")
9191
{0xfefc, "d3"}
9292
};
9393
for (auto const &[version, expected] : values) {
94+
CAPTURE(version, expected);
9495
TLS_summary.TLS_version = version;
9596
CHECK(expected == call_JA4(TLS_summary).substr(1, 2));
9697
}

‎plugins/experimental/txn_box/unit_tests/test_accl_utils.cc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,17 @@ TEST_CASE("Basic single char insert/full_match std::string_view")
4444
};
4545

4646
for (auto const &[k, v] : kv) {
47+
CAPTURE(k, v);
4748
REQUIRE(trie.insert(k, v));
4849
}
4950
// try again.
5051
for (auto const &[k, v] : kv) {
52+
CAPTURE(k, v);
5153
REQUIRE(!trie.insert(k, v));
5254
}
5355

5456
for (auto const &[k, v] : kv) {
57+
CAPTURE(k, v);
5558
auto [found, value] = trie.full_match(k);
5659
REQUIRE(found);
5760
REQUIRE(value == v);
@@ -80,14 +83,17 @@ TEST_CASE("Basic insert/full_match TextView", "")
8083
{"H", "6"}
8184
};
8285
for (auto const &[k, v] : kv) {
86+
CAPTURE(k, v);
8387
REQUIRE(trie.insert(k, v));
8488
}
8589
// try again.
8690
for (auto const &[k, v] : kv) {
91+
CAPTURE(k, v);
8792
REQUIRE(!trie.insert(k, v));
8893
}
8994

9095
for (auto const &[k, v] : kv) {
96+
CAPTURE(k, v);
9197
auto [found, value] = trie.full_match(k);
9298
REQUIRE(found);
9399
REQUIRE(value == v);
@@ -121,16 +127,19 @@ TEST_CASE("Basic Prefix match Test on std::string", "[insert][prefix_match][std:
121127
StringTree<std::string, std::string> trie;
122128
std::vector<std::pair<std::string, std::string>> kvs = generateKVFrom(std::string{"http://www.apache.com/trafficserver"});
123129
for (auto const &[k, v] : kvs) {
130+
CAPTURE(k, v);
124131
trie.insert(k, v);
125132
}
126133

127134
// basic check
128135
for (auto const &[k, v] : kvs) {
136+
CAPTURE(k, v);
129137
auto [found, value] = trie.full_match(k);
130138
REQUIRE(found);
131139
REQUIRE(value == v);
132140
}
133141
for (auto iter = std::begin(kvs); iter != std::end(kvs); ++iter) {
142+
CAPTURE(iter->first);
134143
auto const &keys = trie.prefix_match(iter->first);
135144
REQUIRE(std::equal(iter, std::end(kvs), std::begin(keys), std::end(keys)));
136145
}
@@ -164,6 +173,7 @@ TEST_CASE("Basic Prefix Match Test on a mix case strings", "")
164173
};
165174

166175
for (auto const &[key, expected] : exp_results) {
176+
CAPTURE(key, expected);
167177
auto const &items = trie.prefix_match(key);
168178
INFO("Looking for " << key << ", to be found " << expected.size() << "? found " << items.size());
169179

@@ -188,6 +198,7 @@ TEST_CASE("Basic Suffix Match Test", "")
188198
string_tree_map trie;
189199

190200
for (auto const &[k, v] : kv) {
201+
CAPTURE(k, v);
191202
REQUIRE(trie.insert(k, v));
192203
}
193204

@@ -201,6 +212,7 @@ TEST_CASE("Basic Suffix Match Test", "")
201212
};
202213

203214
for (auto const &[key, expected] : exp_results) {
215+
CAPTURE(key, expected);
204216
auto const &items = trie.suffix_match(key);
205217
REQUIRE(items.size() == expected.size());
206218
for (auto const &pair : items) {

‎src/iocore/net/unit_tests/test_YamlSNIConfig.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ TEST_CASE("YamlConfig handles bad ports appropriately.")
114114
std::string filepath;
115115
swoc::bwprint(filepath, "{}/sni_conf_test_bad_port_{}.yaml", _XSTR(LIBINKNET_UNIT_TEST_DIR), port_str);
116116

117+
CAPTURE(port_str, filepath);
118+
117119
swoc::Errata zret{conf.loader(filepath)};
118120
std::stringstream errorstream;
119121
errorstream << zret;

‎src/proxy/hdrs/unit_tests/test_Hdrs.cc‎

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include <string>
2323
#include <string_view>
24+
#include <vector>
2425
#include <cstring>
2526
#include <cctype>
2627
#include <bitset>
@@ -38,6 +39,8 @@ using namespace std::literals;
3839
#include "tsutil/PostScript.h"
3940

4041
#include <catch2/catch_test_macros.hpp>
42+
#include <catch2/generators/catch_generators.hpp>
43+
#include <catch2/generators/catch_generators_range.hpp>
4144

4245
#include "proxy/hdrs/HTTP.h"
4346
#include "proxy/hdrs/HttpCompat.h"
@@ -50,58 +53,55 @@ TEST_CASE("HdrTestHttpParse", "[proxy][hdrtest]")
5053
ParseResult expected_result;
5154
int expected_bytes_consumed;
5255
};
53-
static const std::array<Test, 26> tests = {
54-
{
55-
{"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
56-
{"GET /index.html HTTP/1.0\r\n\r\n***BODY****", ParseResult::DONE, 28},
57-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n***BODY****", ParseResult::DONE, 48},
58-
{"GET", ParseResult::ERROR, 3},
59-
{"GET /index.html", ParseResult::ERROR, 15},
60-
{"GET /index.html\r\n", ParseResult::ERROR, 17},
61-
{"GET /index.html HTTP/1.0", ParseResult::ERROR, 24},
62-
{"GET /index.html HTTP/1.0\r", ParseResult::ERROR, 25},
63-
{"GET /index.html HTTP/1.0\n", ParseResult::DONE, 25},
64-
{"GET /index.html HTTP/1.0\n\n", ParseResult::DONE, 26},
65-
{"GET /index.html HTTP/1.0\r\n\r\n", ParseResult::DONE, 28},
66-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar", ParseResult::ERROR, 44},
67-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\n", ParseResult::DONE, 45},
68-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
69-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n", ParseResult::DONE, 48},
70-
{"GET /index.html HTTP/1.0\nUser-Agent: foobar\n", ParseResult::DONE, 44},
71-
{"GET /index.html HTTP/1.0\nUser-Agent: foobar\nBoo: foo\n", ParseResult::DONE, 53},
72-
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
73-
{"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
74-
{"GET /index.html hTTP/1.0\r\n", ParseResult::ERROR, 26},
75-
{"POST /index.html HTTP/1.0\r\nContent-Length: 0\r\n\r\n", ParseResult::DONE, 48},
76-
{"POST /index.html HTTP/1.0\r\nContent-Length: \r\n\r\n", ParseResult::ERROR, 47},
77-
{"POST /index.html HTTP/1.0\r\nContent-Length:\r\n\r\n", ParseResult::ERROR, 46},
78-
{"CONNECT foo.example HTTP/1.1\r\n", ParseResult::DONE, 30},
79-
{"GET foo.example HTTP/1.1\r\n", ParseResult::ERROR, 26},
80-
{"", ParseResult::ERROR, 0},
81-
}
56+
57+
static const std::vector<Test> http_parse_tests = {
58+
{"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
59+
{"GET /index.html HTTP/1.0\r\n\r\n***BODY****", ParseResult::DONE, 28},
60+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n***BODY****", ParseResult::DONE, 48},
61+
{"GET", ParseResult::ERROR, 3 },
62+
{"GET /index.html", ParseResult::ERROR, 15},
63+
{"GET /index.html\r\n", ParseResult::ERROR, 17},
64+
{"GET /index.html HTTP/1.0", ParseResult::ERROR, 24},
65+
{"GET /index.html HTTP/1.0\r", ParseResult::ERROR, 25},
66+
{"GET /index.html HTTP/1.0\n", ParseResult::DONE, 25},
67+
{"GET /index.html HTTP/1.0\n\n", ParseResult::DONE, 26},
68+
{"GET /index.html HTTP/1.0\r\n\r\n", ParseResult::DONE, 28},
69+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar", ParseResult::ERROR, 44},
70+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\n", ParseResult::DONE, 45},
71+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
72+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n", ParseResult::DONE, 48},
73+
{"GET /index.html HTTP/1.0\nUser-Agent: foobar\n", ParseResult::DONE, 44},
74+
{"GET /index.html HTTP/1.0\nUser-Agent: foobar\nBoo: foo\n", ParseResult::DONE, 53},
75+
{"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
76+
{"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
77+
{"GET /index.html hTTP/1.0\r\n", ParseResult::ERROR, 26},
78+
{"POST /index.html HTTP/1.0\r\nContent-Length: 0\r\n\r\n", ParseResult::DONE, 48},
79+
{"POST /index.html HTTP/1.0\r\nContent-Length: \r\n\r\n", ParseResult::ERROR, 47},
80+
{"POST /index.html HTTP/1.0\r\nContent-Length:\r\n\r\n", ParseResult::ERROR, 46},
81+
{"CONNECT foo.example HTTP/1.1\r\n", ParseResult::DONE, 30},
82+
{"GET foo.example HTTP/1.1\r\n", ParseResult::ERROR, 26},
83+
{"", ParseResult::ERROR, 0 },
8284
};
8385

84-
HTTPParser parser;
86+
auto test = GENERATE(from_range(http_parse_tests));
87+
CAPTURE(test.msg, test.expected_result, test.expected_bytes_consumed);
8588

89+
HTTPParser parser;
8690
http_parser_init(&parser);
8791

88-
for (auto const &test : tests) {
89-
HTTPHdr req_hdr;
90-
HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); // extra to prevent proxy allocation.
91-
92-
req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap);
92+
HTTPHdr req_hdr;
93+
HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); // extra to prevent proxy allocation.
9394

94-
http_parser_clear(&parser);
95+
req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap);
9596

96-
auto start = test.msg.data();
97-
auto ret = req_hdr.parse_req(&parser, &start, test.msg.data_end(), true);
98-
auto bytes_consumed = start - test.msg.data();
97+
auto start = test.msg.data();
98+
auto ret = req_hdr.parse_req(&parser, &start, test.msg.data_end(), true);
99+
auto bytes_consumed = start - test.msg.data();
99100

100-
REQUIRE(bytes_consumed == test.expected_bytes_consumed);
101-
REQUIRE(ret == test.expected_result);
101+
REQUIRE(bytes_consumed == test.expected_bytes_consumed);
102+
REQUIRE(ret == test.expected_result);
102103

103-
req_hdr.destroy();
104-
}
104+
req_hdr.destroy();
105105
}
106106

107107
TEST_CASE("MIMEScanner_fragments", "[proxy][mimescanner_fragments]")
@@ -125,6 +125,7 @@ TEST_CASE("MIMEScanner_fragments", "[proxy][mimescanner_fragments]")
125125
swoc::TextView output; // only set on last call
126126

127127
for (auto const &frag : fragments) {
128+
CAPTURE(frag.msg, frag.shares_input, frag.expected_result);
128129
swoc::TextView input = frag.msg;
129130
bool got_shares_input = !frag.shares_input;
130131
constexpr bool const is_eof = false;
@@ -611,6 +612,7 @@ TEST_CASE("HdrTest", "[proxy][hdrtest]")
611612
mime_parser_init(&parser);
612613

613614
for (const auto &t : test_cases) {
615+
CAPTURE(t.line, t.expected);
614616
mime_parser_clear(&parser);
615617

616618
const char *start = t.line.data();

‎src/proxy/hdrs/unit_tests/test_URL.cc‎

Lines changed: 36 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,23 @@
1919
*/
2020

2121
#include <cstdio>
22+
#include <vector>
2223

2324
#include <catch2/catch_test_macros.hpp>
25+
#include <catch2/generators/catch_generators.hpp>
26+
#include <catch2/generators/catch_generators_range.hpp>
2427

2528
#include "proxy/hdrs/URL.h"
2629
#include "tscore/CryptoHash.h"
2730

2831
TEST_CASE("ValidateURL", "[proxy][validurl]")
2932
{
30-
static const struct {
33+
struct Test {
3134
const char *const text;
3235
bool valid;
33-
} http_validate_hdr_field_test_case[] = {
36+
};
37+
38+
static const std::vector<Test> http_validate_hdr_field_test_case = {
3439
{"yahoo", true },
3540
{"yahoo.com", true },
3641
{"yahoo.wow.com", true },
@@ -47,21 +52,20 @@ TEST_CASE("ValidateURL", "[proxy][validurl]")
4752
{"!@#$%^ &*(*&^%$#@#$%^&*(*&^%$#))", false},
4853
{":):(:O!!!!!!", false}
4954
};
50-
for (auto i : http_validate_hdr_field_test_case) {
51-
const char *const txt = i.text;
52-
if (validate_host_name({txt}) != i.valid) {
53-
std::printf("Validation of FQDN (host) header: \"%s\", expected %s, but not\n", txt, (i.valid ? "true" : "false"));
54-
CHECK(false);
55-
}
56-
}
55+
56+
auto i = GENERATE(from_range(http_validate_hdr_field_test_case));
57+
CAPTURE(i.text, i.valid);
58+
CHECK(validate_host_name({i.text}) == i.valid);
5759
}
5860

5961
TEST_CASE("Validate Scheme", "[proxy][validscheme]")
6062
{
61-
static const struct {
63+
struct Test {
6264
std::string_view text;
6365
bool valid;
64-
} scheme_test_cases[] = {
66+
};
67+
68+
static const std::vector<Test> scheme_test_cases = {
6569
{"http", true },
6670
{"https", true },
6771
{"example", true },
@@ -76,16 +80,9 @@ TEST_CASE("Validate Scheme", "[proxy][validscheme]")
7680
{"example://", false}
7781
};
7882

79-
for (auto i : scheme_test_cases) {
80-
// it's pretty hard to debug with
81-
// CHECK(validate_scheme(i.text) == i.valid);
82-
83-
std::string_view text = i.text;
84-
if (validate_scheme(text) != i.valid) {
85-
std::printf("Validation of scheme: \"%s\", expected %s, but not\n", text.data(), (i.valid ? "true" : "false"));
86-
CHECK(false);
87-
}
88-
}
83+
auto i = GENERATE(from_range(scheme_test_cases));
84+
CAPTURE(i.text, i.valid);
85+
CHECK(validate_scheme(i.text) == i.valid);
8986
}
9087

9188
namespace UrlImpl
@@ -97,10 +94,12 @@ using namespace UrlImpl;
9794

9895
TEST_CASE("ParseRulesStrictURI", "[proxy][parseuri]")
9996
{
100-
const struct {
97+
struct Test {
10198
const char *const uri;
10299
bool valid;
103-
} http_strict_uri_parsing_test_case[] = {
100+
};
101+
102+
static const std::vector<Test> http_strict_uri_parsing_test_case = {
104103
{"//index.html", true },
105104
{"/home", true },
106105
{"/path/data?key=value#id", true },
@@ -127,21 +126,19 @@ TEST_CASE("ParseRulesStrictURI", "[proxy][parseuri]")
127126
{"é", false}
128127
};
129128

130-
for (auto i : http_strict_uri_parsing_test_case) {
131-
const char *const uri = i.uri;
132-
if (url_is_strictly_compliant(uri, uri + strlen(uri)) != i.valid) {
133-
std::printf("Strictly parse URI: \"%s\", expected %s, but not\n", uri, (i.valid ? "true" : "false"));
134-
CHECK(false);
135-
}
136-
}
129+
auto i = GENERATE(from_range(http_strict_uri_parsing_test_case));
130+
CAPTURE(i.uri, i.valid);
131+
CHECK(url_is_strictly_compliant(i.uri, i.uri + strlen(i.uri)) == i.valid);
137132
}
138133

139134
TEST_CASE("ParseRulesMostlyStrictURI", "[proxy][parseuri]")
140135
{
141-
const struct {
136+
struct Test {
142137
const char *const uri;
143138
bool valid;
144-
} http_mostly_strict_uri_parsing_test_case[] = {
139+
};
140+
141+
static const std::vector<Test> http_mostly_strict_uri_parsing_test_case = {
145142
{"//index.html", true },
146143
{"/home", true },
147144
{"/path/data?key=value#id", true },
@@ -168,13 +165,9 @@ TEST_CASE("ParseRulesMostlyStrictURI", "[proxy][parseuri]")
168165
{"é", false}
169166
}; // Non-printable ascii
170167

171-
for (auto i : http_mostly_strict_uri_parsing_test_case) {
172-
const char *const uri = i.uri;
173-
if (url_is_mostly_compliant(uri, uri + strlen(uri)) != i.valid) {
174-
std::printf("Mostly strictly parse URI: \"%s\", expected %s, but not\n", uri, (i.valid ? "true" : "false"));
175-
CHECK(false);
176-
}
177-
}
168+
auto i = GENERATE(from_range(http_mostly_strict_uri_parsing_test_case));
169+
CAPTURE(i.uri, i.valid);
170+
CHECK(url_is_mostly_compliant(i.uri, i.uri + strlen(i.uri)) == i.valid);
178171
}
179172

180173
struct url_parse_test_case {
@@ -570,10 +563,10 @@ test_parse(url_parse_test_case const &test_case, bool parse_function)
570563

571564
TEST_CASE("UrlParse", "[proxy][parseurl]")
572565
{
573-
for (auto const &test_case : url_parse_test_cases) {
574-
test_parse(test_case, URL_PARSE);
575-
test_parse(test_case, URL_PARSE_REGEX);
576-
}
566+
auto test_case = GENERATE(from_range(url_parse_test_cases));
567+
CAPTURE(test_case.input_uri, test_case.expected_printed_url, test_case.is_valid);
568+
test_parse(test_case, URL_PARSE);
569+
test_parse(test_case, URL_PARSE_REGEX);
577570
}
578571

579572
struct get_hash_test_case {

0 commit comments

Comments
 (0)