In commit "Add test coverage for ConnectStream" (44d191420c6fdc83d2c1da159a409c4c18151cbe)
Having the init_sockets callback and the client_fd and server_fd members seems unnecessarily complicated given that nothing else in the test setup class uses them. Would seem simpler to drop these
<details><summary>diff</summary>
<p>
--- a/test/mp/test/connect_tests.cpp
+++ b/test/mp/test/connect_tests.cpp
@@ -9,16 +9,15 @@
#include <kj/test.h>
#include <mp/proxy.h>
#include <mp/proxy-io.h>
+#include <mp/util.h>
#include <mp/test/foo.capnp.h>
#include <mp/test/foo.capnp.proxy.h>
#include <sys/socket.h>
-#include <sys/types.h>
#include <unistd.h>
#include <chrono>
#include <condition_variable>
#include <cstring> // IWYU pragma: keep
-#include <functional>
#include <future>
#include <memory>
#include <mutex>
@@ -45,9 +44,6 @@ void DefaultLogHandler(mp::LogMessage log)
class TestSetup
{
public:
- int client_fd;
- int server_fd;
-
mp::EventLoop* loop;
std::optional<mp::EventLoopRef> loop_ref;
//! Thread variable should be after other struct members so the thread does
@@ -55,14 +51,6 @@ public:
std::thread loop_thread;
TestSetup(mp::LogFn log_handler = DefaultLogHandler)
- : TestSetup(
- [](int fds[2]) {
- KJ_REQUIRE(socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != -1);
- },
- log_handler) {}
-
- TestSetup(const std::function<void(int[2])>& init_sockets,
- mp::LogFn log_handler = DefaultLogHandler)
{
std::promise<mp::EventLoop*> loop_promise;
loop_thread = std::thread([&, log_handler] {
@@ -72,13 +60,6 @@ public:
});
loop = loop_promise.get_future().get();
loop_ref.emplace(*loop);
-
- // Initialize and store sockets
- int fds[2] = {-1, -1};
- init_sockets(fds);
-
- client_fd = fds[0];
- server_fd = fds[1];
}
~TestSetup()
@@ -91,15 +72,16 @@ public:
KJ_TEST("ConnectStream connects to a socket serving a valid init interface")
{
TestSetup setup;
+ auto [client_fd, server_fd] = SocketPair();
- std::thread server_thread([&setup]() {
+ std::thread server_thread([&]() {
mp::EventLoop server_loop("mptest-valid-server", DefaultLogHandler);
std::unique_ptr<FooInit> init = std::make_unique<FooInit>();
- ServeStream<messages::FooInit>(server_loop, MakeStream(server_loop, setup.server_fd), *init);
+ ServeStream<messages::FooInit>(server_loop, MakeStream(server_loop, server_fd), *init);
server_loop.loop();
});
- auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, client_fd));
init.reset();
server_thread.join();
@@ -109,11 +91,12 @@ KJ_TEST("ConnectStream connects to a socket serving a valid init interface")
KJ_TEST("ConnectStream throws when the socket is already disconnected")
{
TestSetup setup;
+ auto [client_fd, server_fd] = SocketPair();
- close(setup.server_fd);
+ close(server_fd);
try {
- auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, client_fd));
KJ_EXPECT(false);
} catch (const std::runtime_error& e) {
@@ -126,12 +109,13 @@ KJ_TEST("ConnectStream throws when the socket is already disconnected")
KJ_TEST("ConnectStream defers disconnect failure to the first IPC request for interfaces without construct()")
{
TestSetup setup;
+ auto [client_fd, server_fd] = SocketPair();
- close(setup.server_fd);
+ close(server_fd);
// Without a construct() method no IPC call is made during client
// creation, so ConnectStream succeeds even though the peer is gone.
- auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, client_fd));
try {
foo->add(1, 2);
@@ -157,10 +141,11 @@ KJ_TEST("ConnectStream handles a disconnect when no client calls are made")
}
DefaultLogHandler(log);
});
+ auto [client_fd, server_fd] = SocketPair();
- close(setup.server_fd);
+ close(server_fd);
- auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, client_fd));
// The disconnect handler registered by ProxyClientBase should run and
// delete the connection even when no calls are ever made.
@@ -171,20 +156,21 @@ KJ_TEST("ConnectStream handles a disconnect when no client calls are made")
KJ_TEST("ConnectStream throws when the socket disconnects after receiving data")
{
TestSetup setup;
+ auto [client_fd, server_fd] = SocketPair();
- std::thread server_thread([&setup]() {
+ std::thread server_thread([&]() {
char buf[128];
ssize_t bytes_received =
- recv(setup.server_fd, buf, sizeof(buf), 0);
+ recv(server_fd, buf, sizeof(buf), 0);
if (bytes_received > 0) {
- close(setup.server_fd);
+ close(server_fd);
}
});
try {
- auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, client_fd));
if (server_thread.joinable()) server_thread.join();
KJ_EXPECT(false);
@@ -199,16 +185,14 @@ KJ_TEST("ConnectStream throws when the socket disconnects after receiving data")
KJ_TEST("ConnectStream throws when a connection accepted from a listener disconnects after receiving data")
{
UnixListener listener;
+ TestSetup setup;
+ int client_fd = listener.MakeConnectedSocket();
+ int server_fd = listener.release();
- TestSetup setup([&listener](int fds[2]) {
- fds[0] = listener.MakeConnectedSocket(); // client_fd
- fds[1] = listener.release(); // server_fd
- });
-
- std::thread server_thread([&setup]() {
+ std::thread server_thread([&]() {
char buf[128];
- int connection_fd = accept(setup.server_fd, nullptr, nullptr);
+ int connection_fd = accept(server_fd, nullptr, nullptr);
if (connection_fd >= 0) {
ssize_t bytes_received =
@@ -218,11 +202,11 @@ KJ_TEST("ConnectStream throws when a connection accepted from a listener disconn
close(connection_fd);
}
}
- close(setup.server_fd);
+ close(server_fd);
});
try {
- auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
+ auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, client_fd));
if (server_thread.joinable()) server_thread.join();
KJ_EXPECT(false);
</p>
</details>
