pw_rpc: Add Client Server Testing to Pwpb

Move common components into pw_rpc/internal Split threaded from unthreaded version Bug: b/226363461 Change-Id: I31a8ded35afc2ba9522bd498fb08fe95ae92ad24 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/94061 Reviewed-by: Wyatt Hepler <hepler@google.com> Reviewed-by: Scott James Remnant <keybuk@google.com> Commit-Queue: Kacper Kuryllo <kacperkuryllo@google.com>
google · May 27, 2022 · 469c452 · 469c452
1 parent 99fd94c
commit 469c452
Show file tree

Hide file tree

Showing 23 changed files with 1,399 additions and 213 deletions.
diff --git a/pw_rpc/BUILD.bazel b/pw_rpc/BUILD.bazel
@@ -133,6 +133,33 @@ pw_cc_library(
     ],
 )
 
+pw_cc_library(
+    name = "client_server_testing",
+    hdrs = ["public/pw_rpc/internal/client_server_testing.h"],
+    includes = ["public"],
+    deps = [
+        ":client_server",
+        ":internal_test_utils",
+        "//pw_bytes",
+        "//pw_result",
+    ],
+)
+
+pw_cc_library(
+    name = "client_server_testing_threaded",
+    hdrs = ["public/pw_rpc/internal/client_server_testing_threaded.h"],
+    includes = ["public"],
+    deps = [
+        ":client_server_testing",
+        "//pw_bytes",
+        "//pw_result",
+        "//pw_sync:binary_semaphore",
+        "//pw_sync:lock_annotations",
+        "//pw_sync:mutex",
+        "//pw_thread:thread",
+    ],
+)
+
 pw_cc_library(
     name = "thread_testing",
     hdrs = ["public/pw_rpc/thread_testing.h"],

diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
@@ -213,6 +213,33 @@ pw_source_set("fake_channel_output") {
   visibility = [ "./*" ]
 }
 
+pw_source_set("client_server_testing") {
+  public = [ "public/pw_rpc/internal/client_server_testing.h" ]
+  public_configs = [ ":public_include_path" ]
+  public_deps = [
+    ":client_server",
+    ":fake_channel_output",
+    "$dir_pw_bytes",
+    "$dir_pw_result",
+  ]
+  visibility = [ "./*" ]
+}
+
+pw_source_set("client_server_testing_threaded") {
+  public = [ "public/pw_rpc/internal/client_server_testing_threaded.h" ]
+  public_configs = [ ":public_include_path" ]
+  public_deps = [
+    ":client_server_testing",
+    "$dir_pw_bytes",
+    "$dir_pw_result",
+    "$dir_pw_sync:binary_semaphore",
+    "$dir_pw_sync:lock_annotations",
+    "$dir_pw_sync:mutex",
+    "$dir_pw_thread:thread",
+  ]
+  visibility = [ "./*" ]
+}
+
 pw_source_set("thread_testing") {
   public = [ "public/pw_rpc/thread_testing.h" ]
   public_deps = [

diff --git a/pw_rpc/CMakeLists.txt b/pw_rpc/CMakeLists.txt
@@ -126,6 +126,25 @@ pw_add_module_library(pw_rpc.test_utils
 )
 target_include_directories(pw_rpc.test_utils PUBLIC .)
 
+pw_add_module_library(pw_rpc.client_server_testing
+  PUBLIC_DEPS
+    pw_bytes
+    pw_result
+    pw_rpc.client_server
+    pw_rpc.test_utils
+)
+
+pw_add_module_library(pw_rpc.client_server_testing_threaded
+  PUBLIC_DEPS
+    pw_bytes
+    pw_result
+    pw_rpc.client_server_testing
+    pw_sync.binary_semaphore
+    pw_sync.lock_annotations
+    pw_sync.mutex
+    pw_thread.thread
+)
+
 pw_proto_library(pw_rpc.protos
   SOURCES
     internal/packet.proto

diff --git a/pw_rpc/docs.rst b/pw_rpc/docs.rst
@@ -996,10 +996,10 @@ Testing
 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
-helpers, and vice versa. The Nanopb API also provides a test helper with a real
-client-server pair that supports testing of synchronously messaging.
+what packets are sent by an RPC client in tests. Raw, Nanopb and Pwpb interfaces
+are supported. Code that uses the raw API may be tested with the raw test
+helpers, and vice versa. The Nanopb and Pwpb APIs also provides a test helper
+with a real client-server pair that supports testing of asynchronous messaging.
 
 To test sychronous code that invokes RPCs, declare a ``RawClientTestContext``,
 ``PwpbClientTestContext``,  or ``NanopbClientTestContext``. These test context
@@ -1055,9 +1055,12 @@ the expected data was sent and then simulates a response from the server.
     EXPECT_TRUE(thing.SetToTrueWhenRpcCompletes());
   }
 
-To test client code that uses asynchronous responses or encapsulates multiple
-rpc calls to one or more services, declare a ``NanopbClientServerTestContext``.
-This test object is defined in ``pw_rpc/nanopb/client_server_testing.h``.
+To test client code that uses asynchronous responses, encapsulates multiple
+rpc calls to one or more services, or uses a custom service implemenation,
+declare a ``NanopbClientServerTestContextThreaded`` or
+``PwpbClientServerTestContextThreaded``. These test object are defined in
+``pw_rpc/nanopb/client_server_testing_threaded.h`` and
+``pw_rpc/pwpb/client_server_testing_threaded.h``.
 
 Use the context's ``server()`` to register a ``Service`` implementation, and
 ``client()`` and ``channel()`` to invoke RPCs. Create a ``Thread`` using the
@@ -1073,8 +1076,7 @@ response is received. It verifies that expected data was both sent and received.
 .. code-block:: cpp
 
   #include "my_library_protos/my_service.rpc.pb.h"
-  #include "pw_rpc/nanopb/client_server_testing.h"
-  #include "pw_thread/detached_thread.h"
+  #include "pw_rpc/nanopb/client_server_testing_threaded.h"
   #include "pw_thread_stl/options.h"
 
   class ClientUnderTest {
@@ -1098,10 +1100,9 @@ response is received. It verifies that expected data was both sent and received.
   };
 
   TEST(TestServiceTest, ReceivesUnaryRpcReponse) {
-    NanopbClientServerTestContext ctx;
+    NanopbClientServerTestContextThreaded<> ctx(pw::thread::stl::Options{});
     TestService service;
     ctx.server().RegisterService(service);
-    pw::thread::DetachedThread(pw::thread::stl::Options(), ctx);
     ClientUnderTest client(ctx.client(), ctx.channel().id());
 
     // Execute the code that invokes the MyService.TheMethod RPC.
@@ -1116,6 +1117,58 @@ response is received. It verifies that expected data was both sent and received.
     EXPECT_EQ(response.value, value + 1);
   }
 
+Synchronous versions of these test contexts also exist that may be used on
+non-threaded systems ``NanopbClientServerTestContext`` and
+``PwpbClientServerTestContext``. While these do not allow for asynchronous
+messaging they support the use of service implemenations and use a similar
+syntax. When these are used ``.ForwardNewPackets()`` should be called after each
+rpc call to trigger sending of queued messages.
+
+For example, the following tests a class that invokes an RPC that is responded
+to with a test service implemenation.
+
+.. code-block:: cpp
+
+  #include "my_library_protos/my_service.rpc.pb.h"
+  #include "pw_rpc/nanopb/client_server_testing.h"
+
+  class ClientUnderTest {
+   public:
+    ClientUnderTest(pw::rpc::Client& client, uint32_t channel_id);
+
+    Status SendRpcCall(uint32_t value);
+  };
+
+
+  class TestService final : public MyService<TestService> {
+   public:
+    Status TheMethod(const pw_rpc_test_TheMethod& request,
+                        pw_rpc_test_TheMethod& response) {
+      response.value = request.integer + 1;
+      return pw::OkStatus();
+    }
+  };
+
+  TEST(TestServiceTest, ReceivesUnaryRpcReponse) {
+    NanopbClientServerTestContext<> ctx();
+    TestService service;
+    ctx.server().RegisterService(service);
+    ClientUnderTest client(ctx.client(), ctx.channel().id());
+
+    // Execute the code that invokes the MyService.TheMethod RPC.
+    constexpr uint32_t value = 1;
+    const auto result = client.SendRpcCall(value);
+    // Needed after ever RPC call to trigger forward of packets
+    ctx.ForwardNewPackets();
+    const auto request = ctx.request<MyService::TheMethod>(0);
+    const auto response = ctx.resonse<MyService::TheMethod>(0);
+
+    // Verify content of messages
+    EXPECT_EQ(result, pw::OkStatus());
+    EXPECT_EQ(request.value, value);
+    EXPECT_EQ(response.value, value + 1);
+  }
+
 Integration testing with ``pw_rpc``
 -----------------------------------
 ``pw_rpc`` provides utilities to simplify writing integration tests for systems

diff --git a/pw_rpc/nanopb/BUILD.bazel b/pw_rpc/nanopb/BUILD.bazel
@@ -102,13 +102,20 @@ pw_cc_library(
     ],
     includes = ["public"],
     deps = [
-        ":client_testing",
-        "//pw_bytes",
-        "//pw_rpc",
-        "//pw_rpc:client_server",
-        "//pw_sync:binary_semaphore",
-        "//pw_sync:mutex",
-        "//pw_thread:thread_core",
+        ":test_method_context",
+        "//pw_rpc:client_server_testing",
+    ],
+)
+
+pw_cc_library(
+    name = "client_server_testing_threaded",
+    hdrs = [
+        "public/pw_rpc/nanopb/client_server_testing_threaded.h",
+    ],
+    includes = ["public"],
+    deps = [
+        ":test_method_context",
+        "//pw_rpc:client_server_testing_threaded",
     ],
 )
 
@@ -184,10 +191,11 @@ pw_cc_test(
     target_compatible_with = select(TARGET_COMPATIBLE_WITH_HOST_SELECT),
     deps = [
         ":client_api",
-        ":client_server_testing",
+        ":client_server_testing_threaded",
         "//pw_rpc:pw_rpc_test_cc.nanopb_rpc",
         "//pw_sync:binary_semaphore",
-        "//pw_thread:thread",
+        "//pw_thread:test_threads_header",
+        "//pw_thread_stl:test_threads",
     ],
 )
 

diff --git a/pw_rpc/nanopb/BUILD.gn b/pw_rpc/nanopb/BUILD.gn
@@ -106,12 +106,16 @@ pw_source_set("client_testing") {
 pw_source_set("client_server_testing") {
   public = [ "public/pw_rpc/nanopb/client_server_testing.h" ]
   public_deps = [
-    ":client_testing",
-    "$dir_pw_bytes",
-    "$dir_pw_sync:binary_semaphore",
-    "$dir_pw_sync:mutex",
-    "$dir_pw_thread:thread_core",
-    "..:client_server",
+    ":test_method_context",
+    "..:client_server_testing",
+  ]
+}
+
+pw_source_set("client_server_testing_threaded") {
+  public = [ "public/pw_rpc/nanopb/client_server_testing_threaded.h" ]
+  public_deps = [
+    ":test_method_context",
+    "..:client_server_testing_threaded",
   ]
 }
 
@@ -187,28 +191,28 @@ pw_test("client_reader_writer_test") {
   enable_if = dir_pw_third_party_nanopb != ""
 }
 
-_stl_threading_and_nanopb_enabled =
-    pw_thread_THREAD_BACKEND == "$dir_pw_thread_stl:thread" &&
-    pw_sync_BINARY_SEMAPHORE_BACKEND != "" && pw_sync_MUTEX_BACKEND != "" &&
-    dir_pw_third_party_nanopb != ""
-
 pw_test("client_server_context_test") {
   deps = [
     ":client_api",
     ":client_server_testing",
     "..:test_protos.nanopb_rpc",
   ]
   sources = [ "client_server_context_test.cc" ]
-  enable_if = _stl_threading_and_nanopb_enabled
+  enable_if = dir_pw_third_party_nanopb != ""
 }
 
-# requires threading.
+_stl_threading_and_nanopb_enabled =
+    pw_thread_THREAD_BACKEND == "$dir_pw_thread_stl:thread" &&
+    pw_sync_BINARY_SEMAPHORE_BACKEND != "" && pw_sync_MUTEX_BACKEND != "" &&
+    dir_pw_third_party_nanopb != ""
+
 pw_test("client_server_context_threaded_test") {
   deps = [
     ":client_api",
-    ":client_server_testing",
+    ":client_server_testing_threaded",
     "$dir_pw_sync:binary_semaphore",
-    "$dir_pw_thread:thread",
+    "$dir_pw_thread:test_threads",
+    "$dir_pw_thread_stl:test_threads",
     "..:test_protos.nanopb_rpc",
   ]
   sources = [ "client_server_context_threaded_test.cc" ]