Make testing style more consistent (#168)

Several overall style changes:

1.  Avoid plain `Mock()` and `Mock(spec=thing)`., prefer `mock.create_autospec()`.
1.  Don't `mock.patch` without `autospec`.
1.  Don't give mock instances special names. Prefer `thing` over `thing_mock` and `mock_thing`.
1.  When using `mock.patch`, use the same name as the item being patched to refer to the mock.
    ```python
    with mock.patch('module.thing') as thing:
        ...
    ```
    and
    ```
    @mock.patch('module.thing')
    def test(thing):
        ...
    ```
1.  Test helper factories should follow the naming convention `make_thing()`.
1.  Use `ThingStub` when creating semi-functioning subclasses for testing purposes.

tests/compute_engine/test__metadata.py

@@ -24,191 +24,189 @@
 from google.auth import _helpers
 from google.auth import environment_vars
 from google.auth import exceptions
+from google.auth import transport
 from google.auth.compute_engine import _metadata
 
 PATH = 'instance/service-accounts/default'
 
 
-@pytest.fixture
-def mock_request():
- request_mock = mock.Mock()
+def make_request(data, status=http_client.OK, headers=None):
+ response = mock.create_autospec(transport.Response, instance=True)
+ response.status = status
+ response.data = _helpers.to_bytes(data)
+ response.headers = headers or {}
 
- def set_response(data, status=http_client.OK, headers=None):
- response = mock.Mock()
- response.status = status
- response.data = _helpers.to_bytes(data)
- response.headers = headers or {}
- request_mock.return_value = response
- return request_mock
+ request = mock.create_autospec(transport.Request)
+ request.return_value = response
 
- yield set_response
+ return request
 
 
-def test_ping_success(mock_request):
- request_mock = mock_request('', headers=_metadata._METADATA_HEADERS)
+def test_ping_success():
+ request = make_request('', headers=_metadata._METADATA_HEADERS)
 
- assert _metadata.ping(request_mock)
+ assert _metadata.ping(request)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_IP_ROOT,
  headers=_metadata._METADATA_HEADERS,
  timeout=_metadata._METADATA_DEFAULT_TIMEOUT)
 
 
-def test_ping_failure_bad_flavor(mock_request):
- request_mock = mock_request(
+def test_ping_failure_bad_flavor():
+ request = make_request(
  '', headers={_metadata._METADATA_FLAVOR_HEADER: 'meep'})
 
- assert not _metadata.ping(request_mock)
+ assert not _metadata.ping(request)
 
 
-def test_ping_failure_connection_failed(mock_request):
- request_mock = mock_request('')
- request_mock.side_effect = exceptions.TransportError()
+def test_ping_failure_connection_failed():
+ request = make_request('')
+ request.side_effect = exceptions.TransportError()
 
- assert not _metadata.ping(request_mock)
+ assert not _metadata.ping(request)
 
 
-def test_ping_success_custom_root(mock_request):
- request_mock = mock_request('', headers=_metadata._METADATA_HEADERS)
+def test_ping_success_custom_root():
+ request = make_request('', headers=_metadata._METADATA_HEADERS)
 
  fake_ip = '1.2.3.4'
  os.environ[environment_vars.GCE_METADATA_IP] = fake_ip
  reload_module(_metadata)
 
  try:
- assert _metadata.ping(request_mock)
+ assert _metadata.ping(request)
  finally:
  del os.environ[environment_vars.GCE_METADATA_IP]
  reload_module(_metadata)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url='http:https://' + fake_ip,
  headers=_metadata._METADATA_HEADERS,
  timeout=_metadata._METADATA_DEFAULT_TIMEOUT)
 
 
-def test_get_success_json(mock_request):
+def test_get_success_json():
  key, value = 'foo', 'bar'
 
  data = json.dumps({key: value})
- request_mock = mock_request(
+ request = make_request(
  data, headers={'content-type': 'application/json'})
 
- result = _metadata.get(request_mock, PATH)
+ result = _metadata.get(request, PATH)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH,
  headers=_metadata._METADATA_HEADERS)
  assert result[key] == value
 
 
-def test_get_success_text(mock_request):
+def test_get_success_text():
  data = 'foobar'
- request_mock = mock_request(data, headers={'content-type': 'text/plain'})
+ request = make_request(data, headers={'content-type': 'text/plain'})
 
- result = _metadata.get(request_mock, PATH)
+ result = _metadata.get(request, PATH)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH,
  headers=_metadata._METADATA_HEADERS)
  assert result == data
 
 
-def test_get_success_custom_root(mock_request):
- request_mock = mock_request(
+def test_get_success_custom_root():
+ request = make_request(
  '{}', headers={'content-type': 'application/json'})
 
  fake_root = 'another.metadata.service'
  os.environ[environment_vars.GCE_METADATA_ROOT] = fake_root
  reload_module(_metadata)
 
  try:
- _metadata.get(request_mock, PATH)
+ _metadata.get(request, PATH)
  finally:
  del os.environ[environment_vars.GCE_METADATA_ROOT]
  reload_module(_metadata)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url='http:https://{}/computeMetadata/v1/{}'.format(fake_root, PATH),
  headers=_metadata._METADATA_HEADERS)
 
 
