brbzull0 commented on code in PR #13649:
URL: https://github.com/apache/trafficserver/pull/13649#discussion_r3955897146
##########
src/mgmt/rpc/server/unit_tests/test_rpcserver.cc:
##########
@@ -77,6 +77,42 @@ add_method_handler(const std::string &name, Func &&call)
{
return rpc::JsonRPCManager::instance().add_method_handler(name,
std::forward<Func>(call), nullptr, {});
}
+
+/// 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)}
+ {
+ _registered = rpc::add_method_handler(_name, std::forward<Func>(call));
+ }
+
+ ~ScopedMethodHandler()
+ {
+ if (_registered) {
+ rpc::test_remove_handler(_name);
+ }
+ }
Review Comment:
Good call, done in 70789d1d5.
`CHECK` is the right macro here and I have verified it behaves in a
destructor
that runs during unwinding. Added the `CHECK`, then injected a fatal
`REQUIRE(false)` inside a SECTION so the guard is destroyed mid-unwind:
```
test cases: 14 | 13 passed | 1 failed
assertions: 75 | 74 passed | 1 failed
```
One failure, the injected one. No abort, and the cascade this PR is about
stays
collapsed to a single report.
For the record on reachability: `Dispatcher::remove_handler` returns `false`
only when the name is not found, and the destructor only calls it when
registration succeeded, so there is no path in the current tests that can
trip it. The header comment reads more alarming than the code behaves
("@return false If we could not remove it"). Worth asserting anyway -- a
handler silently not being cleaned up is precisely the failure mode this PR
exists to fix, so verifying the cleanup belongs here.
--
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]
