You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2025/12/04 20:02:00 UTC
[PR] Catch2: Use CAPTURE and GENERATE [trafficserver]
bneradt opened a new pull request, #12731:
URL: https://github.com/apache/trafficserver/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Catch2: Use CAPTURE and GENERATE [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #12731:
URL: https://github.com/apache/trafficserver/pull/12731#discussion_r2590417507
##########
src/proxy/hdrs/unit_tests/test_URL.cc:
##########
@@ -76,16 +80,9 @@ TEST_CASE("Validate Scheme", "[proxy][validscheme]")
{"example://", false}
};
- for (auto i : scheme_test_cases) {
- // it's pretty hard to debug with
- // CHECK(validate_scheme(i.text) == i.valid);
-
- std::string_view text = i.text;
- if (validate_scheme(text) != i.valid) {
- std::printf("Validation of scheme: \"%s\", expected %s, but not\n", text.data(), (i.valid ? "true" : "false"));
- CHECK(false);
- }
- }
Review Comment:
Note that the workarounds like this aren't necessary anymore with the CAPTURE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Catch2: Use CAPTURE and GENERATE [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #12731:
URL: https://github.com/apache/trafficserver/pull/12731
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Catch2: Use CAPTURE and GENERATE [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #12731:
URL: https://github.com/apache/trafficserver/pull/12731#discussion_r2590410468
##########
src/proxy/hdrs/unit_tests/test_Hdrs.cc:
##########
@@ -50,58 +53,55 @@ TEST_CASE("HdrTestHttpParse", "[proxy][hdrtest]")
ParseResult expected_result;
int expected_bytes_consumed;
};
- static const std::array<Test, 26> tests = {
- {
- {"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
- {"GET /index.html HTTP/1.0\r\n\r\n***BODY****", ParseResult::DONE, 28},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n***BODY****", ParseResult::DONE, 48},
- {"GET", ParseResult::ERROR, 3},
- {"GET /index.html", ParseResult::ERROR, 15},
- {"GET /index.html\r\n", ParseResult::ERROR, 17},
- {"GET /index.html HTTP/1.0", ParseResult::ERROR, 24},
- {"GET /index.html HTTP/1.0\r", ParseResult::ERROR, 25},
- {"GET /index.html HTTP/1.0\n", ParseResult::DONE, 25},
- {"GET /index.html HTTP/1.0\n\n", ParseResult::DONE, 26},
- {"GET /index.html HTTP/1.0\r\n\r\n", ParseResult::DONE, 28},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar", ParseResult::ERROR, 44},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\n", ParseResult::DONE, 45},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n", ParseResult::DONE, 48},
- {"GET /index.html HTTP/1.0\nUser-Agent: foobar\n", ParseResult::DONE, 44},
- {"GET /index.html HTTP/1.0\nUser-Agent: foobar\nBoo: foo\n", ParseResult::DONE, 53},
- {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
- {"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
- {"GET /index.html hTTP/1.0\r\n", ParseResult::ERROR, 26},
- {"POST /index.html HTTP/1.0\r\nContent-Length: 0\r\n\r\n", ParseResult::DONE, 48},
- {"POST /index.html HTTP/1.0\r\nContent-Length: \r\n\r\n", ParseResult::ERROR, 47},
- {"POST /index.html HTTP/1.0\r\nContent-Length:\r\n\r\n", ParseResult::ERROR, 46},
- {"CONNECT foo.example HTTP/1.1\r\n", ParseResult::DONE, 30},
- {"GET foo.example HTTP/1.1\r\n", ParseResult::ERROR, 26},
- {"", ParseResult::ERROR, 0},
- }
+
+ static const std::vector<Test> http_parse_tests = {
+ {"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
+ {"GET /index.html HTTP/1.0\r\n\r\n***BODY****", ParseResult::DONE, 28},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n***BODY****", ParseResult::DONE, 48},
+ {"GET", ParseResult::ERROR, 3 },
+ {"GET /index.html", ParseResult::ERROR, 15},
+ {"GET /index.html\r\n", ParseResult::ERROR, 17},
+ {"GET /index.html HTTP/1.0", ParseResult::ERROR, 24},
+ {"GET /index.html HTTP/1.0\r", ParseResult::ERROR, 25},
+ {"GET /index.html HTTP/1.0\n", ParseResult::DONE, 25},
+ {"GET /index.html HTTP/1.0\n\n", ParseResult::DONE, 26},
+ {"GET /index.html HTTP/1.0\r\n\r\n", ParseResult::DONE, 28},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar", ParseResult::ERROR, 44},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\n", ParseResult::DONE, 45},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n\r\n", ParseResult::DONE, 48},
+ {"GET /index.html HTTP/1.0\nUser-Agent: foobar\n", ParseResult::DONE, 44},
+ {"GET /index.html HTTP/1.0\nUser-Agent: foobar\nBoo: foo\n", ParseResult::DONE, 53},
+ {"GET /index.html HTTP/1.0\r\nUser-Agent: foobar\r\n", ParseResult::DONE, 46},
+ {"GET /index.html HTTP/1.0\r\n", ParseResult::DONE, 26},
+ {"GET /index.html hTTP/1.0\r\n", ParseResult::ERROR, 26},
+ {"POST /index.html HTTP/1.0\r\nContent-Length: 0\r\n\r\n", ParseResult::DONE, 48},
+ {"POST /index.html HTTP/1.0\r\nContent-Length: \r\n\r\n", ParseResult::ERROR, 47},
+ {"POST /index.html HTTP/1.0\r\nContent-Length:\r\n\r\n", ParseResult::ERROR, 46},
+ {"CONNECT foo.example HTTP/1.1\r\n", ParseResult::DONE, 30},
+ {"GET foo.example HTTP/1.1\r\n", ParseResult::ERROR, 26},
+ {"", ParseResult::ERROR, 0 },
};
- HTTPParser parser;
+ auto test = GENERATE(from_range(http_parse_tests));
+ CAPTURE(test.msg, test.expected_result, test.expected_bytes_consumed);
+ HTTPParser parser;
http_parser_init(&parser);
- for (auto const &test : tests) {
- HTTPHdr req_hdr;
- HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); // extra to prevent proxy allocation.
-
- req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap);
+ HTTPHdr req_hdr;
+ HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); // extra to prevent proxy allocation.
- http_parser_clear(&parser);
+ req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap);
Review Comment:
Note that this http_parser_clear and code like this isn't needed now that GENERATE creates its own section for each which starts state over between iterations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Catch2: Use CAPTURE and GENERATE [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #12731:
URL: https://github.com/apache/trafficserver/pull/12731#discussion_r2590417507
##########
src/proxy/hdrs/unit_tests/test_URL.cc:
##########
@@ -76,16 +80,9 @@ TEST_CASE("Validate Scheme", "[proxy][validscheme]")
{"example://", false}
};
- for (auto i : scheme_test_cases) {
- // it's pretty hard to debug with
- // CHECK(validate_scheme(i.text) == i.valid);
-
- std::string_view text = i.text;
- if (validate_scheme(text) != i.valid) {
- std::printf("Validation of scheme: \"%s\", expected %s, but not\n", text.data(), (i.valid ? "true" : "false"));
- CHECK(false);
- }
- }
Review Comment:
Note that the workarounds for this aren't necessary anymore with the CAPTURE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org