From 6bcf44dcb713dbe22b432c1444380276ddf807c8 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Fri, 4 Dec 2015 16:36:21 -0800 Subject: [PATCH] Improved unit-test line coverage and quality. In particular - Upgraded some unit test modules to `unittest2`. - Got to 100% line coverage in `test_jwt`, `test_clientsecrets`, and `test_service_account` by using `self.assertRaises` instead of `self.fail()` - In some places used `self.assertRaises` as a context manager so the thrown exception can be inspected / compared against. This is done instead of manually catching the exception and holding on to it (`unittest2` backports this feature from `unittest` that was not available in Python 2.6) - Added license to `test_multistore_file` - Removed legacy `python2.4` hashbang from `test_jwt` and `test_service_account` - Added `unittest2` as a test dependency in `tox.ini` - Updated `test_clientsecrets.test_load_by_filename_missing_file` to use the `self.assertRaises` context manager (this was a problem in #349 and #350) - Updated `test_jwt` module docstring to reflect actual purpose --- tests/contrib/test_multistore_file.py | 14 +++++++++ tests/test_clientsecrets.py | 43 +++++++++++++-------------- tests/test_jwt.py | 38 ++++++++++------------- tests/test_service_account.py | 20 ++++--------- tox.ini | 1 + 5 files changed, 57 insertions(+), 59 deletions(-) diff --git a/tests/contrib/test_multistore_file.py b/tests/contrib/test_multistore_file.py index 84a217f1d..8cbe0c6fa 100644 --- a/tests/contrib/test_multistore_file.py +++ b/tests/contrib/test_multistore_file.py @@ -1,3 +1,17 @@ +# Copyright 2015 Google Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + """Unit tests for oauth2client.multistore_file.""" import datetime diff --git a/tests/test_clientsecrets.py b/tests/test_clientsecrets.py index 49ce85b35..1ba69fb9c 100644 --- a/tests/test_clientsecrets.py +++ b/tests/test_clientsecrets.py @@ -18,7 +18,7 @@ from io import StringIO import os import tempfile -import unittest +import unittest2 from oauth2client._helpers import _from_bytes from oauth2client import GOOGLE_AUTH_URI @@ -37,7 +37,7 @@ os.path.dirname(__file__), 'afilethatisntthere.json') -class Test__validate_clientsecrets(unittest.TestCase): +class Test__validate_clientsecrets(unittest2.TestCase): def test_with_none(self): self.assertRaises(clientsecrets.InvalidClientSecretsError, @@ -157,7 +157,7 @@ def test_success_type_installed(self): self.assertEqual(result, (clientsecrets.TYPE_INSTALLED, client_info)) -class Test__loadfile(unittest.TestCase): +class Test__loadfile(unittest2.TestCase): def test_success(self): client_type, client_info = clientsecrets._loadfile(VALID_FILE) @@ -186,7 +186,7 @@ def test_bad_json(self): clientsecrets._loadfile, filename) -class OAuth2CredentialsTests(unittest.TestCase): +class OAuth2CredentialsTests(unittest2.TestCase): def test_validate_error(self): payload = ( @@ -210,32 +210,33 @@ def test_validate_error(self): # Ensure that it is unicode src = _from_bytes(src) # Test load(s) - try: + with self.assertRaises( + clientsecrets.InvalidClientSecretsError) as exc_manager: clientsecrets.loads(src) - self.fail(src + ' should not be a valid client_secrets file.') - except clientsecrets.InvalidClientSecretsError as e: - self.assertTrue(str(e).startswith(match)) + + first_caught_exception = exc_manager.exception + self.assertTrue(str(first_caught_exception).startswith(match)) # Test loads(fp) - try: + with self.assertRaises( + clientsecrets.InvalidClientSecretsError) as exc_manager: fp = StringIO(src) clientsecrets.load(fp) - self.fail(src + ' should not be a valid client_secrets file.') - except clientsecrets.InvalidClientSecretsError as e: - self.assertTrue(str(e).startswith(match)) + + second_caught_exception = exc_manager.exception + self.assertTrue(str(second_caught_exception).startswith(match)) def test_load_by_filename_missing_file(self): - caught_exception = None - try: + with self.assertRaises( + clientsecrets.InvalidClientSecretsError) as exc_manager: clientsecrets._loadfile(NONEXISTENT_FILE) - except clientsecrets.InvalidClientSecretsError as exc: - caught_exception = exc + caught_exception = exc_manager.exception self.assertEquals(caught_exception.args[1], NONEXISTENT_FILE) self.assertEquals(caught_exception.args[3], errno.ENOENT) -class CachedClientsecretsTests(unittest.TestCase): +class CachedClientsecretsTests(unittest2.TestCase): class CacheMock(object): def __init__(self): @@ -282,12 +283,8 @@ def test_cache_hit(self): self.assertEqual(None, self.cache_mock.last_set_ns) def test_validation(self): - try: + with self.assertRaises(clientsecrets.InvalidClientSecretsError): clientsecrets.loadfile(INVALID_FILE, cache=self.cache_mock) - self.fail('Expected InvalidClientSecretsError to be raised ' - 'while loading %s' % INVALID_FILE) - except clientsecrets.InvalidClientSecretsError: - pass def test_without_cache(self): # this also ensures loadfile() is backward compatible @@ -297,4 +294,4 @@ def test_without_cache(self): if __name__ == '__main__': # pragma: NO COVER - unittest.main() + unittest2.main() diff --git a/tests/test_jwt.py b/tests/test_jwt.py index 72ebba4c9..3f69dc3e9 100644 --- a/tests/test_jwt.py +++ b/tests/test_jwt.py @@ -1,6 +1,4 @@ -#!/usr/bin/python2.4 -# -# Copyright 2014 Google Inc. All rights reserved. +# Copyright 2015 Google Inc. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -14,15 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Oauth2client tests - -Unit tests for oauth2client. -""" +"""Unit tests for JWT related methods in oauth2client.""" import os import tempfile import time -import unittest +import unittest2 from .http_mock import HttpMockSequence from oauth2client.client import Credentials @@ -45,7 +40,7 @@ def datafile(filename): return data -class CryptTests(unittest.TestCase): +class CryptTests(unittest2.TestCase): def setUp(self): self.format = 'p12' @@ -83,11 +78,12 @@ def _check_jwt_failure(self, jwt, expected_error): certs = {'foo': public_key} audience = ('https://www.googleapis.com/auth/id?client_id=' 'external_public_key@testing.gserviceaccount.com') - try: + + with self.assertRaises(crypt.AppIdentityError) as exc_manager: crypt.verify_signed_jwt_with_certs(jwt, certs, audience) - self.fail() - except crypt.AppIdentityError as e: - self.assertTrue(expected_error in str(e)) + + caught_exception = exc_manager.exception + self.assertTrue(expected_error in str(caught_exception)) def _create_signed_jwt(self): private_key = datafile('privatekey.%s' % self.format) @@ -216,7 +212,7 @@ def setUp(self): self.verifier = crypt.OpenSSLVerifier -class SignedJwtAssertionCredentialsTests(unittest.TestCase): +class SignedJwtAssertionCredentialsTests(unittest2.TestCase): def setUp(self): self.format = 'p12' @@ -310,7 +306,7 @@ def setUp(self): crypt.Signer = crypt.PyCryptoSigner -class PKCSSignedJwtAssertionCredentialsPyCryptoTests(unittest.TestCase): +class PKCSSignedJwtAssertionCredentialsPyCryptoTests(unittest2.TestCase): def test_for_failure(self): crypt.Signer = crypt.PyCryptoSigner @@ -320,14 +316,12 @@ def test_for_failure(self): private_key, scope='read+write', sub='joe@example.org') - try: - credentials._generate_assertion() - self.fail() - except NotImplementedError: - pass + + self.assertRaises(NotImplementedError, + credentials._generate_assertion) -class TestHasOpenSSLFlag(unittest.TestCase): +class TestHasOpenSSLFlag(unittest2.TestCase): def test_true(self): self.assertEqual(True, HAS_OPENSSL) @@ -335,4 +329,4 @@ def test_true(self): if __name__ == '__main__': # pragma: NO COVER - unittest.main() + unittest2.main() diff --git a/tests/test_service_account.py b/tests/test_service_account.py index 5cf1c1c15..eb6e76333 100644 --- a/tests/test_service_account.py +++ b/tests/test_service_account.py @@ -1,6 +1,4 @@ -#!/usr/bin/python2.4 -# -# Copyright 2014 Google Inc. All rights reserved. +# Copyright 2015 Google Inc. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -61,17 +59,11 @@ def test_sign_blob(self): self.assertTrue(rsa.pkcs1.verify(b'Google', signature, pub_key)) - try: - rsa.pkcs1.verify(b'Orest', signature, pub_key) - self.fail('Verification should have failed!') - except rsa.pkcs1.VerificationError: - pass # Expected - - try: - rsa.pkcs1.verify(b'Google', b'bad signature', pub_key) - self.fail('Verification should have failed!') - except rsa.pkcs1.VerificationError: - pass # Expected + self.assertRaises(rsa.pkcs1.VerificationError, + rsa.pkcs1.verify, b'Orest', signature, pub_key) + self.assertRaises(rsa.pkcs1.VerificationError, + rsa.pkcs1.verify, + b'Google', b'bad signature', pub_key) def test_service_account_email(self): self.assertEqual(self.service_account_email, diff --git a/tox.ini b/tox.ini index db459cb95..efdf9a791 100644 --- a/tox.ini +++ b/tox.ini @@ -10,6 +10,7 @@ basedeps = keyring webtest nose flask + unittest2 deps = {[testenv]basedeps} django setenv =