Flip the default Android snippet client version to V2 (#839)

3 files changed, 51 insertions, 120 deletions

diff --git a/mobly/controllers/android_device_lib/services/snippet_management_service.py b/mobly/controllers/android_device_lib/services/snippet_management_service.py
index 9dc9d75..72e5f16 100644
--- a/mobly/controllers/android_device_lib/services/snippet_management_service.py
+++ b/mobly/controllers/android_device_lib/services/snippet_management_service.py
@@ -62,12 +62,12 @@ class SnippetManagementService(base_service.BaseService):
     if self._use_client_v2_switch is None:
       device_dimensions = getattr(self._device, 'dimensions', {})
       switch_from_dimension = (device_dimensions.get(_CLIENT_V2_CONFIG_KEY,
-                                                     'false').lower() == 'true')
+                                                     'true').lower() == 'true')
 
       switch_from_attribute = (getattr(self._device, _CLIENT_V2_CONFIG_KEY,
-                                       'false').lower() == 'true')
+                                       'true').lower() == 'true')
 
-      self._use_client_v2_switch = (switch_from_dimension or
+      self._use_client_v2_switch = (switch_from_dimension and
                                     switch_from_attribute)
     return self._use_client_v2_switch
 
diff --git a/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py b/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py
index 27d389a..f92e60e 100755
--- a/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py
+++ b/tests/mobly/controllers/android_device_lib/services/snippet_management_service_test.py
@@ -18,7 +18,6 @@ from unittest import mock
 from mobly.controllers.android_device_lib.services import snippet_management_service
 
 MOCK_PACKAGE = 'com.mock.package'
-SNIPPET_CLIENT_CLASS_PATH = 'mobly.controllers.android_device_lib.snippet_client.SnippetClient'
 SNIPPET_CLIENT_V2_CLASS_PATH = 'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2'
 
 
@@ -28,136 +27,149 @@ class SnippetManagementServiceTest(unittest.TestCase):
   def test_empty_manager_start_stop(self):
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.start()
     # When no client is registered, manager is never alive.
     self.assertFalse(manager.is_alive)
     manager.stop()
     self.assertFalse(manager.is_alive)
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_get_snippet_client(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     self.assertEqual(manager.get_snippet_client('foo'), mock_client)
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_get_snippet_client_fail(self, _):
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     self.assertIsNone(manager.get_snippet_client('foo'))
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_stop_with_live_client(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
-    mock_client.start_app_and_connect.assert_called_once_with()
+    mock_client.initialize.assert_called_once_with()
     manager.stop()
-    mock_client.stop_app.assert_called_once_with()
-    mock_client.stop_app.reset_mock()
+    mock_client.stop.assert_called_once_with()
+    mock_client.stop.reset_mock()
     mock_client.is_alive = False
     self.assertFalse(manager.is_alive)
     manager.stop()
-    mock_client.stop_app.assert_not_called()
+    mock_client.stop.assert_not_called()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_add_snippet_client_dup_name(self, _):
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     msg = ('.* Name "foo" is already registered with package ".*", it '
            'cannot be used again.')
     with self.assertRaisesRegex(snippet_management_service.Error, msg):
       manager.add_snippet_client('foo', MOCK_PACKAGE + 'ha')
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_add_snippet_client_dup_package(self, mock_class):
     mock_client = mock_class.return_value
     mock_client.package = MOCK_PACKAGE
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     msg = ('Snippet package "com.mock.package" has already been loaded '
            'under name "foo".')
     with self.assertRaisesRegex(snippet_management_service.Error, msg):
       manager.add_snippet_client('bar', MOCK_PACKAGE)
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_remove_snippet_client(self, mock_class):
     mock_client = mock.MagicMock()
     mock_class.return_value = mock_client
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     manager.remove_snippet_client('foo')
     msg = 'No snippet client is registered with name "foo".'
     with self.assertRaisesRegex(snippet_management_service.Error, msg):
       manager.foo.do_something()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_remove_snippet_client(self, mock_class):
     mock_client = mock.MagicMock()
     mock_class.return_value = mock_client
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     with self.assertRaisesRegex(
         snippet_management_service.Error,
         'No snippet client is registered with name "foo".'):
       manager.remove_snippet_client('foo')
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_start_with_live_service(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
-    mock_client.start_app_and_connect.reset_mock()
+    mock_client.initialize.reset_mock()
     mock_client.is_alive = True
     manager.start()
-    mock_client.start_app_and_connect.assert_not_called()
+    mock_client.initialize.assert_not_called()
     self.assertTrue(manager.is_alive)
     mock_client.is_alive = False
     manager.start()
-    mock_client.start_app_and_connect.assert_called_once_with()
+    mock_client.initialize.assert_called_once_with()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_pause(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     manager.pause()
-    mock_client.disconnect.assert_called_once_with()
+    mock_client.close_connection.assert_called_once_with()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_resume_positive_case(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     mock_client.is_alive = False
     manager.resume()
-    mock_client.restore_app_connection.assert_called_once_with()
+    mock_client.restore_server_connection.assert_called_once_with()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_resume_negative_case(self, mock_class):
     mock_client = mock_class.return_value
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     mock_client.is_alive = True
     manager.resume()
-    mock_client.restore_app_connection.assert_not_called()
+    mock_client.restore_server_connection.assert_not_called()
 
-  @mock.patch(SNIPPET_CLIENT_CLASS_PATH)
+  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
   def test_attribute_access(self, mock_class):
     mock_client = mock.MagicMock()
     mock_class.return_value = mock_client
     manager = snippet_management_service.SnippetManagementService(
         mock.MagicMock())
+    manager._use_client_v2_switch = True
     manager.add_snippet_client('foo', MOCK_PACKAGE)
     manager.foo.ha('param')
     mock_client.ha.assert_called_once_with('param')
@@ -166,7 +178,7 @@ class SnippetManagementServiceTest(unittest.TestCase):
   # tests after we complete the migration from v1 to v2.
   def test_client_v2_switch_default_value(self):
     self._set_device_attribute_and_check_client_v2_switch(
-        expect_switch_value=False)
+        expect_switch_value=True)
 
   def test_client_v2_switch_when_set_device_dimension_to_false(self):
     self._set_device_attribute_and_check_client_v2_switch(
@@ -198,91 +210,10 @@ class SnippetManagementServiceTest(unittest.TestCase):
         use_mobly_snippet_client_v2='true',
         dimensions={'use_mobly_snippet_client_v2': 'true'})
 
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_add_snippet_client(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-    self.assertIs(manager.get_snippet_client('foo'), mock_client)
-    mock_client.initialize.assert_called_once_with()
-
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_remove_snippet_client(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-    manager.remove_snippet_client('foo')
-    mock_client.stop.assert_called_once_with()
-
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_start(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-
-    mock_client.initialize.reset_mock()
-    mock_client.is_alive = False
-    manager.start()
-
-    mock_client.initialize.assert_called_once_with()
-
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_stop(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-
-    mock_client.stop.reset_mock()
-    mock_client.is_alive = True
-    manager.stop()
-
-    mock_client.stop.assert_called_once_with()
-
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_pause(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-
-    mock_client.close_connection.reset_mock()
-    manager.pause()
-
-    mock_client.close_connection.assert_called_once_with()
-
-  @mock.patch(SNIPPET_CLIENT_V2_CLASS_PATH)
-  def test_client_v2_resume(self, mock_class):
-    mock_client = mock.MagicMock()
-    mock_class.return_value = mock_client
-    mock_device = mock.MagicMock(
-        dimensions={'use_mobly_snippet_client_v2': 'true'})
-    manager = snippet_management_service.SnippetManagementService(mock_device)
-    manager.add_snippet_client('foo', MOCK_PACKAGE)
-
-    mock_client.restore_server_connection.reset_mock()
-    mock_client.is_alive = False
-    manager.resume()
-
-    mock_client.restore_server_connection.assert_called_once_with()
-
   def _set_device_attribute_and_check_client_v2_switch(self,
                                                        expect_switch_value,
                                                        **device_attributes):
-    mock_device = mock.MagicMock(**device_attributes)
+    mock_device = mock.MagicMock(**device_attributes, spec=['log'])
     manager = snippet_management_service.SnippetManagementService(mock_device)
     self.assertEqual(expect_switch_value, manager._is_using_client_v2())
 
diff --git a/tests/mobly/controllers/android_device_test.py b/tests/mobly/controllers/android_device_test.py
index 6adb8f5..a99f1e3 100755
--- a/tests/mobly/controllers/android_device_test.py
+++ b/tests/mobly/controllers/android_device_test.py
@@ -1004,7 +1004,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_load_snippet(self, MockGetPort, MockSnippetClient,
                                       MockFastboot, MockAdbProxy):
@@ -1017,7 +1017,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_getattr(self, MockGetPort, MockSnippetClient,
                                  MockFastboot, MockAdbProxy):
@@ -1032,7 +1032,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient',
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2',
       return_value=MockSnippetClient)
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_load_snippet_dup_package(self, MockGetPort,
@@ -1050,7 +1050,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient',
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2',
       return_value=MockSnippetClient)
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_load_snippet_dup_snippet_name(self, MockGetPort,
@@ -1069,7 +1069,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_load_snippet_dup_attribute_name(
       self, MockGetPort, MockSnippetClient, MockFastboot, MockAdbProxy):
@@ -1084,7 +1084,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_load_snippet_start_app_fails(self, MockGetPort,
                                                       MockSnippetClient,
@@ -1092,13 +1092,13 @@ class AndroidDeviceTest(unittest.TestCase):
                                                       MockAdbProxy):
     """Verifies that the correct exception is raised if start app failed.
 
-    It's possible that the `stop_app` call as part of the start app failure
+    It's possible that the `stop` call as part of the start app failure
     teardown also fails. So we want the exception from the start app
     failure.
     """
     expected_e = Exception('start failed.')
-    MockSnippetClient.start_app_and_connect = mock.Mock(side_effect=expected_e)
-    MockSnippetClient.stop_app = mock.Mock(
+    MockSnippetClient.initialize = mock.Mock(side_effect=expected_e)
+    MockSnippetClient.stop = mock.Mock(
         side_effect=Exception('stop failed.'))
     ad = android_device.AndroidDevice(serial='1')
     try:
@@ -1111,7 +1111,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   def test_AndroidDevice_unload_snippet(self, MockGetPort, MockSnippetClient,
                                         MockFastboot, MockAdbProxy):
@@ -1132,7 +1132,7 @@ class AndroidDeviceTest(unittest.TestCase):
   @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
               return_value=mock_android_device.MockFastbootProxy('1'))
   @mock.patch(
-      'mobly.controllers.android_device_lib.snippet_client.SnippetClient')
+      'mobly.controllers.android_device_lib.snippet_client_v2.SnippetClientV2')
   @mock.patch('mobly.utils.get_available_host_port')
   @mock.patch.object(logcat.Logcat, '_open_logcat_file')
   def test_AndroidDevice_snippet_cleanup(self, open_logcat_mock, MockGetPort,