diff options
author | Minghao Li <minghaoli@google.com> | 2022-04-09 11:13:36 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-08 20:13:36 -0700 |
commit | 8e0499c80d339eeb411ae28f5e328af722d7302d (patch) | |
tree | 3fb7ccac6fb70c91ffe9295968ec97cae431ecbb | |
parent | d1b010452445ec06207e03de23ae94cbe95a0bc7 (diff) | |
download | mobly-8e0499c80d339eeb411ae28f5e328af722d7302d.tar.gz |
Rename Snippet client v2's startup sequence steps (#806)
Rename the startup sequence steps (hopefully) for the better.
-rw-r--r-- | mobly/snippet/client_base.py | 116 | ||||
-rwxr-xr-x | tests/mobly/snippet/client_base_test.py | 111 |
2 files changed, 92 insertions, 135 deletions
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 @@ The JSON RPC protocol expected by this module is: 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 @@ _MAX_RPC_RESP_LOGGING_LENGTH = 1024 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 @@ class ClientBase(abc.ABC): 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 @@ class ClientBase(abc.ABC): 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() - - with _execute_one_stage(StartServerStages.BUILD_CONNECTION): - self._build_connection() + self.log.debug('Starting the snippet server of %s.', self.package) + self.start_server() - 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 @@ class ClientBase(abc.ABC): """ @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 @@ class ClientBase(abc.ABC): 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 @@ class ClientBase(abc.ABC): """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 @@ from mobly.snippet import errors 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 @@ class FakeClient(client_base.ClientBase): 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 @@ class ClientBaseTest(unittest.TestCase): 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') - - 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') + @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() + self.client.initialize() mock_stop_server.assert_called() @mock.patch.object(FakeClient, 'check_server_proc_running') @@ -177,9 +160,9 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): @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 @@ class ClientBaseTest(unittest.TestCase): 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 @@ class ClientBaseTest(unittest.TestCase): 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) |