pw_unit_test: Fix and test RPC service

- Update pw_unit_test's pw_rpc server streaming call to the new API.
- Fix issue where the test runner would crash if the first test was
  disabled.
- Add test that runs unit tests in a C++ binary from Python with the
  unit test RPC server.

Change-Id: I11657c5813696e44b50e71aa6c2e62cc80635e1e
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/55511
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/BUILD.gn b/BUILD.gn
index efc8e2e..df37b8a 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -128,15 +128,20 @@
 
 _default_toolchain = _default_toolchain_prefix + pw_default_optimization_level
 
-# Tests that larger than unit tests, that are typically run in a specific
-# configuration.
+# Tests larger than unit tests, typically run in a specific configuration.
 group("integration_tests") {
   deps = []
 
   if (host_os != "windows") {
     deps += [
+      # Uses port 33000
       "$dir_pw_rpc/py:python_client_cpp_server_test.action($_default_toolchain)",
+
+      # Uses port 33001
       "$dir_pw_transfer/py:python_cpp_transfer_test.action($_default_toolchain)",
+
+      # Uses port 33002
+      "$dir_pw_unit_test/py:rpc_service_test.action($_default_toolchain)",
     ]
   } else {
     not_needed([ "_default_toolchain" ])
diff --git a/pw_unit_test/BUILD.bazel b/pw_unit_test/BUILD.bazel
index 8c7bc8e..053ca65 100644
--- a/pw_unit_test/BUILD.bazel
+++ b/pw_unit_test/BUILD.bazel
@@ -157,3 +157,14 @@
         ":pw_unit_test",
     ],
 )
+
+cc_binary(
+    name = "test_rpc_server",
+    srcs = ["test_rpc_server.cc"],
+    deps = [
+        ":pw_unit_test",
+        ":rpc_service",
+        "//pw_log",
+        "//pw_rpc/system_server",
+    ],
+)
diff --git a/pw_unit_test/BUILD.gn b/pw_unit_test/BUILD.gn
index 2f36f7b..b46d33f 100644
--- a/pw_unit_test/BUILD.gn
+++ b/pw_unit_test/BUILD.gn
@@ -132,6 +132,17 @@
   sources = [ "rpc_main.cc" ]
 }
 
+pw_executable("test_rpc_server") {
+  sources = [ "test_rpc_server.cc" ]
+  deps = [
+    ":pw_unit_test",
+    ":rpc_service",
+    "$dir_pw_rpc/system_server",
+    "$dir_pw_rpc/system_server:socket",
+    dir_pw_log,
+  ]
+}
+
 pw_proto_library("unit_test_proto") {
   sources = [ "pw_unit_test_proto/unit_test.proto" ]
 }
diff --git a/pw_unit_test/py/BUILD.gn b/pw_unit_test/py/BUILD.gn
index 468af15..8f6d1e8 100644
--- a/pw_unit_test/py/BUILD.gn
+++ b/pw_unit_test/py/BUILD.gn
@@ -30,3 +30,14 @@
   ]
   pylintrc = "$dir_pigweed/.pylintrc"
 }
+
+pw_python_script("rpc_service_test") {
+  sources = [ "rpc_service_test.py" ]
+  pylintrc = "$dir_pigweed/.pylintrc"
+
+  action = {
+    args = [ "<TARGET_FILE(..:test_rpc_server)>" ]
+    deps = [ "..:test_rpc_server" ]
+    stamp = true
+  }
+}
diff --git a/pw_unit_test/py/pw_unit_test/__init__.py b/pw_unit_test/py/pw_unit_test/__init__.py
index e69de29..b1a7814 100644
--- a/pw_unit_test/py/pw_unit_test/__init__.py
+++ b/pw_unit_test/py/pw_unit_test/__init__.py
@@ -0,0 +1,16 @@
+# 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.
+"""Utilities for running unit tests over Pigweed RPC."""
+
+from pw_unit_test.rpc import run_tests, EventHandler, TestCase
diff --git a/pw_unit_test/py/pw_unit_test/rpc.py b/pw_unit_test/py/pw_unit_test/rpc.py
index 9fc8280..095186e 100644
--- a/pw_unit_test/py/pw_unit_test/rpc.py
+++ b/pw_unit_test/py/pw_unit_test/rpc.py
@@ -22,7 +22,7 @@
 from pw_rpc.callback_client import OptionalTimeout, UseDefault
 from pw_unit_test_proto import unit_test_pb2
 
-_LOG = logging.getLogger(__name__)
+_LOG = logging.getLogger(__package__)
 
 
 @dataclass(frozen=True)
@@ -38,6 +38,11 @@
         return f'TestCase({str(self)})'
 
 
+def _test_case(raw_test_case: unit_test_pb2.TestCaseDescriptor) -> TestCase:
+    return TestCase(raw_test_case.suite_name, raw_test_case.test_name,
+                    raw_test_case.file_name)
+
+
 @dataclass(frozen=True)
 class TestExpectation:
     expression: str
