Skip to content

Commit

Permalink
Merge pull request #3063 from dhermes/make-mocks-less-broad
Browse files Browse the repository at this point in the history
Drop usage of overly broad mock.Mock() in tests.
  • Loading branch information
dhermes authored Mar 1, 2017
2 parents c090fb5 + c8e8a49 commit 17ac87f
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 35 deletions.
32 changes: 21 additions & 11 deletions error_reporting/unit_tests/test__gax.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,34 @@ class Test_ErrorReportingGaxApi(unittest.TestCase):

PROJECT = 'PROJECT'

def _call_fut(self, gax_api, project):
def _make_one(self, gax_api, project):
from google.cloud.error_reporting._gax import _ErrorReportingGaxApi

return _ErrorReportingGaxApi(gax_api, project)

def test_constructor(self):
gax_api = mock.Mock()
gax_client_wrapper = self._call_fut(gax_api, self.PROJECT)
gax_api = mock.Mock(spec=[])
gax_client_wrapper = self._make_one(gax_api, self.PROJECT)

self.assertEqual(gax_client_wrapper._project, self.PROJECT)
self.assertEqual(gax_client_wrapper._gax_api, gax_api)

@mock.patch("google.cloud.error_reporting._gax.ParseDict")
def test_report_error_event(self, _):
gax_api = mock.Mock()
gax_client_wrapper = self._call_fut(gax_api, self.PROJECT)
def test_report_error_event(self):
from google.cloud.proto.devtools.clouderrorreporting.v1beta1 import (
report_errors_service_pb2)

mock_error_report = mock.Mock()
gax_client_wrapper.report_error_event(mock_error_report)
self.assertTrue(gax_api.report_error_event.called_with,
mock_error_report)
gax_api = mock.Mock(spec=['project_path', 'report_error_event'])
gax_client_wrapper = self._make_one(gax_api, self.PROJECT)

error_report = {
'message': 'The cabs are here.',
}
gax_client_wrapper.report_error_event(error_report)

gax_api.project_path.assert_called_once_with(self.PROJECT)
project_name = gax_api.project_path.return_value
error_pb = report_errors_service_pb2.ReportedErrorEvent(
message=error_report['message'],
)
gax_api.report_error_event.assert_called_once_with(
project_name, error_pb)
34 changes: 21 additions & 13 deletions error_reporting/unit_tests/test__logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,36 @@ def _make_credentials():
class Test_ErrorReportingLoggingAPI(unittest.TestCase):

PROJECT = 'PROJECT'
SERVICE = 'SERVICE'
VERSION = 'myversion'

def _call_fut(self, project, credentials):
def _make_one(self, project, credentials):
from google.cloud.error_reporting._logging import (
_ErrorReportingLoggingAPI)

return _ErrorReportingLoggingAPI(project, credentials)

def test_constructor(self):
credentials = _make_credentials()
logger_client = self._call_fut(self.PROJECT, credentials)
logging_api = self._make_one(self.PROJECT, credentials)

