CB-388: Reduce scope of mocks

critiquebrainz/db/test/review_test.py

@@ -1,6 +1,5 @@
-from unittest.mock import MagicMock
-
-from brainzutils import cache
+from unittest import mock
+from unittest.mock import Mock
 
 import critiquebrainz.db.exceptions as db_exceptions
 import critiquebrainz.db.license as db_license
@@ -30,9 +29,6 @@ def setUp(self):
             full_name=u"Test License",
         )
 
-        # disable caching
-        cache.get = MagicMock(return_value=None)
-
     def test_review_creation(self):
         review = db_review.create(
             user_id=self.user.id,
@@ -211,7 +207,9 @@ def test_list_reviews(self):
         self.assertEqual(count, 1)
         self.assertEqual(len(reviews), 1)
 
-    def test_get_popular(self):
+    @mock.patch('brainzutils.cache.get')
+    def test_get_popular(self, cache_get):
+        cache_get.return_value = None
         reviews = db_review.get_popular()
         self.assertEqual(len(reviews), 0)
 

critiquebrainz/frontend/external/test/spotify_test.py

@@ -1,8 +1,5 @@
 from http import HTTPStatus
-from unittest.mock import MagicMock
-
-import requests
-from brainzutils import cache
+from unittest import mock
 
 from critiquebrainz.frontend.external import spotify
 from critiquebrainz.frontend.testing import FrontendTestCase
@@ -63,76 +60,93 @@ def setUp(self):
         }
         self.fresh_token_response = MockResponse(content=self.fresh_token)
 
-    def test_get(self):
+    @mock.patch('critiquebrainz.frontend.external.spotify._fetch_access_token')
+    @mock.patch('requests.get')
+    def test_valid_get(self, requests_get, _fetch_access_token):
         # test when _fetch_access_token returns acceptable token
-        requests.get = MagicMock(return_value=self.ok_response)
-        spotify._fetch_access_token = MagicMock(return_value="valid-token")
+        requests_get.return_value = self.ok_response
+        _fetch_access_token.return_value = "valid-token"
         self.assertDictEqual(spotify._get("test-query"), self.some_albums_response)
-        spotify._fetch_access_token.assert_called_with(refresh=False)
+        _fetch_access_token.assert_called_with(refresh=False)
 
         # test when _fetch_access_token returns expired token
-        requests.get = MagicMock(return_value=self.unauth_response)
-        spotify._fetch_access_token = MagicMock(return_value="expired-token")
+        _fetch_access_token.reset_mock()
+        requests_get.return_value = self.unauth_response
+        _fetch_access_token.return_value = "expired-token"
         with self.assertRaises(spotify.SpotifyUnexpectedResponseException):
             spotify._get("test-query")
-        spotify._fetch_access_token.assert_called_with(refresh=True)
+        _fetch_access_token.assert_called_with(refresh=True)
 
-    def test_fetch_access_token(self):
-        cache.set = MagicMock()
-        requests.post = MagicMock(return_value=self.fresh_token_response)
+    @mock.patch('brainzutils.cache.set')
+    @mock.patch('brainzutils.cache.get')
+    @mock.patch('requests.post')
+    def test_fetch_access_token(self, requests_post, cache_get, cache_set):
+        requests_post.return_value = self.fresh_token_response
 
         # token present in cache
-        cache.get = MagicMock(return_value=self.cached_token["access_token"])
+        cache_get.return_value = self.cached_token["access_token"]
         # use cached token
         self.assertEqual(spotify._fetch_access_token(refresh=False), self.cached_token["access_token"])
         # force fetch fresh token (used for case when token is expired)
         self.assertEqual(spotify._fetch_access_token(refresh=True), self.fresh_token["access_token"])
 
         # token not present in cache
-        cache.get = MagicMock(return_value=None)
+        cache_get.reset_mock()
+        cache_get.return_value = None
         # fresh token should be fetched, whatever be the value of 'refresh'
         self.assertEqual(spotify._fetch_access_token(refresh=False), self.fresh_token["access_token"])
         self.assertEqual(spotify._fetch_access_token(refresh=True), self.fresh_token["access_token"])
 
         # should raise SpotifyException if could not fetch fresh token
-        requests.post = MagicMock(return_value=MockResponse(content={}))
+        requests_post.reset_mock()
+        requests_post.return_value = MockResponse(content={})
         with self.assertRaises(spotify.SpotifyException):
             spotify._fetch_access_token(refresh=True)
 