-def test_get_failure(mock_request):
- request_mock = mock_request(
+def test_get_failure():
+ request = make_request(
  'Metadata error', status=http_client.NOT_FOUND)
 
  with pytest.raises(exceptions.TransportError) as excinfo:
- _metadata.get(request_mock, PATH)
+ _metadata.get(request, PATH)
 
  assert excinfo.match(r'Metadata error')
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH,
  headers=_metadata._METADATA_HEADERS)
 
 
-def test_get_failure_bad_json(mock_request):
- request_mock = mock_request(
+def test_get_failure_bad_json():
+ request = make_request(
  '{', headers={'content-type': 'application/json'})
 
  with pytest.raises(exceptions.TransportError) as excinfo:
- _metadata.get(request_mock, PATH)
+ _metadata.get(request, PATH)
 
  assert excinfo.match(r'invalid JSON')
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH,
  headers=_metadata._METADATA_HEADERS)
 
 
-def test_get_project_id(mock_request):
+def test_get_project_id():
  project = 'example-project'
- request_mock = mock_request(
+ request = make_request(
  project, headers={'content-type': 'text/plain'})
 
- project_id = _metadata.get_project_id(request_mock)
+ project_id = _metadata.get_project_id(request)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + 'project/project-id',
  headers=_metadata._METADATA_HEADERS)
  assert project_id == project
 
 
 @mock.patch('google.auth._helpers.utcnow', return_value=datetime.datetime.min)
-def test_get_service_account_token(utcnow, mock_request):
+def test_get_service_account_token(utcnow):
  ttl = 500
- request_mock = mock_request(
+ request = make_request(
  json.dumps({'access_token': 'token', 'expires_in': ttl}),
  headers={'content-type': 'application/json'})
 
- token, expiry = _metadata.get_service_account_token(request_mock)
+ token, expiry = _metadata.get_service_account_token(request)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH + '/token',
  headers=_metadata._METADATA_HEADERS)
  assert token == 'token'
  assert expiry == utcnow() + datetime.timedelta(seconds=ttl)
 
 
-def test_get_service_account_info(mock_request):
+def test_get_service_account_info():
  key, value = 'foo', 'bar'
- request_mock = mock_request(
+ request = make_request(
  json.dumps({key: value}),
  headers={'content-type': 'application/json'})
 
- info = _metadata.get_service_account_info(request_mock)
+ info = _metadata.get_service_account_info(request)
 
- request_mock.assert_called_once_with(
+ request.assert_called_once_with(
  method='GET',
  url=_metadata._METADATA_ROOT + PATH + '/?recursive=true',
  headers=_metadata._METADATA_HEADERS)

tests/compute_engine/test_credentials.py

@@ -19,6 +19,7 @@
 
 from google.auth import _helpers
 from google.auth import exceptions
+from google.auth import transport
 from google.auth.compute_engine import credentials
 
 
@@ -41,9 +42,9 @@ def test_default_state(self):
  @mock.patch(
  'google.auth._helpers.utcnow',
  return_value=datetime.datetime.min + _helpers.CLOCK_SKEW)
- @mock.patch('google.auth.compute_engine._metadata.get')
- def test_refresh_success(self, get_mock, now_mock):
- get_mock.side_effect = [{
+ @mock.patch('google.auth.compute_engine._metadata.get', autospec=True)
+ def test_refresh_success(self, get, utcnow):
+ get.side_effect = [{
  # First request is for sevice account info.
  'email': '[email protected]',
  'scopes': ['one', 'two']
@@ -59,7 +60,7 @@ def test_refresh_success(self, get_mock, now_mock):
  # Check that the credentials have the token and proper expiration
  assert self.credentials.token == 'token'
  assert self.credentials.expiry == (
- now_mock() + datetime.timedelta(seconds=500))
+ utcnow() + datetime.timedelta(seconds=500))
 
  # Check the credential info
  assert (self.credentials.service_account_email ==
@@ -71,17 +72,17 @@ def test_refresh_success(self, get_mock, now_mock):
  assert self.credentials.valid
 
  @mock.patch('google.auth.compute_engine._metadata.get', autospec=True)
- def test_refresh_error(self, get_mock):
- get_mock.side_effect = exceptions.TransportError('http error')
+ def test_refresh_error(self, get):
+ get.side_effect = exceptions.TransportError('http error')
 
  with pytest.raises(exceptions.RefreshError) as excinfo:
  self.credentials.refresh(None)
 
  assert excinfo.match(r'http error')
 
  @mock.patch('google.auth.compute_engine._metadata.get', autospec=True)
- def test_before_request_refreshes(self, get_mock):
- get_mock.side_effect = [{
+ def test_before_request_refreshes(self, get):
+ get.side_effect = [{
  # First request is for sevice account info.
  'email': '[email protected]',
  'scopes': 'one two'
@@ -95,11 +96,12 @@ def test_before_request_refreshes(self, get_mock):
  assert not self.credentials.valid
 
  # before_request should cause a refresh
+ request = mock.create_autospec(transport.Request, instance=True)
  self.credentials.before_request(
- mock.Mock(), 'GET', 'http:https://example.com?a=1#3', {})
+ request, 'GET', 'http:https://example.com?a=1#3', {})
 
  # The refresh endpoint should've been called.
- assert get_mock.called
+ assert get.called
 
  # Credentials should now be valid.
  assert self.credentials.valid