self.assertEqual(logger_client.logging_client._connection.credentials,
self.assertEqual(logging_api.logging_client._connection.credentials,
credentials)
self.assertEqual(logger_client.logging_client.project, self.PROJECT)
self.assertEqual(logging_api.logging_client.project, self.PROJECT)

@mock.patch('google.cloud.logging.client')
def test_report_error_event(self, _):
@mock.patch('google.cloud.logging.client.Client')
def test_report_error_event(self, mocked_cls):
credentials = _make_credentials()
logger_client = self._call_fut(self.PROJECT, credentials)
payload = mock.Mock()
logger_client.report_error_event(payload)
logger_mock = mock.Mock()
self.assertTrue(logger_mock.log_struct.called_with, payload)
logging_api = self._make_one(self.PROJECT, credentials)
mocked_cls.assert_called_once_with(self.PROJECT, credentials, None)
self.assertIs(logging_api.logging_client, mocked_cls.return_value)

logger = mock.Mock(spec=['log_struct'])
logging_api.logging_client.logger.return_value = logger

# Actually make the API call.
error_report = {
'message': 'The cabs are here.',
}
logging_api.report_error_event(error_report)

# Check the mocks.
logger.log_struct.assert_called_once_with(error_report)
13 changes: 10 additions & 3 deletions error_reporting/unit_tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ def _make_http(self, *args, **kw):

return HTTPContext(*args, **kw)

def _get_report_payload(self, error_api):
self.assertEqual(error_api.report_error_event.call_count, 1)
call = error_api.report_error_event.mock_calls[0]
_, positional, kwargs = call
self.assertEqual(kwargs, {})
self.assertEqual(len(positional), 1)
return positional[0]

@mock.patch(
'google.cloud.error_reporting.client._determine_default_project')
def test_ctor_default(self, default_mock):
Expand Down Expand Up @@ -131,7 +139,7 @@ def test_report_exception_with_service_version_in_constructor(

make_api.assert_called_once_with(client)

payload = error_api.report_error_event.call_args[0][0]
payload = self._get_report_payload(error_api)
self.assertEqual(payload['serviceContext'], {
'service': service,
'version': version
Expand All @@ -158,8 +166,7 @@ def test_report(self, make_api):
message = 'this is an error'
client.report(message)

payload = error_api.report_error_event.call_args[0][0]
make_api.assert_called_once_with(client)
payload = self._get_report_payload(error_api)

self.assertEqual(payload['message'], message)
report_location = payload['context']['reportLocation']
Expand Down
2 changes: 1 addition & 1 deletion logging/unit_tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ def test_setup_logging(self):

http_mock = mock.Mock(spec=httplib2.Http)
deepcopy = mock.Mock(return_value=http_mock)
setup_logging = mock.Mock()
setup_logging = mock.Mock(spec=[])

credentials = _make_credentials()

Expand Down
5 changes: 4 additions & 1 deletion vision/unit_tests/test__gax.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ def _make_one(self, *args, **kwargs):
return self._get_target_class()(*args, **kwargs)

def test_ctor(self):
client = mock.Mock()
client = mock.Mock(
_credentials=_make_credentials(),
spec=['_credentials'],
)
with mock.patch('google.cloud.vision._gax.image_annotator_client.'
'ImageAnnotatorClient'):
api = self._make_one(client)
Expand Down
11 changes: 8 additions & 3 deletions vision/unit_tests/test_batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_ctor(self):
from google.cloud.vision.feature import FeatureTypes
from google.cloud.vision.image import Image

client = mock.Mock()
client = mock.Mock(spec=[])
image = Image(client, source_uri='gs://images/imageone.jpg')
face_feature = Feature(FeatureTypes.FACE_DETECTION, 5)
logo_feature = Feature(FeatureTypes.LOGO_DETECTION, 3)
Expand All @@ -66,8 +66,13 @@ def test_batch_from_client(self):
image_two = client.image(source_uri='gs://images/imagtwo.jpg')
face_feature = Feature(FeatureTypes.FACE_DETECTION, 5)
logo_feature = Feature(FeatureTypes.LOGO_DETECTION, 3)
client._vision_api_internal = mock.Mock()
client._vision_api_internal.annotate.return_value = True

# Make mocks.
annotate = mock.Mock(return_value=True, spec=[])
vision_api = mock.Mock(annotate=annotate, spec=['annotate'])
client._vision_api_internal = vision_api

# Actually call the partially-mocked method.
batch = client.batch()
batch.add_image(image_one, [face_feature])
batch.add_image(image_two, [logo_feature, face_feature])
Expand Down
6 changes: 3 additions & 3 deletions vision/unit_tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ def test_annotate_with_preset_api(self):
vision_api = client._vision_api
vision_api._connection = _Connection()

api = mock.Mock()
api.annotate.return_value = mock.sentinel.annotated
annotate = mock.Mock(return_value=mock.sentinel.annotated, spec=[])
api = mock.Mock(annotate=annotate, spec=['annotate'])

client._vision_api_internal = api
client._vision_api.annotate()
api.annotate.assert_called_once_with()
annotate.assert_called_once_with()

def test_make_gax_client(self):
from google.cloud.vision._gax import _GAPICVisionAPI
Expand Down

0 comments on commit 17ac87f

Please sign in to comment.