-    def test_search(self):
+    @mock.patch('critiquebrainz.frontend.external.spotify._get')
+    @mock.patch('brainzutils.cache.get')
+    def test_search(self, cache_get, spotify_get):
         # results are in cache
-        cache.get = MagicMock(return_value=self.all_albums)
+        cache_get.return_value = self.all_albums
         self.assertDictEqual(spotify.search("test-query"), self.all_albums)
 
         # results are not in cache
-        cache.get = MagicMock(return_value=None)
-        spotify._get = MagicMock(return_value=self.all_albums)
+        cache_get.reset_mock()
+        cache_get.return_value = None
+        spotify_get.return_value = self.all_albums
         self.assertDictEqual(spotify.search(query="Test Artist"), self.all_albums)
 
-    def test_get_album(self):
+    @mock.patch('critiquebrainz.frontend.external.spotify._get')
+    @mock.patch('brainzutils.cache.get')
+    def test_get_album(self, cache_get, spotify_get):
         # results are in cache
-        cache.get = MagicMock(return_value=self.all_albums)
+        cache_get.return_value = self.all_albums
         self.assertDictEqual(spotify.get_album("test-spotify-id"), self.all_albums)
 
         # results are not in cache
-        cache.get = MagicMock(return_value=None)
-        spotify._get = MagicMock(return_value=self.all_albums)
+        cache_get.reset_mock()
+        cache_get.return_value = None
+        spotify_get.return_value = self.all_albums
         self.assertDictEqual(spotify.get_album("test-spotify-id"), self.all_albums)
 
-    def test_get_multiple_albums(self):
+    @mock.patch('critiquebrainz.frontend.external.spotify._get')
+    @mock.patch('brainzutils.cache.get_many')
+    def test_get_multiple_albums(self, cache_get_many, spotify_get):
         # if no spotify ids are sent
         self.assertDictEqual(spotify.get_multiple_albums([]), {})
 
         # all ids are in cache
-        cache.get_many = MagicMock(return_value=self.all_albums)
+        cache_get_many.return_value = self.all_albums
         self.assertDictEqual(spotify.get_multiple_albums(self.all_spotify_ids), self.all_albums)
 
         # some ids are not in cache
-        cache.get_many = MagicMock(return_value=self.some_albums)
+        cache_get_many.reset_mock()
+        cache_get_many.return_value = self.some_albums
         # _get sends an unexpected response
-        spotify._get = MagicMock(return_value={})
+        spotify_get.return_value = {}
         with self.assertRaises(spotify.SpotifyUnexpectedResponseException):
             spotify.get_multiple_albums(self.all_spotify_ids)
         # _get sends expected response
-        spotify._get = MagicMock(return_value=self.some_albums_response)
+        spotify_get.reset_mock()
+        spotify_get.return_value = self.some_albums_response
         self.assertDictEqual(spotify.get_multiple_albums(self.all_spotify_ids), self.all_albums)

critiquebrainz/frontend/views/test/test_artist.py

@@ -1,7 +1,5 @@
-from unittest.mock import MagicMock
+from unittest import mock
 
-import critiquebrainz.frontend.external.musicbrainz_db.artist as mb_artist
-import critiquebrainz.frontend.external.musicbrainz_db.release_group as mb_release_group
 from critiquebrainz.frontend.testing import FrontendTestCase
 
 
@@ -32,16 +30,17 @@ class ArtistViewsTestCase(FrontendTestCase):
 
     def setUp(self):
         super(ArtistViewsTestCase, self).setUp()
-        mb_artist.get_artist_by_id = MagicMock()
-        mb_release_group.browse_release_groups = MagicMock(side_effect=return_release_groups)
 
-    def test_artist_page(self):
-        # Basic artist page should be available.
-        mb_artist.get_artist_by_id.return_value = {
+    @mock.patch('critiquebrainz.frontend.external.musicbrainz_db.release_group.browse_release_groups')
+    @mock.patch('critiquebrainz.frontend.external.musicbrainz_db.artist.get_artist_by_id')
+    def test_artist_page(self, get_artist_by_id, browse_release_groups):
+        get_artist_by_id.return_value = {
             'id': 'f59c5520-5f46-4d2c-b2c4-822eabf53419',
             'name': 'Linkin Park',
-            'sort-name': 'Linkin Park',
+            'sort-name': 'Linkin Park'
         }
+        browse_release_groups.side_effect = return_release_groups
+        # Basic artist page should be available.
         response = self.client.get('/artist/f59c5520-5f46-4d2c-b2c4-822eabf53419')
         self.assert200(response)
         self.assertIn('Linkin Park', str(response.data))