Updated tests for currency API endpoint

The tests now use py.test.

conftest.py

+import logging
+
+import pytest
+from django.core.cache import cache
+from pytest_django.lazy_django import skip_if_no_django
+
+logger = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def django_cache(request, settings):
+    skip_if_no_django()
+
+    xdist_prefix = getattr(request.config, 'slaveinput', {}).get('slaveid')
+
+    if xdist_prefix:
+        for name, cache_settings in settings.CACHES.items():
+            # Put a prefix like _gw0, _gw1 etc on xdist processes
+            cache_settings['KEY_PREFIX'] = xdist_prefix + '_' + cache_settings.get('KEY_PREFIX', '')
+            logger.info('Set cache key prefix for [%s] cache to [%s]', name, cache_settings['KEY_PREFIX'])
+
+    yield cache
+
+    cache.clear()

course_discovery/apps/api/v1/tests/test_views/test_currency.py

 import mock
-from django.core.cache import cache
-from django.test import override_settings
+import pytest
 from django.urls import reverse
 
-from course_discovery.apps.api.v1.tests.test_views.mixins import APITestCase
 from course_discovery.apps.api.v1.views.currency import CurrencyView
-from course_discovery.apps.core.tests.factories import USER_PASSWORD, UserFactory
 
 
-class CurrencyViewTests(APITestCase):
+# NOTE: All of the tests that make authenticated requests result in the response being cached. We
+# force the use of a fixture here to ensure the cache is cleared after each test.
+@pytest.mark.usefixtures('django_cache')
+@pytest.mark.django_db
+class TestCurrencyCurrencyView:
     list_path = reverse('api:v1:currency')
 
-    def setUp(self):
-        super(CurrencyViewTests, self).setUp()
-        self.user = UserFactory(is_staff=True, is_superuser=True)
-        self.request.user = self.user
-        self.client.login(username=self.user.username, password=USER_PASSWORD)
+    def test_authentication_required(self, client):
+        response = client.get(self.list_path)
+        assert response.status_code == 403
 
