diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 88666e2..6029272 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -1,4 +1,26 @@ -load("@rules_cc//cc:defs.bzl", "cc_test") +load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_test") + +cc_binary( + name = "death-client", + srcs = [ + "death-client.cc", + ], + deps = [ + "//:grpc", + "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc", + ], +) + +cc_binary( + name = "death-server", + srcs = [ + "death-server.cc", + ], + deps = [ + "//:grpc", + "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc", + ], +) cc_test( name = "grpc", @@ -13,6 +35,7 @@ cc_test( "greeter-server.cc", "helloworld.eventuals.cc", "helloworld.eventuals.h", + "main.cc", "multiple-hosts.cc", "server-death-test.cc", "server-unavailable.cc", @@ -21,13 +44,18 @@ cc_test( "unary.cc", "unimplemented.cc", ], + data = [ + ":death-client", + ":death-server", + ], # NOTE: need to add 'linkstatic = True' in order to get this to # link until https://github.com/grpc/grpc/issues/13856 gets # resolved. linkstatic = True, deps = [ "//:grpc", - "@com_github_google_googletest//:gtest_main", + "@bazel_tools//tools/cpp/runfiles", + "@com_github_google_googletest//:gtest", "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc", "@com_github_grpc_grpc//examples/protos:keyvaluestore", ], diff --git a/test/cancelled-by-client.cc b/test/cancelled-by-client.cc index 76c6236..c54972b 100644 --- a/test/cancelled-by-client.cc +++ b/test/cancelled-by-client.cc @@ -56,7 +56,7 @@ TEST_F(EventualsGrpcTest, CancelledByClient) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/cancelled-by-server.cc b/test/cancelled-by-server.cc index dc8b78d..e8c24e3 100644 --- a/test/cancelled-by-server.cc +++ b/test/cancelled-by-server.cc @@ -57,7 +57,7 @@ TEST_F(EventualsGrpcTest, CancelledByServer) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/client-death-test.cc b/test/client-death-test.cc index 4e8ffa6..f080653 100644 --- a/test/client-death-test.cc +++ b/test/client-death-test.cc @@ -1,9 +1,6 @@ -#include "eventuals/eventual.h" -#include "eventuals/grpc/client.h" #include "eventuals/grpc/server.h" #include "eventuals/head.h" #include "eventuals/let.h" -#include "eventuals/loop.h" #include "eventuals/terminal.h" #include "eventuals/then.h" #include "examples/protos/helloworld.grpc.pb.h" @@ -14,17 +11,11 @@ using helloworld::Greeter; using helloworld::HelloReply; using helloworld::HelloRequest; -using stout::Borrowable; - -using eventuals::Eventual; using eventuals::Head; using eventuals::Let; -using eventuals::Loop; using eventuals::Terminate; using eventuals::Then; -using eventuals::grpc::Client; -using eventuals::grpc::CompletionPool; using eventuals::grpc::Server; using eventuals::grpc::ServerBuilder; @@ -35,57 +26,57 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) { // https://github.com/grpc/grpc/issues/14055) and (2) the server can // send the client it's port. struct { - int fork[2]; - int port[2]; + int fork[2]; // 'fork[0]' is for reading, 'fork[1]' for writing. + int port[2]; // 'port[0]' is for reading, 'port[1]' for writing. } pipes; ASSERT_NE(-1, pipe(pipes.fork)); ASSERT_NE(-1, pipe(pipes.port)); - auto wait_for_fork = [&]() { + auto WaitForFork = [&]() { int _; CHECK_GT(read(pipes.fork[0], &_, sizeof(int)), 0); }; - auto notify_forked = [&]() { - int _ = 1; - CHECK_GT(write(pipes.fork[1], &_, sizeof(int)), 0); - }; - - auto wait_for_port = [&]() { - int port; - CHECK_GT(read(pipes.port[0], &port, sizeof(int)), 0); - return port; - }; - - auto send_port = [&](int port) { + auto SendPort = [&](int port) { CHECK_GT(write(pipes.port[1], &port, sizeof(port)), 0); }; - auto client = [&]() { - notify_forked(); - - int port = wait_for_port(); - - Borrowable pool; - - Client client( - "0.0.0.0:" + stringify(port), - grpc::InsecureChannelCredentials(), - pool.Borrow()); - - auto call = [&]() { - return client.Call("SayHello") - | Then([](auto&& call) { - exit(1); - }); - }; - - *call(); - }; - + // We use a thread to fork/exec the 'death-client' so that we can + // simultaneously run and wait for the client to die while also + // running the server. std::thread thread([&]() { - ASSERT_DEATH(client(), ""); + std::string path = GetRunfilePathFor("death-client").string(); + + std::string pipe_fork = std::to_string(pipes.fork[1]); + std::string pipe_port = std::to_string(pipes.port[0]); + + // NOTE: doing a 'fork()' when a parent has multiple threads is + // wrought with potential issues because the child only gets a + // single one of those threads (the one that called 'fork()') so + // we ensure here that there is only the single extra thread. + ASSERT_EQ(GetThreadCount(), 2); + + ASSERT_DEATH( + [&]() { + // Conventional wisdom is to do the least amount possible + // after a 'fork()', ideally just an 'exec*()', and that's + // what we're doing because when we tried to do more the + // tests were flaky likely do to some library that doesn't + // properly work after doing a 'fork()'. + execl( + path.c_str(), + path.c_str(), + pipe_fork.c_str(), + pipe_port.c_str(), + nullptr); + + // NOTE: if 'execve()' failed then this lamdba will return + // and gtest will see consider this "death" to be a failure. + }(), + // NOTE: to avoid false positives we check for a death with + // the string 'connected' printed to stderr. + "connected"); }); // NOTE: we detach the thread so that there isn't a race with the @@ -97,7 +88,9 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) { // test which might have been destructed before it destructs. thread.detach(); - wait_for_fork(); + // NOTE: need to wait to call into gRPC till _after_ we've forked + // (see comment at top of test for more details). + WaitForFork(); ServerBuilder builder; @@ -128,7 +121,9 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) { k.Start(); - send_port(port); + // NOTE: sending this _after_ we start the eventual so that we're + // ready to accept clients! + SendPort(port); EXPECT_TRUE(cancelled.get()); diff --git a/test/deadline.cc b/test/deadline.cc index c4c697f..c2f71b9 100644 --- a/test/deadline.cc +++ b/test/deadline.cc @@ -59,7 +59,7 @@ TEST_F(EventualsGrpcTest, Deadline) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/death-client.cc b/test/death-client.cc new file mode 100644 index 0000000..c0fe361 --- /dev/null +++ b/test/death-client.cc @@ -0,0 +1,77 @@ +#include "eventuals/grpc/client.h" +#include "eventuals/terminal.h" +#include "eventuals/then.h" +#include "examples/protos/helloworld.grpc.pb.h" + +using helloworld::Greeter; +using helloworld::HelloReply; +using helloworld::HelloRequest; + +using stout::Borrowable; + +using eventuals::Eventual; +using eventuals::Terminate; +using eventuals::Then; + +using eventuals::grpc::Client; +using eventuals::grpc::CompletionPool; + +// Should only be run from tests! +// +// Expects two arguments. +// +// Expects 'argv[1]' to be a string representing the file descriptor +// that this process has inherited from its parent (the test) that can +// be used to indicate that forking has completed and the test can +// continue. +// +// Expects 'argv[2]' to be a string representing the file descriptor +// that this process has inherited from its parent (the test) that can +// be used to read the bound port of the gRPC server to connect to. +// +// See 'client-death-test.cc' for more details. +int main(int argc, char** argv) { + // TODO(benh): use stout-flags! + CHECK_EQ(argc, 3) + << "expecting 'pipe_fork' and 'pipe_port' to be passed as arguments"; + + int pipe_fork = atoi(argv[1]); + int pipe_port = atoi(argv[2]); + + auto NotifyForked = [&]() { + int one = 1; + CHECK_GT(write(pipe_fork, &one, sizeof(int)), 0); + }; + + auto WaitForPort = [&]() { + int port; + CHECK_GT(read(pipe_port, &port, sizeof(int)), 0); + return port; + }; + + NotifyForked(); + + int port = WaitForPort(); + + Borrowable pool; + + Client client( + "0.0.0.0:" + std::to_string(port), + grpc::InsecureChannelCredentials(), + pool.Borrow()); + + auto call = [&]() { + return client.Call("SayHello") + | Then([](auto&& call) { + // NOTE: to avoid false positives with, for example, one + // of the 'CHECK()'s failing above, the 'ClientDeathTest' + // expects the string 'connected' to be written to stderr. + std::cerr << "connected" << std::endl; + exit(1); + }); + }; + + *call(); + + return 0; +} diff --git a/test/death-server.cc b/test/death-server.cc new file mode 100644 index 0000000..e645100 --- /dev/null +++ b/test/death-server.cc @@ -0,0 +1,75 @@ +#include "eventuals/grpc/server.h" +#include "eventuals/head.h" +#include "eventuals/terminal.h" +#include "eventuals/then.h" +#include "examples/protos/helloworld.grpc.pb.h" + +using helloworld::Greeter; +using helloworld::HelloReply; +using helloworld::HelloRequest; + +using eventuals::Head; +using eventuals::Terminate; +using eventuals::Then; + +using eventuals::grpc::Server; +using eventuals::grpc::ServerBuilder; + +// Should only be run from tests! +// +// Expects one argument. +// +// Expects as 'argv[1]' a string representing the file descriptor that +// this process has inherited from its parent (the test) that can be +// used to send the bound port of the gRPC server. +// +// See 'server-death-test.cc' for more details. +int main(int argc, char** argv) { + // TODO(benh): use stout-flags! + CHECK_EQ(argc, 2) << "expecting 'pipe' to be passed as an argument"; + + int pipe = atoi(argv[1]); + + auto SendPort = [&](int port) { + CHECK_GT(write(pipe, &port, sizeof(port)), 0); + }; + + ServerBuilder builder; + + int port = 0; + + builder.AddListeningPort( + "0.0.0.0:0", + grpc::InsecureServerCredentials(), + &port); + + auto build = builder.BuildAndStart(); + + CHECK(build.status.ok()); + + auto server = std::move(build.server); + + CHECK(server); + + auto serve = [&]() { + return server->Accept("SayHello") + | Head() + | Then([](auto&& call) { + // NOTE: to avoid false positives with, for example, one + // of the 'CHECK()'s failing above, the 'ServerDeathTest' + // expects the string 'accepted' to be written to stderr. + std::cerr << "accepted" << std::endl; + exit(1); + }); + }; + + auto [future, k] = Terminate(serve()); + + k.Start(); + + // NOTE: sending this _after_ we start the eventual so that we're + // ready to accept clients! + SendPort(port); + + future.get(); +} diff --git a/test/greeter-server.cc b/test/greeter-server.cc index 457e457..00060b7 100644 --- a/test/greeter-server.cc +++ b/test/greeter-server.cc @@ -60,7 +60,7 @@ TEST_F(EventualsGrpcTest, Greeter) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/main.cc b/test/main.cc new file mode 100644 index 0000000..341571f --- /dev/null +++ b/test/main.cc @@ -0,0 +1,50 @@ +#include "gtest/gtest.h" +#include "test/test.h" +#include "tools/cpp/runfiles/runfiles.h" + +//////////////////////////////////////////////////////////////////////// + +// NOTE: using a raw pointer here as per Google C++ Style Guide +// because 'bazel::tools::cpp::runfiles::Runfiles' is not trivially +// destructible. +static bazel::tools::cpp::runfiles::Runfiles* runfiles = nullptr; + +//////////////////////////////////////////////////////////////////////// + +// Declared in test.h. +std::filesystem::path GetRunfilePathFor(const std::filesystem::path& runfile) { + std::string path = CHECK_NOTNULL(runfiles)->Rlocation(runfile); + + CHECK(!path.empty()) << "runfile " << runfile << " does not exist"; + + return path; +} + +//////////////////////////////////////////////////////////////////////// + +int main(int argc, char** argv) { + std::string error; + + // NOTE: using 'Create()' instead of 'CreateForTest()' so that we + // can support both running the test via 'bazel test' or explicitly + // (i.e., ./path/to/test --gtest_...). + runfiles = bazel::tools::cpp::runfiles::Runfiles::Create( + argv[0], + std::string(), + std::filesystem::absolute(argv[0]).parent_path().string(), + &error); + + if (runfiles == nullptr) { + std::cerr + << "Failed to construct 'Runfiles' necessary for getting " + << "paths to assets needed in order to run tests: " + << error << std::endl; + return -1; + } + + ::testing::InitGoogleTest(&argc, argv); + + return RUN_ALL_TESTS(); +} + +//////////////////////////////////////////////////////////////////////// diff --git a/test/multiple-hosts.cc b/test/multiple-hosts.cc index 4298a8a..5d11ffc 100644 --- a/test/multiple-hosts.cc +++ b/test/multiple-hosts.cc @@ -69,7 +69,7 @@ TEST_F(EventualsGrpcTest, MultipleHosts) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/server-death-test.cc b/test/server-death-test.cc index 0371895..b2547b1 100644 --- a/test/server-death-test.cc +++ b/test/server-death-test.cc @@ -1,3 +1,5 @@ +#include + #include "eventuals/grpc/client.h" #include "eventuals/grpc/server.h" #include "eventuals/head.h" @@ -27,57 +29,45 @@ using eventuals::grpc::ServerBuilder; TEST_F(EventualsGrpcTest, ServerDeathTest) { // NOTE: need pipes to get the server's port, this also helps // synchronize when the server is ready to have the client connect. - int pipefds[2]; + int pipefds[2]; // 'port[0]' is for reading, 'port[1]' for writing. ASSERT_NE(-1, pipe(pipefds)); - auto wait_for_port = [&]() { + auto WaitForPort = [&]() { int port; CHECK_GT(read(pipefds[0], &port, sizeof(int)), 0); return port; }; - auto send_port = [&](int port) { - CHECK_GT(write(pipefds[1], &port, sizeof(port)), 0); - }; - - auto server = [&]() { - ServerBuilder builder; - - int port = 0; - - builder.AddListeningPort( - "0.0.0.0:0", - grpc::InsecureServerCredentials(), - &port); - - auto build = builder.BuildAndStart(); - - ASSERT_TRUE(build.status.ok()); - - auto server = std::move(build.server); - - ASSERT_TRUE(server); - - auto serve = [&]() { - return server->Accept("SayHello") - | Head() - | Then([](auto&& call) { - exit(1); - }); - }; - - auto [future, k] = Terminate(serve()); - - k.Start(); - - send_port(port); - - future.get(); - }; - + // We use a thread to fork/exec the 'death-server' so that we can + // simultaneously run and wait for the server to die while also + // running the client. std::thread thread([&]() { - ASSERT_DEATH(server(), ""); + std::string path = GetRunfilePathFor("death-server").string(); + + std::string pipe = std::to_string(pipefds[1]); + + // NOTE: doing a 'fork()' when a parent has multiple threads is + // wrought with potential issues because the child only gets a + // single one of those threads (the one that called 'fork()') so + // we ensure here that there is only the single extra thread. + ASSERT_EQ(GetThreadCount(), 2); + + ASSERT_DEATH( + [&]() { + // Conventional wisdom is to do the least amount possible + // after a 'fork()', ideally just an 'exec*()', and that's + // what we're doing because when we tried to do more the + // tests were flaky likely do to some library that doesn't + // properly work after doing a 'fork()'. + execl(path.c_str(), path.c_str(), pipe.c_str(), nullptr); + + // NOTE: if 'execve()' failed then this lamdba will return + // and gtest will see consider this "death" to be a failure. + }(), + // NOTE: to avoid false positives we check for a death with + // the string 'accepted' printed to stderr. + "accepted"); }); // NOTE: we detach the thread so that there isn't a race with the @@ -89,12 +79,12 @@ TEST_F(EventualsGrpcTest, ServerDeathTest) { // test which might have been destructed before it destructs. thread.detach(); - int port = wait_for_port(); + int port = WaitForPort(); Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/server-unavailable.cc b/test/server-unavailable.cc index b8f731b..8f27486 100644 --- a/test/server-unavailable.cc +++ b/test/server-unavailable.cc @@ -23,7 +23,7 @@ TEST_F(EventualsGrpcTest, ServerUnavailable) { // NOTE: we use 'getpid()' to create a _unique_ UNIX domain socket // path that should never have a server listening on for this test. Client client( - "unix:eventuals-grpc-test-server-unavailable-" + stringify(getpid()), + "unix:eventuals-grpc-test-server-unavailable-" + std::to_string(getpid()), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/streaming.cc b/test/streaming.cc index 8aebc35..004baae 100644 --- a/test/streaming.cc +++ b/test/streaming.cc @@ -85,7 +85,7 @@ void test_client_behavior(Handler handler) { std::vector responses; for (size_t i = 0; i < 3; i++) { keyvaluestore::Response response; - response.set_value(stringify(i + 10)); + response.set_value(std::to_string(i + 10)); responses.push_back(response); } return Iterate(std::move(responses)); @@ -101,7 +101,7 @@ void test_client_behavior(Handler handler) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/test.h b/test/test.h index 37fb602..3c93650 100644 --- a/test/test.h +++ b/test/test.h @@ -1,8 +1,12 @@ #pragma once +#include + #include "eventuals/grpc/server.h" #include "gtest/gtest.h" +//////////////////////////////////////////////////////////////////////// + class EventualsGrpcTest : public ::testing::Test { protected: void SetUp() override { @@ -22,15 +26,12 @@ class EventualsGrpcTest : public ::testing::Test { } }; +//////////////////////////////////////////////////////////////////////// -// TODO(benh): Move to stout-stringify. -template -std::string stringify(const T& t) { - std::ostringstream out; - out << t; - if (!out.good()) { - std::cerr << "Failed to stringify!" << std::endl; - abort(); - } - return out.str(); -} +// Helper which returns a path for the specified runfile. This is a +// wrapper around 'bazel::tools::cpp::runfiles::Runfiles' which +// amongst other things uses 'std::filesystem::path' instead of just +// 'std::string'. +std::filesystem::path GetRunfilePathFor(const std::filesystem::path& runfile); + +//////////////////////////////////////////////////////////////////////// diff --git a/test/unary.cc b/test/unary.cc index 4b9eb1c..991cc2f 100644 --- a/test/unary.cc +++ b/test/unary.cc @@ -69,7 +69,7 @@ TEST_F(EventualsGrpcTest, Unary) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow()); diff --git a/test/unimplemented.cc b/test/unimplemented.cc index 29a4755..dfbb1b5 100644 --- a/test/unimplemented.cc +++ b/test/unimplemented.cc @@ -38,7 +38,7 @@ TEST_F(EventualsGrpcTest, Unimplemented) { Borrowable pool; Client client( - "0.0.0.0:" + stringify(port), + "0.0.0.0:" + std::to_string(port), grpc::InsecureChannelCredentials(), pool.Borrow());