@@ -123,12 +128,11 @@
     True if all tests pass.
     """
     unit_test_service = rpcs.pw.unit_test.UnitTest  # type: ignore[attr-defined]
-
+    request = unit_test_service.Run.request(
+        report_passed_expectations=report_passed_expectations,
+        test_suite=test_suites)
     test_responses = iter(
-        unit_test_service.Run(
-            report_passed_expectations=report_passed_expectations,
-            test_suite=test_suites,
-            pw_rpc_timeout_s=timeout_s))
+        unit_test_service.Run.invoke(request, timeout_s=timeout_s))
 
     # Read the first response, which must be a test_run_start message.
     first_response = next(test_responses)
@@ -146,9 +150,7 @@
     for response in test_responses:
         if response.HasField('test_case_start'):
             raw_test_case = response.test_case_start
-            current_test_case = TestCase(raw_test_case.suite_name,
-                                         raw_test_case.test_name,
-                                         raw_test_case.file_name)
+            current_test_case = _test_case(raw_test_case)
 
         for event_handler in event_handlers:
             if response.HasField('test_run_start'):
@@ -164,7 +166,8 @@
                 event_handler.test_case_end(current_test_case,
                                             response.test_case_end)
             elif response.HasField('test_case_disabled'):
-                event_handler.test_case_disabled(current_test_case)
+                event_handler.test_case_disabled(
+                    _test_case(response.test_case_disabled))
             elif response.HasField('test_case_expectation'):
                 raw_expectation = response.test_case_expectation
                 expectation = TestExpectation(
diff --git a/pw_unit_test/py/rpc_service_test.py b/pw_unit_test/py/rpc_service_test.py
new file mode 100755
index 0000000..b10d54d
--- /dev/null
+++ b/pw_unit_test/py/rpc_service_test.py
@@ -0,0 +1,128 @@
+#!/usr/bin/env python3
+# 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.
+"""Tests using the callback client for pw_rpc."""
+
+import logging
+from pathlib import Path
+from typing import List, Tuple
+import unittest
+from unittest import mock
+
+from pw_hdlc import rpc
+from pw_rpc import testing
+from pw_unit_test_proto import unit_test_pb2
+from pw_unit_test import run_tests, EventHandler, TestCase
+from pw_status import Status
+
+PORT = 33002
+
+# The three suites (Passing, Failing, and DISABLED_Disabled) have these cases.
+_CASES = ('Zero', 'One', 'Two', 'DISABLED_Disabled')
+_FILE = 'pw_unit_test/test_rpc_server.cc'
+
+PASSING = tuple(TestCase('Passing', case, _FILE) for case in _CASES[:-1])
+FAILING = tuple(TestCase('Failing', case, _FILE) for case in _CASES[:-1])
+EXECUTED_TESTS = PASSING + FAILING
+
+DISABLED_SUITE = tuple(
+    TestCase('DISABLED_Disabled', case, _FILE) for case in _CASES)
+
+ALL_DISABLED_TESTS = (
+    TestCase('Passing', 'DISABLED_Disabled', _FILE),
+    TestCase('Failing', 'DISABLED_Disabled', _FILE),
+    *DISABLED_SUITE,
+)
+
+
+class RpcIntegrationTest(unittest.TestCase):
+    """Calls RPCs on an RPC server through a socket."""
+    test_server_command: Tuple[str, ...] = ()
+
+    def setUp(self) -> None:
+        server_and_client = rpc.HdlcRpcLocalServerAndClient(
+            self.test_server_command, PORT, [unit_test_pb2])
+        self._server = server_and_client.server
+        self.rpcs = server_and_client.client.channel(1).rpcs
+        self.handler = mock.NonCallableMagicMock(spec=EventHandler)
+
+    def tearDown(self) -> None:
+        self._server.close()
+
+    def test_run_tests_default_handler(self) -> None:
+        with self.assertLogs(logging.getLogger('pw_unit_test'),
+                             'INFO') as logs:
+            self.assertFalse(run_tests(self.rpcs))
+
+        for test in EXECUTED_TESTS:
+            self.assertTrue(any(str(test) in log for log in logs.output), test)
+
+    def test_run_tests_calls_test_case_start(self) -> None:
+        self.assertFalse(run_tests(self.rpcs, event_handlers=[self.handler]))
+
+        self.handler.test_case_start.assert_has_calls(
+            [mock.call(case) for case in EXECUTED_TESTS], any_order=True)
+
+    def test_run_tests_calls_test_case_end(self) -> None:
+        self.assertFalse(run_tests(self.rpcs, event_handlers=[self.handler]))
+
+        calls = [
+            mock.call(
+                case, unit_test_pb2.SUCCESS
+                if case.suite_name == 'Passing' else unit_test_pb2.FAILURE)
+            for case in EXECUTED_TESTS
+        ]
+        self.handler.test_case_end.assert_has_calls(calls, any_order=True)
+
+    def test_run_tests_calls_test_case_disabled(self) -> None:
+        self.assertFalse(run_tests(self.rpcs, event_handlers=[self.handler]))
+
+        self.handler.test_case_disabled.assert_has_calls(
+            [mock.call(case) for case in ALL_DISABLED_TESTS], any_order=True)
+
+    def test_passing_tests_only(self) -> None:
+        self.assertTrue(
+            run_tests(self.rpcs,
+                      test_suites=['Passing'],
+                      event_handlers=[self.handler]))
+        calls = [mock.call(case, unit_test_pb2.SUCCESS) for case in PASSING]
+        self.handler.test_case_end.assert_has_calls(calls, any_order=True)
+
+    def test_disabled_tests_only(self) -> None:
+        self.assertTrue(
+            run_tests(self.rpcs,
+                      test_suites=['DISABLED_Disabled'],
+                      event_handlers=[self.handler]))
+
+        self.handler.test_case_start.assert_not_called()
+        self.handler.test_case_end.assert_not_called()
+        self.handler.test_case_disabled.assert_has_calls(
+            [mock.call(case) for case in DISABLED_SUITE], any_order=True)
+
+    def test_failing_tests(self) -> None:
+        self.assertFalse(
+            run_tests(self.rpcs,
+                      test_suites=['Failing'],
+                      event_handlers=[self.handler]))
+        calls = [mock.call(case, unit_test_pb2.FAILURE) for case in FAILING]
+        self.handler.test_case_end.assert_has_calls(calls, any_order=True)
+
+
+def _main(test_server_command: List[str], unittest_args: List[str]) -> None:
+    RpcIntegrationTest.test_server_command = tuple(test_server_command)
+    unittest.main(argv=unittest_args)
+
+
+if __name__ == '__main__':
+    _main(**vars(testing.parse_test_server_args()))
diff --git a/pw_unit_test/test_rpc_server.cc b/pw_unit_test/test_rpc_server.cc
new file mode 100644
index 0000000..6dbad78
--- /dev/null
+++ b/pw_unit_test/test_rpc_server.cc
@@ -0,0 +1,78 @@
+// 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 "gtest/gtest.h"
+#include "pw_log/log.h"
+#include "pw_rpc_system_server/rpc_server.h"
+#include "pw_rpc_system_server/socket.h"
+#include "pw_unit_test/unit_test_service.h"
+
+namespace {
+
+pw::unit_test::UnitTestService unit_test_service;
+
+TEST(Passing, Zero) {}
+
+TEST(Passing, One) { EXPECT_TRUE(true); }
+
+TEST(Passing, Two) {
+  EXPECT_FALSE(0);
+  EXPECT_STREQ("Yes!", "Yes!\0extra stuff!");
+}
+
+TEST(Passing, DISABLED_Disabled) {
+  EXPECT_FALSE(0);
+  EXPECT_STREQ("Yes!", "Yes!\0extra stuff!");
+}
+
+TEST(Failing, Zero) { FAIL(); }
+
+TEST(Failing, One) { EXPECT_TRUE(false); }
+
+TEST(Failing, Two) {
+  EXPECT_FALSE(1);
+  EXPECT_STREQ("No!", "No?");
+}
+
+TEST(Failing, DISABLED_Disabled) {
+  EXPECT_FALSE(1);
+  EXPECT_STREQ("No!", "No?");
+}
+
+TEST(DISABLED_Disabled, Zero) { FAIL(); }
+
+TEST(DISABLED_Disabled, One) { EXPECT_TRUE(false); }
+
+TEST(DISABLED_Disabled, Two) {
+  EXPECT_FALSE(1);
+  EXPECT_STREQ("No!", "No?");
+}
+
+TEST(DISABLED_Disabled, DISABLED_Disabled) {
+  EXPECT_FALSE(1);
+  EXPECT_STREQ("No!", "No?");
+}
+
+}  // namespace
+
+int main() {
+  pw::rpc::system_server::set_socket_port(33002);
+  pw::rpc::system_server::Init();
+  pw::rpc::system_server::Server().RegisterService(unit_test_service);
+
+  PW_LOG_INFO("Starting pw_rpc server");
+  pw::rpc::system_server::Start();
+
+  return 0;
+}
diff --git a/pw_unit_test/unit_test_service.cc b/pw_unit_test/unit_test_service.cc
index e4eac53..43bc1fd 100644
--- a/pw_unit_test/unit_test_service.cc
+++ b/pw_unit_test/unit_test_service.cc
@@ -50,8 +50,8 @@
         if (!suites_to_run.full()) {
           suites_to_run.push_back(suite_name);
         } else {
-          PW_LOG_ERROR("Maximum of %d test suite filters supported",
-                       suites_to_run.max_size());
+          PW_LOG_ERROR("Maximum of %u test suite filters supported",
+                       static_cast<unsigned>(suites_to_run.max_size()));
           writer_.Finish(Status::InvalidArgument());
           return;
         }