cmcfarlen commented on code in PR #13658:
URL: https://github.com/apache/trafficserver/pull/13658#discussion_r3973348729
##########
src/mgmt/rpc/server/unit_tests/test_rpcserver.cc:
##########
@@ -185,12 +185,15 @@ struct RPCServerTestListener : Catch::EventListenerBase {
R"(", "backlog": 5,"max_retry_on_transient_errors": 64,
"incoming_request_max_size": 32000 }}})"};
YAML::Node configNode = YAML::Load(confStr);
serverConfig.load(configNode["rpc"]);
+ // Report this loudly: otherwise every socket test just fails later with a
+ // confusing "no such file" on the socket path, which is especially noisy
+ // now that each test case runs as its own process.
try {
jsonrpcServer = new rpc::RPCServer(serverConfig);
jsonrpcServer->start_thread();
} catch (std::exception const &ex) {
- Dbg(dbg_ctl, "Oops: %s", ex.what());
+ std::fprintf(stderr, "Failed to start the JSONRPC test server on %s:
%s\n", sockPath.c_str(), ex.what());
Review Comment:
I tried fail-fast first and backed it out, because the mechanism doesn't
work: **a failing `REQUIRE` inside a Catch2 listener callback segfaults.**
Minimal repro against the vendored Catch2 v3.9.1:
```cpp
struct Listener : Catch::EventListenerBase {
using EventListenerBase::EventListenerBase;
void testRunStarting(Catch::TestRunInfo const &) override {
REQUIRE(false); }
};
CATCH_REGISTER_LISTENER(Listener)
TEST_CASE("some case") { CHECK(1 == 1); }
```
`exit 139` (SIGSEGV), no output at all. Note this makes the *pre-existing*
`REQUIRE(setup_ok)` a few lines above a latent landmine — it just never fires
today.
Rethrowing does work mechanically (prints the message, exits 1), but it
fails all 14 cases including the 6 that never touch the server —
`Layout::create()`, `RecProcessInit()` and `eventProcessor.start()` all
succeeded, so those cases are still valid. I'd rather not convert valid passes
into failures.
On the cascade concern itself: the message goes to stderr before Catch2
starts, so ctest captures it as the first line of the failing test's output,
e.g.
```
Failed to start the JSONRPC test server on /var/.../ats_rpc_I6J7pB/s:
Operation not permitted
```
That names the root cause directly, which is what the old swallowed `Dbg`
did not. For what it's worth, this is how I diagnosed the failure in the first
place — it's environment-specific (macOS under load) and these cases pass in CI.
Happy to revisit if you'd prefer the blanket rethrow.
--
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]
