Rename Snippet client v2's startup sequence steps (#806)
Rename the startup sequence steps (hopefully) for the better.
diff --git a/mobly/snippet/client_base.py b/mobly/snippet/client_base.py
index 8fdd6c9..31e99dc 100644
--- a/mobly/snippet/client_base.py
+++ b/mobly/snippet/client_base.py
@@ -38,13 +38,11 @@
or returned void.>,
'callback': <Required. String that represents a callback ID used to
identify events associated with a particular CallbackHandler
- object, `null` if this is not a async RPC.>,
+ object, `null` if this is not an asynchronous RPC.>,
}
"""
import abc
-import contextlib
-import enum
import json
import threading
import time
@@ -59,14 +57,6 @@
RPC_RESPONSE_REQUIRED_FIELDS = ('id', 'error', 'result', 'callback')
-class StartServerStages(enum.Enum):
- """The stages for the starting server process."""
- BEFORE_STARTING_SERVER = 1
- DO_START_SERVER = 2
- BUILD_CONNECTION = 3
- AFTER_STARTING_SERVER = 4
-
-
class ClientBase(abc.ABC):
"""Base class for JSON RPC clients that connect to snippet servers.
@@ -107,14 +97,13 @@
def __del__(self):
self.close_connection()
- def start_server(self):
- """Starts the server on the remote device and connects to it.
+ def initialize(self):
+ """Initializes the snippet client to interact with the remote device.
- This process contains four stages:
- - before_starting_server: prepares for starting the server.
- - do_start_server: starts the server on the remote device.
- - build_connection: builds a connection with the server.
- - after_starting_server: does the things after the server is available.
+ This function contains following stages:
+ 1. preparing to start the snippet server.
+ 2. starting the snippet server on the remote device.
+ 3. making a connection to the snippet server.
After this, the self.host_port and self.device_port attributes must be
set.
@@ -127,55 +116,42 @@
errors.ServerStartError: when failed to start the snippet server.
"""
- @contextlib.contextmanager
- def _execute_one_stage(stage):
- """Context manager for executing one stage.
-
- Args:
- stage: StartServerStages, the stage which is running under this
- context manager.
-
- Yields:
- None.
- """
- self.log.debug('[START_SERVER] Running the stage %s.', stage.name)
- yield
- self.log.debug('[START_SERVER] Finished the stage %s.', stage.name)
-
- self.log.debug('Starting the server.')
+ self.log.debug('Initializing the snippet package %s.', self.package)
start_time = time.perf_counter()
- with _execute_one_stage(StartServerStages.BEFORE_STARTING_SERVER):
- self.before_starting_server()
+ self.log.debug('Preparing to start the snippet server of %s.', self.package)
+ self.before_starting_server()
try:
- with _execute_one_stage(StartServerStages.DO_START_SERVER):
- self.do_start_server()
+ self.log.debug('Starting the snippet server of %s.', self.package)
+ self.start_server()
- with _execute_one_stage(StartServerStages.BUILD_CONNECTION):
- self._build_connection()
-
- with _execute_one_stage(StartServerStages.AFTER_STARTING_SERVER):
- self.after_starting_server()
+ self.log.debug('Making a connection to the snippet server of %s.',
+ self.package)
+ self._make_connection()
except Exception:
- self.log.error('[START SERVER] Error occurs when starting the server.')
+ self.log.error(
+ 'Error occurred trying to start and connect to the snippet server '
+ 'of %s.', self.package)
try:
self.stop_server()
except Exception: # pylint: disable=broad-except
# Only prints this exception and re-raises the original exception
- self.log.exception('[START_SERVER] Failed to stop server because of '
- 'new exception.')
+ self.log.exception(
+ 'Failed to stop the snippet server of %s after failure to start '
+ 'and connect.', self.package)
raise
- self.log.debug('Snippet %s started after %.1fs on host port %d.',
- self.package,
- time.perf_counter() - start_time, self.host_port)
+ self.log.debug(
+ 'Snippet package %s initialized after %.1fs on host port %d.',
+ self.package,
+ time.perf_counter() - start_time, self.host_port)
@abc.abstractmethod
def before_starting_server(self):
- """Prepares for starting the server.
+ """Performs the preparation steps before starting the remote server.
For example, subclass can check or modify the device settings at this
stage.
@@ -186,45 +162,43 @@
"""
@abc.abstractmethod
- def do_start_server(self):
+ def start_server(self):
"""Starts the server on the remote device.
The client has completed the preparations, so the client calls this
function to start the server.
"""
- def _build_connection(self):
- """Proxy function of build_connection.
+ def _make_connection(self):
+ """Proxy function of make_connection.
- This function resets the RPC id counter before calling `build_connection`.
+ This function resets the RPC id counter before calling `make_connection`.
"""
self._counter = self._id_counter()
- self.build_connection()
+ self.make_connection()
@abc.abstractmethod
- def build_connection(self):
- """Builds a connection with the server on the remote device.
+ def make_connection(self):
+ """Makes a connection to the snippet server on the remote device.
- The command to start the server has been already sent before calling this
- function. So the client builds a connection to it and sends a handshake
- to ensure the server is available for upcoming RPCs.
+ This function makes a connection to the server and sends a handshake
+ request to ensure the server is available for upcoming RPCs.
+
+ There are two types of connections used by snippet clients:
+ * The client makes a new connection each time it needs to send an RPC.
+ * The client makes a connection in this stage and uses it for all the RPCs.
+ In this case, the client should implement `close_connection` to close
+ the connection.
This function uses self.host_port for communicating with the server. If
self.host_port is 0 or None, this function finds an available host port to
- build connection and set self.host_port to the found port.
+ make the connection and set self.host_port to the found port.
Raises:
errors.ProtocolError: something went wrong when exchanging data with the
server.
"""
- @abc.abstractmethod
- def after_starting_server(self):
- """Does the things after the server is available.
-
- For example, subclass can get device information from the server.
- """
-
def __getattr__(self, name):
"""Wrapper for python magic to turn method calls into RPCs."""
@@ -273,11 +247,11 @@
Raises:
errors.ServerRestoreConnectionError: when failed to restore the connection
- with the snippet server.
+ to the snippet server.
"""
def _rpc(self, rpc_func_name, *args, **kwargs):
- """Sends a RPC to the server.
+ """Sends an RPC to the server.
Args:
rpc_func_name: str, the name of the snippet function to execute on the
@@ -325,7 +299,7 @@
"""Checks whether the server is still running.
If the server is not running, it throws an error. As this function is called
- each time the client tries to send a RPC, this should be a quick check
+ each time the client tries to send an RPC, this should be a quick check
without affecting performance. Otherwise it is fine to not check anything.
Raises:
diff --git a/tests/mobly/snippet/client_base_test.py b/tests/mobly/snippet/client_base_test.py
index 4da74d6..3eb6521 100755
--- a/tests/mobly/snippet/client_base_test.py
+++ b/tests/mobly/snippet/client_base_test.py
@@ -26,7 +26,7 @@
def _generate_fix_length_rpc_response(
response_length,
template='{"id": 0, "result": "%s", "error": null, "callback": null}'):
- """Generates a RPC response string with specified length.
+ """Generates an RPC response string with specified length.
This function generates a random string and formats the template with the
generated random string to get the response string. This function formats
@@ -66,13 +66,10 @@
def before_starting_server(self):
pass
- def do_start_server(self):
+ def start_server(self):
pass
- def build_connection(self):
- pass
-
- def after_starting_server(self):
+ def make_connection(self):
pass
def restore_server_connection(self, port=None):
@@ -103,70 +100,56 @@
self.client.host_port = 12345
@mock.patch.object(FakeClient, 'before_starting_server')
- @mock.patch.object(FakeClient, 'do_start_server')
- @mock.patch.object(FakeClient, '_build_connection')
- @mock.patch.object(FakeClient, 'after_starting_server')
- def test_start_server_stage_order(self, mock_after_func, mock_build_conn_func,
- mock_do_start_func, mock_before_func):
- """Test that starting server runs its stages in expected order."""
+ @mock.patch.object(FakeClient, 'start_server')
+ @mock.patch.object(FakeClient, '_make_connection')
+ def test_init_server_stage_order(self, mock_make_conn_func, mock_start_func,
+ mock_before_func):
+ """Test that initialization runs its stages in expected order."""
order_manager = mock.Mock()
order_manager.attach_mock(mock_before_func, 'mock_before_func')
- order_manager.attach_mock(mock_do_start_func, 'mock_do_start_func')
- order_manager.attach_mock(mock_build_conn_func, 'mock_build_conn_func')
- order_manager.attach_mock(mock_after_func, 'mock_after_func')
+ order_manager.attach_mock(mock_start_func, 'mock_start_func')
+ order_manager.attach_mock(mock_make_conn_func, 'mock_make_conn_func')
- self.client.start_server()
+ self.client.initialize()
expected_call_order = [
mock.call.mock_before_func(),
- mock.call.mock_do_start_func(),
- mock.call.mock_build_conn_func(),
- mock.call.mock_after_func(),
+ mock.call.mock_start_func(),
+ mock.call.mock_make_conn_func(),
]
self.assertListEqual(order_manager.mock_calls, expected_call_order)
@mock.patch.object(FakeClient, 'stop_server')
@mock.patch.object(FakeClient, 'before_starting_server')
- def test_start_server_before_starting_server_fail(self, mock_before_func,
- mock_stop_server):
- """Test starting server's stage before_starting_server fails."""
+ def test_init_server_before_starting_server_fail(self, mock_before_func,
+ mock_stop_server):
+ """Test before_starting_server stage of initialization fails."""
mock_before_func.side_effect = Exception('ha')
with self.assertRaisesRegex(Exception, 'ha'):
- self.client.start_server()
+ self.client.initialize()
mock_stop_server.assert_not_called()
@mock.patch.object(FakeClient, 'stop_server')
- @mock.patch.object(FakeClient, 'do_start_server')
- def test_start_server_do_start_server_fail(self, mock_do_start_func,
- mock_stop_server):
- """Test starting server's stage do_start_server fails."""
- mock_do_start_func.side_effect = Exception('ha')
+ @mock.patch.object(FakeClient, 'start_server')
+ def test_init_server_start_server_fail(self, mock_start_func,
+ mock_stop_server):
+ """Test start_server stage of initialization fails."""
+ mock_start_func.side_effect = Exception('ha')
with self.assertRaisesRegex(Exception, 'ha'):
- self.client.start_server()
+ self.client.initialize()
mock_stop_server.assert_called()
@mock.patch.object(FakeClient, 'stop_server')
- @mock.patch.object(FakeClient, '_build_connection')
- def test_start_server_build_connection_fail(self, mock_build_conn_func,
- mock_stop_server):
- """Test starting server's stage _build_connection fails."""
- mock_build_conn_func.side_effect = Exception('ha')
+ @mock.patch.object(FakeClient, '_make_connection')
+ def test_init_server_make_connection_fail(self, mock_make_conn_func,
+ mock_stop_server):
+ """Test _make_connection stage of initialization fails."""
+ mock_make_conn_func.side_effect = Exception('ha')
with self.assertRaisesRegex(Exception, 'ha'):
- self.client.start_server()
- mock_stop_server.assert_called()
-
- @mock.patch.object(FakeClient, 'stop_server')
- @mock.patch.object(FakeClient, 'after_starting_server')
- def test_start_server_after_starting_server_fail(self, mock_after_func,
- mock_stop_server):
- """Test starting server's stage after_starting_server fails."""
- mock_after_func.side_effect = Exception('ha')
-
- with self.assertRaisesRegex(Exception, 'ha'):
- self.client.start_server()
+ self.client.initialize()
mock_stop_server.assert_called()
@mock.patch.object(FakeClient, 'check_server_proc_running')
@@ -177,9 +160,9 @@
def test_rpc_stage_dependencies(self, mock_handle_resp, mock_decode_resp_str,
mock_send_request, mock_gen_request,
mock_precheck):
- """Test the internal dependencies when sending a RPC.
+ """Test the internal dependencies when sending an RPC.
- When sending a RPC, it calls multiple functions in specific order, and
+ When sending an RPC, it calls multiple functions in specific order, and
each function uses the output of the previously called function. This test
case checks above dependencies.
@@ -191,7 +174,7 @@
mock_gen_request: the mock function of FakeClient._gen_rpc_request.
mock_precheck: the mock function of FakeClient.check_server_proc_running.
"""
- self.client.start_server()
+ self.client.initialize()
expected_response_str = ('{"id": 0, "result": 123, "error": null, '
'"callback": null}')
@@ -226,8 +209,8 @@
def test_rpc_precheck_fail(self, mock_handle_resp, mock_decode_resp_str,
mock_send_request, mock_gen_request,
mock_precheck):
- """Test when RPC precheck fails it will skip sending RPC."""
- self.client.start_server()
+ """Test when RPC precheck fails it will skip sending the RPC."""
+ self.client.initialize()
mock_precheck.side_effect = Exception('server_died')
with self.assertRaisesRegex(Exception, 'server_died'):
@@ -239,7 +222,7 @@
mock_decode_resp_str.assert_not_called()
def test_gen_request(self):
- """Test generating a RPC request.
+ """Test generating an RPC request.
Test that _gen_rpc_request returns a string represents a JSON dict
with all required fields.
@@ -270,7 +253,7 @@
self.client._decode_response_string_and_validate_format(0, None)
def test_rpc_response_missing_fields(self):
- """Test parsing a RPC response that misses some required fields."""
+ """Test parsing an RPC response that misses some required fields."""
mock_resp_without_id = '{"result": 123, "error": null, "callback": null}'
with self.assertRaisesRegex(
errors.ProtocolError,
@@ -300,7 +283,7 @@
10, mock_resp_without_callback)
def test_rpc_response_error(self):
- """Test parsing a RPC response with a non-empty error field."""
+ """Test parsing an RPC response with a non-empty error field."""
mock_resp_with_error = {
'id': 10,
'result': 123,
@@ -343,7 +326,7 @@
mock_handle_callback.assert_not_called()
def test_rpc_response_id_mismatch(self):
- """Test parsing a RPC response with wrong id."""
+ """Test parsing an RPC response with a wrong id."""
right_id = 5
wrong_id = 20
resp = f'{{"id": {right_id}, "result": 1, "error": null, "callback": null}}'
@@ -358,7 +341,7 @@
mock_log = mock.Mock()
self.client.log = mock_log
self.client.set_snippet_client_verbose_logging(True)
- self.client.start_server()
+ self.client.initialize()
resp = _generate_fix_length_rpc_response(
client_base._MAX_RPC_RESP_LOGGING_LENGTH * 2)
@@ -372,7 +355,7 @@
mock_log = mock.Mock()
self.client.log = mock_log
self.client.set_snippet_client_verbose_logging(False)
- self.client.start_server()
+ self.client.initialize()
resp = _generate_fix_length_rpc_response(
int(client_base._MAX_RPC_RESP_LOGGING_LENGTH // 2))
@@ -386,7 +369,7 @@
mock_log = mock.Mock()
self.client.log = mock_log
self.client.set_snippet_client_verbose_logging(False)
- self.client.start_server()
+ self.client.initialize()
resp = _generate_fix_length_rpc_response(
client_base._MAX_RPC_RESP_LOGGING_LENGTH)
@@ -400,7 +383,7 @@
mock_log = mock.Mock()
self.client.log = mock_log
self.client.set_snippet_client_verbose_logging(False)
- self.client.start_server()
+ self.client.initialize()
max_len = client_base._MAX_RPC_RESP_LOGGING_LENGTH
resp = _generate_fix_length_rpc_response(max_len * 40)
@@ -414,7 +397,7 @@
@mock.patch.object(FakeClient, 'send_rpc_request')
def test_rpc_call_increment_counter(self, mock_send_request):
"""Test that with each RPC call the counter is incremented by 1."""
- self.client.start_server()
+ self.client.initialize()
resp = '{"id": %d, "result": 123, "error": null, "callback": null}'
mock_send_request.side_effect = (resp % (i,) for i in range(10))
@@ -424,9 +407,9 @@
self.assertEqual(next(self.client._counter), 10)
@mock.patch.object(FakeClient, 'send_rpc_request')
- def test_build_connection_reset_counter(self, mock_send_request):
- """Test that _build_connection resets the counter to zero."""
- self.client.start_server()
+ def test_init_connection_reset_counter(self, mock_send_request):
+ """Test that _make_connection resets the counter to zero."""
+ self.client.initialize()
resp = '{"id": %d, "result": 123, "error": null, "callback": null}'
mock_send_request.side_effect = (resp % (i,) for i in range(10))
@@ -434,7 +417,7 @@
self.client.some_rpc()
self.assertEqual(next(self.client._counter), 10)
- self.client._build_connection()
+ self.client._make_connection()
self.assertEqual(next(self.client._counter), 0)
