pw_rpc: Make C++ integration test code shareable
- Move C++ client integration test setup code to an integration testing
library.
- Update pw_rpc/testing.py to use the same arguments for both binaries.
- Support generating a temp directory for integration tests.
- Always create the integration_tests group, but only add it to the
default target if pw_RUN_INTEGRATION_TESTS is set.
Change-Id: Ib8b6fdd62ead7520ecdbcaddb9fa836db0433700
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/69380
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
diff --git a/BUILD.gn b/BUILD.gn
index d6a9364..ddf6321 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -49,12 +49,15 @@
deps = [
":docs",
":host",
- ":integration_tests",
":static_analysis",
":stm32f429i",
"$dir_pw_env_setup:python.lint",
"$dir_pw_env_setup:python.tests",
]
+
+ if (pw_RUN_INTEGRATION_TESTS) {
+ deps += [ ":integration_tests" ]
+ }
}
# This template generates a group that builds pigweed_default with a particular
@@ -146,15 +149,13 @@
# Tests larger than unit tests, typically run in a specific configuration. Only
# added if pw_RUN_INTEGRATION_TESTS is true.
group("integration_tests") {
- if (pw_RUN_INTEGRATION_TESTS) {
- _default_tc = _default_toolchain_prefix + pw_default_optimization_level
- deps = [
- "$dir_pw_rpc:cpp_client_server_integration_test($_default_tc)",
- "$dir_pw_rpc/py:python_client_cpp_server_test.action($_default_tc)",
- "$dir_pw_transfer/py:python_cpp_transfer_test.action($_default_tc)",
- "$dir_pw_unit_test/py:rpc_service_test.action($_default_tc)",
- ]
- }
+ _default_tc = _default_toolchain_prefix + pw_default_optimization_level
+ deps = [
+ "$dir_pw_rpc:cpp_client_server_integration_test($_default_tc)",
+ "$dir_pw_rpc/py:python_client_cpp_server_test.action($_default_tc)",
+ "$dir_pw_transfer/py:python_cpp_transfer_test.action($_default_tc)",
+ "$dir_pw_unit_test/py:rpc_service_test.action($_default_tc)",
+ ]
}
# OSS-Fuzz uses this target to build fuzzers alone.
diff --git a/pw_rpc/BUILD.bazel b/pw_rpc/BUILD.bazel
index 3ba6300..ad1c820 100644
--- a/pw_rpc/BUILD.bazel
+++ b/pw_rpc/BUILD.bazel
@@ -49,7 +49,7 @@
],
)
-# TODO(hepler): Build this as a cc_binary and use it in integration tests.
+# TODO(pwbug/507): Build this as a cc_binary and use it in integration tests.
filegroup(
name = "test_rpc_server",
srcs = ["test_rpc_server.cc"],
@@ -162,6 +162,28 @@
],
)
+# TODO(pwbug/507): Enable this library when logging_event_handler can be used.
+filegroup(
+ name = "integration_testing",
+ srcs = [
+ "integration_testing.cc",
+ # ],
+ # hdrs = [
+ "public/pw_rpc/integration_test_socket_client.h",
+ "public/pw_rpc/integration_testing.h",
+ ],
+ #deps = [
+ # ":client",
+ # "//pw_assert",
+ # "//pw_hdlc:pw_rpc",
+ # "//pw_hdlc:rpc_channel_output",
+ # "//pw_log",
+ # "//pw_stream:socket_stream",
+ # "//pw_unit_test",
+ # "//pw_unit_test:logging_event_handler",
+ #],
+)
+
# TODO(pwbug/507): Add the client integration test to the build.
filegroup(
name = "client_integration_test",
diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
index 32e0507..e969e25 100644
--- a/pw_rpc/BUILD.gn
+++ b/pw_rpc/BUILD.gn
@@ -217,6 +217,24 @@
visibility = [ "./*" ]
}
+pw_source_set("integration_testing") {
+ public = [
+ "public/pw_rpc/integration_test_socket_client.h",
+ "public/pw_rpc/integration_testing.h",
+ ]
+ sources = [ "integration_testing.cc" ]
+ public_deps = [
+ ":client",
+ "$dir_pw_hdlc:pw_rpc",
+ "$dir_pw_hdlc:rpc_channel_output",
+ "$dir_pw_stream:socket_stream",
+ "$dir_pw_unit_test:logging_event_handler",
+ dir_pw_assert,
+ dir_pw_unit_test,
+ ]
+ deps = [ dir_pw_log ]
+}
+
pw_executable("test_rpc_server") {
sources = [ "test_rpc_server.cc" ]
deps = [
@@ -232,13 +250,9 @@
sources = [ "client_integration_test.cc" ]
deps = [
":client",
+ ":integration_testing",
":protos.raw_rpc",
- "$dir_pw_hdlc:pw_rpc",
- "$dir_pw_hdlc:rpc_channel_output",
- "$dir_pw_stream:socket_stream",
"$dir_pw_sync:binary_semaphore",
- "$dir_pw_unit_test:logging_event_handler",
- dir_pw_assert,
dir_pw_log,
dir_pw_unit_test,
]
@@ -251,10 +265,11 @@
pw_python_action("cpp_client_server_integration_test") {
script = "py/pw_rpc/testing.py"
args = [
+ "--server",
"<TARGET_FILE(:test_rpc_server)>",
- "$pw_rpc_CPP_CLIENT_INTEGRATION_TEST_PORT",
- "--",
+ "--client",
"<TARGET_FILE(:client_integration_test)>",
+ "--",
"$pw_rpc_CPP_CLIENT_INTEGRATION_TEST_PORT",
]
deps = [
diff --git a/pw_rpc/client_integration_test.cc b/pw_rpc/client_integration_test.cc
index d326cd3..f376efa 100644
--- a/pw_rpc/client_integration_test.cc
+++ b/pw_rpc/client_integration_test.cc
@@ -13,38 +13,19 @@
// the License.
#include <cstring>
-#include <thread>
#include "gtest/gtest.h"
#include "pw_assert/check.h"
-#include "pw_hdlc/rpc_channel.h"
-#include "pw_hdlc/rpc_packets.h"
#include "pw_log/log.h"
#include "pw_rpc/benchmark.raw_rpc.pb.h"
-#include "pw_stream/socket_stream.h"
+#include "pw_rpc/integration_testing.h"
#include "pw_sync/binary_semaphore.h"
-#include "pw_unit_test/framework.h"
-#include "pw_unit_test/logging_event_handler.h"
namespace rpc_test {
namespace {
-constexpr size_t kMaxTransmissionUnit = 512;
-constexpr uint32_t kChannelId = 1;
constexpr int kIterations = 3;
-pw::stream::SocketStream stream;
-pw::hdlc::RpcChannelOutputBuffer<kMaxTransmissionUnit> channel_output(
- stream, pw::hdlc::kDefaultRpcAddress, "socket");
-pw::rpc::Channel channel =
- pw::rpc::Channel::Create<kChannelId>(&channel_output);
-
-} // namespace
-
-pw::rpc::Client client(std::span(&channel, 1));
-
-namespace {
-
using namespace std::chrono_literals;
using pw::ByteSpan;
using pw::ConstByteSpan;
@@ -54,29 +35,8 @@
using pw::rpc::pw_rpc::raw::Benchmark;
-void ProcessClientPackets() {
- std::byte decode_buffer[kMaxTransmissionUnit];
- pw::hdlc::Decoder decoder(decode_buffer);
-
- while (true) {
- std::byte byte[1];
- pw::Result<ByteSpan> read = stream.Read(byte);
-
- if (!read.ok() || read->size() == 0u) {
- continue;
- }
-
- if (auto result = decoder.Process(*byte); result.ok()) {
- pw::hdlc::Frame& frame = result.value();
- if (frame.address() == pw::hdlc::kDefaultRpcAddress) {
- PW_CHECK_OK(client.ProcessPacket(frame.data()),
- "Processing a packet failed");
- }
- }
- }
-}
-
-constexpr Benchmark::Client kServiceClient(client, kChannelId);
+Benchmark::Client kServiceClient(pw::rpc::integration_test::client(),
+ pw::rpc::integration_test::kChannelId);
class StringReceiver {
public:
@@ -134,20 +94,8 @@
} // namespace rpc_test
int main(int argc, char* argv[]) {
- if (argc != 2) {
- PW_LOG_ERROR("Usage: %s PORT", argv[0]);
+ if (!pw::rpc::integration_test::InitializeClient(argc, argv).ok()) {
return 1;
}
-
- const int port = std::atoi(argv[1]);
-
- PW_LOG_INFO("Connecting to pw_rpc client at localhost:%d", port);
- PW_CHECK_OK(rpc_test::stream.Connect("localhost", port));
-
- PW_LOG_INFO("Starting pw_rpc client");
- std::thread{rpc_test::ProcessClientPackets}.detach();
-
- pw::unit_test::LoggingEventHandler log_test_events;
- pw::unit_test::RegisterEventHandler(&log_test_events);
return RUN_ALL_TESTS();
}
diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
index 3948631..11b5468 100644
--- a/pw_rpc/docs.rst
+++ b/pw_rpc/docs.rst
@@ -852,12 +852,12 @@
client_server.ProcessPacket(packet, output);
}
-Unit testing
-============
+Testing
+=======
``pw_rpc`` provides utilities for unit testing RPC services and client calls.
-Client testing in C++
----------------------
+Client unit testing in C++
+--------------------------
``pw_rpc`` supports invoking RPCs, simulating server responses, and checking
what packets are sent by an RPC client in tests. Both raw and Nanopb interfaces
are supported. Code that uses the raw API may be tested with the Nanopb test
@@ -915,6 +915,29 @@
EXPECT_TRUE(thing.SetToTrueWhenRpcCompletes());
}
+Integration testing with ``pw_rpc``
+-----------------------------------
+``pw_rpc`` provides utilities to simplify writing integration tests for systems
+that communicate with ``pw_rpc``. The integration test utitilies set up a socket
+to use for IPC between an RPC server and client process.
+
+The server binary uses the system RPC server facade defined
+``pw_rpc_system_server/rpc_server.h``. The client binary uses the functions
+defined in ``pw_rpc/integration_testing.h``:
+
+.. cpp:var:: constexpr uint32_t kChannelId
+
+ The RPC channel for integration test RPCs.
+
+.. cpp:function:: pw::rpc::Client& pw::rpc::integration_test::Client()
+
+ Returns the global RPC client for integration test use.
+
+.. cpp:function:: pw::Status pw::rpc::integration_test::InitializeClient(int argc, char* argv[], const char* usage_args = "PORT")
+
+ Initializes logging and the global RPC client for integration testing. Starts
+ a background thread that processes incoming.
+
Module Configuration Options
============================
The following configurations can be adjusted via compile-time configuration of
diff --git a/pw_rpc/integration_testing.cc b/pw_rpc/integration_testing.cc
new file mode 100644
index 0000000..4212d4b
--- /dev/null
+++ b/pw_rpc/integration_testing.cc
@@ -0,0 +1,54 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+#include "pw_rpc/integration_testing.h"
+
+#include <limits>
+
+#include "gtest/gtest.h"
+#include "pw_log/log.h"
+#include "pw_rpc/integration_test_socket_client.h"
+#include "pw_unit_test/logging_event_handler.h"
+
+namespace pw::rpc::integration_test {
+namespace {
+
+SocketClientContext<512> context;
+unit_test::LoggingEventHandler log_test_events;
+
+} // namespace
+
+Client& client() { return context.client(); }
+
+Status InitializeClient(int argc, char* argv[], const char* usage_args) {
+ unit_test::RegisterEventHandler(&log_test_events);
+
+ if (argc < 2) {
+ PW_LOG_INFO("Usage: %s %s", argv[0], usage_args);
+ return Status::InvalidArgument();
+ }
+
+ const int port = std::atoi(argv[1]);
+
+ if (port <= 0 || port > std::numeric_limits<uint16_t>::max()) {
+ PW_LOG_CRITICAL("Port numbers must be between 1 and 65535; %d is invalid",
+ port);
+ return Status::InvalidArgument();
+ }
+
+ PW_LOG_INFO("Connecting to pw_rpc client at localhost:%d", port);
+ return context.Start(port);
+}
+
+} // namespace pw::rpc::integration_test
diff --git a/pw_rpc/nanopb/BUILD.gn b/pw_rpc/nanopb/BUILD.gn
index 36ce36f..db7701c 100644
--- a/pw_rpc/nanopb/BUILD.gn
+++ b/pw_rpc/nanopb/BUILD.gn
@@ -119,6 +119,7 @@
public_configs = [ ":public" ]
public_deps = [
"$dir_pw_sync:binary_semaphore",
+ "..:integration_testing",
"..:protos.nanopb_rpc",
dir_pw_assert,
dir_pw_unit_test,
diff --git a/pw_rpc/nanopb/client_integration_test.cc b/pw_rpc/nanopb/client_integration_test.cc
index 5e4a8e6..07d3ca9 100644
--- a/pw_rpc/nanopb/client_integration_test.cc
+++ b/pw_rpc/nanopb/client_integration_test.cc
@@ -15,14 +15,9 @@
#include "gtest/gtest.h"
#include "pw_assert/check.h"
#include "pw_rpc/benchmark.rpc.pb.h"
+#include "pw_rpc/integration_testing.h"
#include "pw_sync/binary_semaphore.h"
-namespace rpc_test {
-
-extern pw::rpc::Client client;
-
-} // namespace rpc_test
-
namespace nanopb_rpc_test {
namespace {
@@ -36,7 +31,6 @@
using pw::rpc::pw_rpc::nanopb::Benchmark;
constexpr int kIterations = 10;
-constexpr uint32_t kChannelId = 1;
class PayloadReceiver {
public:
@@ -72,7 +66,8 @@
return payload;
}
-constexpr Benchmark::Client kClient(rpc_test::client, kChannelId);
+const Benchmark::Client kClient(pw::rpc::integration_test::client(),
+ pw::rpc::integration_test::kChannelId);
TEST(NanopbRpcIntegrationTest, Unary) {
char value[] = {"hello, world!"};
@@ -101,8 +96,8 @@
}
TEST(NanopbRpcIntegrationTest, Unary_DiscardCalls) {
- // TODO: This is reliable now, yay.
- for (int i = 0; i < 10000; ++i) {
+ constexpr int iterations = PW_RPC_USE_GLOBAL_MUTEX ? 10000 : 1;
+ for (int i = 0; i < iterations; ++i) {
kClient.UnaryEcho(Payload("O_o"));
}
}
diff --git a/pw_rpc/public/pw_rpc/integration_test_socket_client.h b/pw_rpc/public/pw_rpc/integration_test_socket_client.h
new file mode 100644
index 0000000..9dd33e0
--- /dev/null
+++ b/pw_rpc/public/pw_rpc/integration_test_socket_client.h
@@ -0,0 +1,82 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+#include <cstdint>
+#include <span>
+#include <thread>
+
+#include "pw_hdlc/rpc_channel.h"
+#include "pw_hdlc/rpc_packets.h"
+#include "pw_rpc/integration_testing.h"
+#include "pw_status/try.h"
+#include "pw_stream/socket_stream.h"
+
+namespace pw::rpc::integration_test {
+
+// Wraps an RPC client with a socket stream and a channel configured to use it.
+// Useful for integration tests that run across a socket.
+template <size_t kMaxTransmissionUnit>
+class SocketClientContext {
+ public:
+ constexpr SocketClientContext()
+ : channel_output_(stream_, hdlc::kDefaultRpcAddress, "socket"),
+ channel_(Channel::Create<kChannelId>(&channel_output_)),
+ client_(std::span(&channel_, 1)) {}
+
+ Client& client() { return client_; }
+
+ // Connects to the specified host:port and starts a background thread to read
+ // packets from the socket.
+ Status Start(const char* host, uint16_t port) {
+ PW_TRY(stream_.Connect(host, port));
+ std::thread{&SocketClientContext::ProcessPackets, this}.detach();
+ return OkStatus();
+ }
+
+ // Calls Start for localhost.
+ Status Start(uint16_t port) { return Start("localhost", port); }
+
+ private:
+ void ProcessPackets();
+
+ stream::SocketStream stream_;
+ hdlc::RpcChannelOutputBuffer<kMaxTransmissionUnit> channel_output_;
+ Channel channel_;
+ Client client_;
+};
+
+template <size_t kMaxTransmissionUnit>
+void SocketClientContext<kMaxTransmissionUnit>::ProcessPackets() {
+ std::byte decode_buffer[kMaxTransmissionUnit];
+ hdlc::Decoder decoder(decode_buffer);
+
+ while (true) {
+ std::byte byte[1];
+ Result<ByteSpan> read = stream_.Read(byte);
+
+ if (!read.ok() || read->size() == 0u) {
+ continue;
+ }
+
+ if (auto result = decoder.Process(*byte); result.ok()) {
+ hdlc::Frame& frame = result.value();
+ if (frame.address() == hdlc::kDefaultRpcAddress) {
+ PW_ASSERT(client_.ProcessPacket(frame.data()).ok());
+ }
+ }
+ }
+}
+
+} // namespace pw::rpc::integration_test
diff --git a/pw_rpc/public/pw_rpc/integration_testing.h b/pw_rpc/public/pw_rpc/integration_testing.h
new file mode 100644
index 0000000..3f80049
--- /dev/null
+++ b/pw_rpc/public/pw_rpc/integration_testing.h
@@ -0,0 +1,35 @@
+// Copyright 2021 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+#include <cstdint>
+
+#include "pw_rpc/client.h"
+#include "pw_status/status.h"
+
+namespace pw::rpc::integration_test {
+
+// The RPC channel for integration test RPCs.
+inline constexpr uint32_t kChannelId = 1;
+
+// Returns the global RPC client for integration test use.
+Client& client();
+
+// Initializes logging and the global RPC client for integration testing. Starts
+// a background thread that processes incoming.
+Status InitializeClient(int argc,
+ char* argv[],
+ const char* usage_args = "PORT");
+
+} // namespace pw::rpc::integration_test
diff --git a/pw_rpc/py/pw_rpc/testing.py b/pw_rpc/py/pw_rpc/testing.py
index c7cfd84..a9ab4d3 100644
--- a/pw_rpc/py/pw_rpc/testing.py
+++ b/pw_rpc/py/pw_rpc/testing.py
@@ -16,8 +16,11 @@
import argparse
import subprocess
import sys
+import tempfile
import time
-from typing import Sequence
+from typing import Optional, Sequence
+
+TEMP_DIR_MARKER = '(pw_rpc:CREATE_TEMP_DIR)'
def parse_test_server_args(
@@ -55,41 +58,49 @@
def _parse_subprocess_integration_test_args() -> argparse.Namespace:
parser = argparse.ArgumentParser(
description='Executes a test between two subprocesses')
- parser.add_argument('server_command',
- nargs='+',
- help='Command that starts the server')
- parser.add_argument('separator', nargs=1, metavar='--')
+ parser.add_argument('--client', required=True, help='Client binary to run')
+ parser.add_argument('--server', required=True, help='Server binary to run')
parser.add_argument(
- 'client_command',
- nargs='+',
- help='Command that starts the client and runs the test')
+ 'common_args',
+ metavar='-- ...',
+ nargs=argparse.REMAINDER,
+ help=('Arguments to pass to both the server and client; '
+ f'pass {TEMP_DIR_MARKER} to generate a temporary directory'))
- if '-h' in sys.argv or '--help' in sys.argv:
- parser.print_help()
- sys.exit(0)
+ args = parser.parse_args()
- try:
- index = sys.argv.index('--')
- except ValueError:
- parser.error('The server and client commands must be separated by --.')
+ if not args.common_args or args.common_args[0] != '--':
+ parser.error('The common arguments must start with "--"')
- args = argparse.Namespace()
- args.server_command = sys.argv[1:index]
- args.client_command = sys.argv[index + 1:]
+ args.common_args.pop(0)
+
return args
-def execute_integration_test(server_command: Sequence[str],
- client_command: Sequence[str],
- setup_time_s: float = 0.1) -> int:
- server = subprocess.Popen(server_command)
- # TODO(pwbug/508): Replace this delay with some sort of IPC.
- time.sleep(setup_time_s)
+def execute_integration_test(server: str,
+ client: str,
+ common_args: Sequence[str],
+ setup_time_s: float = 0.2) -> int:
+ temp_dir: Optional[tempfile.TemporaryDirectory] = None
- result = subprocess.run(client_command).returncode
+ if TEMP_DIR_MARKER in common_args:
+ temp_dir = tempfile.TemporaryDirectory(prefix='pw_rpc_test_')
+ common_args = [
+ temp_dir.name if a == TEMP_DIR_MARKER else a for a in common_args
+ ]
- server.terminate()
- server.communicate()
+ try:
+ server_process = subprocess.Popen([server, *common_args])
+ # TODO(pwbug/508): Replace this delay with some sort of IPC.
+ time.sleep(setup_time_s)
+
+ result = subprocess.run([client, *common_args]).returncode
+
+ server_process.terminate()
+ server_process.communicate()
+ finally:
+ if temp_dir:
+ temp_dir.cleanup()
return result
