test: listen_tests and connect_tests follow-ups #344

pull xyzconstant wants to merge 9 commits into bitcoin-core:master from xyzconstant:listenconnections-and-connectstream-tests-follow-ups changing 3 files +79 −95

xyzconstant commented at 3:35 AM on August 13, 2026: contributor

Addresses review suggestions left (all of them made by ryanofsky) in the now-merged PRs #298 and #310.

These are non-critical test cleanups (naming, simplification, comments, etc.) with zero changes to library code.
test: replace capnp fix link with upstream PR cc260f2526
test: add m_ prefix to TestSetup members in connect tests 70467c5a72
DrahtBot commented at 3:35 AM on August 13, 2026: none



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK ryanofsky

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Type	Reviewers
ACK	ryanofsky

test: drop TestSetup socket members in connect tests

Use the existing `SocketPair()` helper directly in each test instead of the
`init_sockets` callback and fd members, which nothing else in the class used.

b54a163308

test: share DefaultLogHandler between test files

Move it to a common.h file. The connect_tests copy previously didn't log
messages, now it does through the shared version.

038d33eb31

test: drop mp:: prefixes in connect tests

Unnecessary since the tests are inside the mp namespace.

44bc4630bc

test: join server thread unconditionally in connect tests 113f1d4d28
test: drop unnecessary KJ_EXPECT(true) 7eb741e635

test: close sockets unconditionally and check errors with KJ_SYSCALL

The conditional served no purpose and leaked the descriptor when `recv()`
failed or returned zero.

b9c36c6175

doc: note construct() call in valid init interface test c39c7850c6
xyzconstant force-pushed on Aug 13, 2026
in test/mp/test/common.h:20 in 038d33eb31
```
  15 | +
  16 | +//! Default event loop log handler used by tests. Logs all messages and throws
  17 | +//! on errors so calling code can assert on them.
  18 | +inline void DefaultLogHandler(LogMessage log)
  19 | +{
  20 | +    KJ_LOG(INFO, log.level, log.message);
```
ryanofsky commented at 6:54 PM on August 13, 2026:

In commit "test: share DefaultLogHandler between test files" (038d33eb31eb0d5742a27caeb0c5b244f0f794a1)

Looks like test.cpp is another place this could be used. It would also be nice to take the comment there about showing log output with mptest --verbose
in test/mp/test/connect_tests.cpp:41 in 44bc4630bc
```
  37 | @@ -38,17 +38,17 @@ constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30};
  38 |  class TestSetup
  39 |  {
  40 |  public:
  41 | -    mp::EventLoop* m_loop;
  42 | -    std::optional<mp::EventLoopRef> m_loop_ref;
  43 | +    EventLoop* m_loop;
```
ryanofsky commented at 7:14 PM on August 13, 2026:

In commit "test: drop mp:: prefixes in connect tests" (44bc4630bc18bb03977a3616bf979fb2b3192f54)

Note: looks like there are a few remaining mp:: prefixes that could be dropped in other test files, but reasonable to limit commit to this file
in test/mp/test/connect_tests.cpp:185 in b9c36c6175
```
 186 | -                recv(connection_fd, buf, sizeof(buf), 0);
 187 | -
 188 | -            if (bytes_received > 0) {
 189 | -                close(connection_fd);
 190 | -            }
 191 | +            recv(connection_fd, buf, sizeof(buf), 0);
```
ryanofsky commented at 7:21 PM on August 13, 2026:

In commit "test: close sockets unconditionally and check errors with KJ_SYSCALL" (b9c36c617518c0026f3b8c35014415b7437f6b8a)

Would be nice to capture errors here with KJ_SYSCALL(recv(server_fd, buf, sizeof(buf), 0)); or KJ_SYSCALL(bytes_received = recv(server_fd, buf, sizeof(buf), 0)); or KJ_SYSCALL(bytes_received = recv(...), "receiving client handshake");

in test/mp/test/connect_tests.cpp:184 in b9c36c6175

 181 | @@ -186,14 +182,10 @@ KJ_TEST("ConnectStream throws when a connection accepted from a listener disconn
 182 |          int connection_fd = accept(server_fd, nullptr, nullptr);
 183 |  
 184 |          if (connection_fd >= 0) {

ryanofsky commented at 7:24 PM on August 13, 2026:

In commit "test: close sockets unconditionally and check errors with KJ_SYSCALL" (b9c36c617518c0026f3b8c35014415b7437f6b8a)

Seems probably best to fail if accept fails:

int connection_fd;
KJ_SYSCALL(connection_fd = accept(server_fd, nullptr, nullptr));

ryanofsky approved
ryanofsky commented at 7:28 PM on August 13, 2026: collaborator

Code review ACK c39c7850c663d1915ae2478dc53f2c7b690504db. Thanks for the followup!
ryanofsky commented at 7:30 PM on August 13, 2026: collaborator

Note: I'll probably merge this PR soon since it conflicts with #231, which I'd like to stay up to date. But happy to review more followups
in test/mp/test/connect_tests.cpp:5 in 038d33eb31
```
   0 | @@ -1,6 +1,7 @@
   1 |  // Copyright (c) The Bitcoin Core developers
   2 |  // Distributed under the MIT software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +#include "common.h"
   5 |  #include "unixlistener.h"
```
ryanofsky commented at 7:39 PM on August 13, 2026:

In commit "test: share DefaultLogHandler between test files" (038d33eb31eb0d5742a27caeb0c5b244f0f794a1)

For consistency would be good to use full include paths <mp/test/common.h> We should probably have a linter check for this
ryanofsky merged this on Aug 13, 2026
ryanofsky closed this on Aug 13, 2026

Contributors

xyzconstant

DrahtBot

ryanofsky

Linked (view graph)

#298 Fix error handling when creating clients (`mp::ConnectStream`)#310 test: cover immediate client disconnects for `ListenConnections`

test: listen_tests and connect_tests follow-ups #344

Reviews