-        # Clear the cache between test cases, so they don't interfere with each other.
-        cache.clear()
-
-    def test_get(self):
-        """ Verify the endpoint returns the right currency data and uses the cache. """
-        rates = {"GBP": 0.766609, "CAD": 1.222252, "CNY": 6.514431, "EUR": 0.838891}
-        currencies = {
-            'GBR': {'code': 'GBP', 'symbol': u'£'},
-            'CHN': {'code': 'CNY', 'symbol': u'¥'},
-            'CAN': {'code': 'CAD', 'symbol': '$'}
+    def test_get(self, admin_client, django_cache, responses, settings):
+        settings.OPENEXCHANGERATES_API_KEY = 'test'
+        rates = {
+            'GBP': 0.766609,
+            'CAD': 1.222252,
+            'CNY': 6.514431,
+            'EUR': 0.838891
         }
-        eurozone_countries = ['FRA']
-        get_data_return_value = [rates, currencies, eurozone_countries]
-
         expected = {
-            "GBR": {"code": "GBP", "symbol": u"£", "rate": 0.766609},
-            "CAN": {"code": "CAD", "symbol": "$", "rate": 1.222252},
-            "CHN": {"code": "CNY", "symbol": u"¥", "rate": 6.514431},
-            "FRA": {"code": "EUR", "symbol": "€", "rate": 0.838891}
+            'GBR': {'code': 'GBP', 'symbol': '£', 'rate': 0.766609},
+            'CAN': {'code': 'CAD', 'symbol': '$', 'rate': 1.222252},
+            'CHN': {'code': 'CNY', 'symbol': '¥', 'rate': 6.514431},
+            'FRA': {'code': 'EUR', 'symbol': '€', 'rate': 0.838891}
         }
 
-        with mock.patch.object(CurrencyView, 'get_data', return_value=get_data_return_value) as mock_get_rates:
-            response = self.client.get(self.list_path)
-            self.assertDictEqual(response.data, expected)
-            self.assertEqual(mock_get_rates.call_count, 1)
+        responses.add(responses.GET, CurrencyView.EXTERNAL_API_URL, json={'rates': rates})
+
+        response = admin_client.get(self.list_path)
+        assert all(item in response.data.items() for item in expected.items())
+        assert len(responses.calls) == 1
+
+        # Subsequent requests should hit the cache
+        response = admin_client.get(self.list_path)
+        assert all(item in response.data.items() for item in expected.items())
+        assert len(responses.calls) == 1
 
-            # next request hits the cache
-            response = self.client.get(self.list_path)
-            self.assertEqual(mock_get_rates.call_count, 1)
+        # Clearing the cache should result in the external service being called again
+        django_cache.clear()
+        response = admin_client.get(self.list_path)
+        assert all(item in response.data.items() for item in expected.items())
+        assert len(responses.calls) == 2
 
-            # clearing the cache calls means the function gets called again
-            cache.clear()
-            response = self.client.get(self.list_path)
-            self.assertEqual(mock_get_rates.call_count, 2)
+    def test_get_without_api_key(self, admin_client, settings):
+        settings.OPENEXCHANGERATES_API_KEY = None
 
-    def test_no_api_key(self):
-        response = self.client.get(self.list_path)
-        self.assertEqual(response.json(), {})
+        with mock.patch('course_discovery.apps.api.v1.views.currency.logger.warning') as mock_logger:
+            response = admin_client.get(self.list_path)
+            mock_logger.assert_called_with('Unable to retrieve exchange rate data. No API key is set.')
+            assert response.status_code == 200
+            assert response.data == {}
 
-    @override_settings(OPENEXCHANGERATES_API_KEY='test')
-    def test_get_rates(self):
-        def mocked_requests_get(*args, **kwargs):  # pylint: disable=unused-argument
-            class MockResponse:
-                def __init__(self, json_data, status_code, text):
-                    self.json_data = json_data
-                    self.status_code = status_code
-                    self.text = text
+    def test_get_with_external_error(self, admin_client, responses, settings):
+        settings.OPENEXCHANGERATES_API_KEY = 'test'
+        status = 500
+        responses.add(responses.GET, CurrencyView.EXTERNAL_API_URL, json={}, status=status)
 
-                def json(self):
-                    return self.json_data
-            return MockResponse({"bad": "data"}, 500, "baddata")
-        with mock.patch('course_discovery.apps.api.v1.views.currency.requests.get', side_effect=mocked_requests_get):
-            response = self.client.get(self.list_path)
-            response_json = response.json()
-            self.assertEqual(response_json, {})
+        with mock.patch('course_discovery.apps.api.v1.views.currency.logger.error') as mock_logger:
+            response = admin_client.get(self.list_path)
+            mock_logger.assert_called_with(
+                'Failed to retrieve exchange rates from [%s]. Status: [%d], Body: %s',
+                CurrencyView.EXTERNAL_API_URL, status, b'{}'
+            )
+            assert response.status_code == 200
+            assert response.data == {}

course_discovery/apps/api/v1/views/currency.py

@@ -7,40 +7,45 @@ from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 from rest_framework_extensions.cache.decorators import cache_response
 
+logger = logging.getLogger(__name__)
+
 
 class CurrencyView(views.APIView):
     permission_classes = (IsAuthenticated,)
+    EXTERNAL_API_URL = 'https://openexchangerates.org/api/latest.json'
 
     def get_rates(self):
+        app_id = settings.OPENEXCHANGERATES_API_KEY
+
+        if not app_id:
+            logger.warning('Unable to retrieve exchange rate data. No API key is set.')
+            return {}
+
         try:
-            app_id = settings.OPENEXCHANGERATES_API_KEY
-            if app_id:
-                url = 'https://openexchangerates.org/api/latest.json'
-                response = requests.get(url, params={'app_id': app_id}, timeout=2)
+            response = requests.get(self.EXTERNAL_API_URL, params={'app_id': app_id}, timeout=2)
+
+            if response.status_code == requests.codes.ok:  # pylint: disable=no-member
                 response_json = response.json()
-                result = response_json['rates']
-                return result
+                return response_json['rates']
             else:
-                logging.warning('No app id available for openexchangerates')
-                return {}
-        except Exception as e:  # pylint: disable=broad-except
-            response_text = '' if not isinstance(response, object) else response.text
-            message = 'Exception Type {}. Message {}. Response {}.'.format(
-                type(e).__name__, e, response_text
-            )
-            logging.error('Could not retrieve rates from openexchangerates. ' + message)
-            return {}
+                logger.error(
+                    'Failed to retrieve exchange rates from [%s]. Status: [%d], Body: %s',
+                    self.EXTERNAL_API_URL, response.status_code, response.content)
+        except Exception:  # pylint: disable=broad-except
+            logger.exception('An error occurred while requesting exchange rates from [%s]', self.EXTERNAL_API_URL)
+
+        return {}
 
     def get_data(self):
         rates = self.get_rates()
         # ISO 3166-1 alpha-3 codes
         currencies = {
-            'IND': {'code': 'INR', 'symbol': u'₹'},
+            'IND': {'code': 'INR', 'symbol': '₹'},
             'BRA': {'code': 'BRL', 'symbol': 'R$'},
             'MEX': {'code': 'MXN', 'symbol': '$'},
-            'GBR': {'code': 'GBP', 'symbol': u'£'},
+            'GBR': {'code': 'GBP', 'symbol': '£'},
             'AUS': {'code': 'AUD', 'symbol': '$'},
-            'CHN': {'code': 'CNY', 'symbol': u'¥'},
+            'CHN': {'code': 'CNY', 'symbol': '¥'},
             'COL': {'code': 'COP', 'symbol': '$'},
             'PER': {'code': 'PEN', 'symbol': 'S/.'},
             'CAN': {'code': 'CAD', 'symbol': '$'}

requirements/test.txt

@@ -13,6 +13,7 @@ pep8==1.7.0
 pytest==3.0.6
 pytest-django==3.1.2
 pytest-django-ordering==1.0.1
+pytest-responses==0.3.0
 responses==0.7.0
 selenium==3.4.0
 testfixtures==4.13.1