brbzull0 commented on code in PR #13649:
URL: https://github.com/apache/trafficserver/pull/13649#discussion_r3957373658
##########
src/mgmt/rpc/server/unit_tests/test_rpcserver.cc:
##########
@@ -81,6 +82,53 @@ add_method_handler(const std::string &name, Func &&call)
namespace
{
+/// Registers a method handler and removes it when the scope ends.
+///
+/// Catch2 re-runs a TEST_CASE body once per leaf SECTION. Registering at the
top of the body and
+/// removing at the bottom only works while every assertion passes: REQUIRE is
fatal, so a failure
+/// inside a SECTION unwinds before the trailing removal and the handler
survives into the next
+/// SECTION's run, where re-registering it fails. One real failure then
reports as two, and the
+/// second points at a registration that was never the problem.
+class ScopedMethodHandler
+{
+public:
+ template <typename Func> ScopedMethodHandler(std::string name, Func &&call)
: _name{std::move(name)}
Review Comment:
Added in 6c9e1399a.
One correction to the rationale: a single `std::string` cannot convert here,
since
the constructor takes two required parameters. And the class is non-copyable
with
move suppressed, so it cannot be passed or returned by value either -- the
implicit
conversion path is narrower than described.
What `explicit` does block is `ScopedMethodHandler x = {"name", func};`. The
intent
argument is the one that carries it: this type is meant to be constructed
deliberately at a specific scope, and the keyword costs nothing. All eight
call
sites already use direct brace initialisation, so nothing changed
behaviourally --
confirmed by rebuilding the translation unit with its own
`compile_commands.json`
flags, exit 0 with no diagnostics.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
