Upgrading system tests for new dependencies.

In order to get passing system tests, Client.start()
and Client.stop() had to stop being used.

Also add a newline (per feedback) in setup.py.

Author: dhermes

setup.py

@@ -54,6 +54,7 @@
 ]
 
 SETUP_BASE.pop('url')
+
 setup(
     name='google-cloud-happybase',
     version='0.19.0',

src/google/cloud/happybase/connection.py

@@ -85,11 +85,7 @@ def _get_instance(timeout=None):
     if timeout is not None:
         client_kwargs['timeout_seconds'] = timeout / 1000.0
     client = Client(**client_kwargs)
-    try:
-        client.start()
-        instances, failed_locations = client.list_instances()
-    finally:
-        client.stop()
+    instances, failed_locations = client.list_instances()
 
     if len(failed_locations) != 0:
         raise ValueError('Determining instance via ListInstances encountered '
@@ -207,26 +203,14 @@ def _handle_legacy_args(arguments_dict):
     def open(self):
         """Open the underlying transport to Cloud Bigtable.
 
-        This method opens the underlying HTTP/2 gRPC connection using a
-        :class:`~google.cloud.bigtable.client.Client` bound to the
-        :class:`~google.cloud.bigtable.instance.Instance` owned by
-        this connection.
+        This method does nothing and is provided for compatibility.
         """
-        self._instance._client.start()
 
     def close(self):
         """Close the underlying transport to Cloud Bigtable.
 
-        This method closes the underlying HTTP/2 gRPC connection using a
-        :class:`~google.cloud.bigtable.client.Client` bound to the
-        :class:`~google.cloud.bigtable.instance.Instance` owned by
-        this connection.
+        This method does nothing and is provided for compatibility.
         """
-        self._instance._client.stop()
-
-    def __del__(self):
-        if self._instance is not None:
-            self.close()
 
     def _table_name(self, name):
         """Construct a table name by optionally adding a table name prefix.

src/google/cloud/happybase/pool.py

@@ -142,8 +142,7 @@ def connection(self, timeout=None):
             with self._lock:
                 self._thread_connections.current = connection
 
-        # This is a no-op for connections that have already been opened
-        # since they just call Client.start().
+        # This is a no-op.
         connection.open()
         yield connection
 

system_tests/happybase.py

@@ -18,18 +18,15 @@
 
 import unittest
 
-from google.cloud import _helpers
 from google.cloud.bigtable import client as client_mod
-from google.cloud.bigtable.happybase.connection import Connection
-from google.cloud.environment_vars import TESTS_PROJECT
+from google.cloud.happybase.connection import Connection
 
 from retry import RetryResult
 from system_test_utils import unique_resource_id
 
 
 _PACK_I64 = struct.Struct('>q').pack
 _FIRST_ELT = operator.itemgetter(0)
-_helpers.PROJECT = TESTS_PROJECT
 LOCATION_ID = 'us-central1-c'
 # NOTE: Avoid using the same name as in bigtable.py
 INSTANCE_ID = 'gcl-hb' + unique_resource_id('-')
@@ -63,33 +60,37 @@ class Config(object):
     TABLE = None
 
 
-def _operation_wait(operation, max_attempts=5):
+def _operation_complete(result):
+    """Identity function.
+
+    Meant to return a boolean, but the value passed in is
+    already equal to that value.
+    """
+    return result
+
+
+def _wait_until_complete(operation, max_attempts=5):
     """Wait until an operation has completed.
 
-    :type operation: :class:`~google.cloud.bigtable.instance.Operation`
-    :param operation: Operation that has not finished.
+    :type operation: :class:`google.cloud.operation.Operation`
+    :param operation: Operation that has not completed.
 
     :type max_attempts: int
     :param max_attempts: (Optional) The maximum number of times to check if
-                         the operation has finished. Defaults to 5.
+                         the operation has completed. Defaults to 5.
 
     :rtype: bool
-    :returns: Boolean indicating if the operation finished.
+    :returns: Boolean indicating if the operation is complete.
     """
-
-    def _operation_finished(result):
-        return result
-
-    retry = RetryResult(_operation_finished, max_tries=max_attempts)
-    return retry(operation.finished)()
+    retry = RetryResult(_operation_complete, max_tries=max_attempts)
+    return retry(operation.poll)()
 
 
 def set_connection():
     client = client_mod.Client(admin=True)
     instance = client.instance(INSTANCE_ID, LOCATION_ID)
-    client.start()
     operation = instance.create()
-    if not _operation_wait(operation):
+    if not _wait_until_complete(operation):
         raise RuntimeError('Instance creation exceed 5 seconds.')
     Config.CONNECTION = Connection(instance=instance)
 

system_tests/system_test_utils.py

@@ -18,11 +18,9 @@
 import time
 
 from google.cloud.environment_vars import CREDENTIALS as TEST_CREDENTIALS
-from google.cloud.environment_vars import TESTS_PROJECT
 
 
 # From shell environ. May be None.
-PROJECT_ID = os.getenv(TESTS_PROJECT)
 CREDENTIALS = os.getenv(TEST_CREDENTIALS)
 
 ENVIRON_ERROR_MSG = """\
@@ -44,24 +42,15 @@ def create_scoped_required():
 
 
 def check_environ():
-    missing = []
-    extra = ''
-
-    if PROJECT_ID is None:
-        missing.append(TESTS_PROJECT)
-
+    err_msg = None
     if CREDENTIALS is None:
-        missing.append(TEST_CREDENTIALS)
+        err_msg = '\nMissing variables: ' + TEST_CREDENTIALS
     elif not os.path.isfile(CREDENTIALS):
-        extra = '\nThe %s path %r is not a file.' % (TEST_CREDENTIALS,
-                                                     CREDENTIALS)
+        err_msg = '\nThe %s path %r is not a file.' % (TEST_CREDENTIALS,
+                                                       CREDENTIALS)
 
-    if missing or extra:
-        msg = ENVIRON_ERROR_MSG
-        if missing:
-            msg += '\nMissing variables: ' + ', '.join(missing)
-        if extra:
-            msg += extra
+    if err_msg is not None:
+        msg = ENVIRON_ERROR_MSG + err_msg
         print(msg, file=sys.stderr)
         sys.exit(1)
 

unit_tests/test_connection.py

@@ -44,8 +44,6 @@ def _helper(self, timeout=None, instances=(), failed_locations=()):
         if timeout is not None:
             expected_kwargs['timeout_seconds'] = timeout / 1000.0
         self.assertEqual(client.kwargs, expected_kwargs)
-        self.assertEqual(client.start_calls, 1)
-        self.assertEqual(client.stop_calls, 1)
 
     def test_default(self):
         instance = _Instance()
@@ -83,10 +81,7 @@ def _makeOne(self, *args, **kwargs):
 
     def test_constructor_defaults(self):
         instance = _Instance()  # Avoid implicit environ check.
-        self.assertEqual(instance._client.start_calls, 0)
         connection = self._makeOne(instance=instance)
-        self.assertEqual(instance._client.start_calls, 1)
-        self.assertEqual(instance._client.stop_calls, 0)
 
         self.assertEqual(connection._instance, instance)
         self.assertEqual(connection.table_prefix, None)
@@ -95,8 +90,6 @@ def test_constructor_defaults(self):
     def test_constructor_no_autoconnect(self):
         instance = _Instance()  # Avoid implicit environ check.
         connection = self._makeOne(autoconnect=False, instance=instance)
-        self.assertEqual(instance._client.start_calls, 0)
-        self.assertEqual(instance._client.stop_calls, 0)
         self.assertEqual(connection.table_prefix, None)
         self.assertEqual(connection.table_prefix_separator, '_')
 
@@ -183,37 +176,6 @@ def test_constructor_non_string_prefix_separator(self):
             self._makeOne(autoconnect=False,
                           table_prefix_separator=table_prefix_separator)
 
-    def test_open(self):
-        instance = _Instance()  # Avoid implicit environ check.
-        connection = self._makeOne(autoconnect=False, instance=instance)
-        self.assertEqual(instance._client.start_calls, 0)
-        connection.open()
-        self.assertEqual(instance._client.start_calls, 1)
-        self.assertEqual(instance._client.stop_calls, 0)
-
-    def test_close(self):
-        instance = _Instance()  # Avoid implicit environ check.
-        connection = self._makeOne(autoconnect=False, instance=instance)
-        self.assertEqual(instance._client.stop_calls, 0)
-        connection.close()
-        self.assertEqual(instance._client.stop_calls, 1)
-        self.assertEqual(instance._client.start_calls, 0)
-
-    def test___del__with_instance(self):
-        instance = _Instance()  # Avoid implicit environ check.
-        connection = self._makeOne(autoconnect=False, instance=instance)
-        self.assertEqual(instance._client.stop_calls, 0)
-        connection.__del__()
-        self.assertEqual(instance._client.stop_calls, 1)
-
-    def test___del__no_instance(self):
-        instance = _Instance()  # Avoid implicit environ check.
-        connection = self._makeOne(autoconnect=False, instance=instance)
-        self.assertEqual(instance._client.stop_calls, 0)
-        del connection._instance
-        connection.__del__()
-        self.assertEqual(instance._client.stop_calls, 0)
-
     def test__table_name_with_prefix_set(self):
         table_prefix = 'table-prefix'
         table_prefix_separator = '<>'
@@ -655,14 +617,6 @@ def __init__(self, *args, **kwargs):
         self.failed_locations = kwargs.pop('failed_locations', [])
         self.args = args
         self.kwargs = kwargs
-        self.start_calls = 0
-        self.stop_calls = 0
-
-    def start(self):
-        self.start_calls += 1
-
-    def stop(self):
-        self.stop_calls += 1
 
     def list_instances(self):
         return self.instances, self.failed_locations

unit_tests/test_pool.py

@@ -215,12 +215,7 @@ def test_connection_with_current_cnxn(self):
 
 
 class _Client(object):
-
-    def __init__(self):
-        self.stop_calls = 0
-
-    def stop(self):
-        self.stop_calls += 1
+    pass
 
 
 class _Connection(object):