Merge lp:~fgallina/django-preflight/safer-cache-gather into lp:django-preflight

[Fabián Ezequiel Gallina]

More robust hit_rate calculation for gather_caches

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

=== modified file 'preflight/models.py'
--- preflight/models.py	2014-01-28 17:37:22 +0000
+++ preflight/models.py	2014-07-14 19:46:56 +0000
@@ -275,8 +275,11 @@
         except Exception as exc:
             stats = dict(error=unicode(exc))
         else:
-            stats['hit_rate'] = 100 * int(
-                stats.get('get_hits')) / int(stats.get('cmd_get'))
+            try:
+                stats['hit_rate'] = 100 * int(
+                    stats.get('get_hits')) / int(stats.get('cmd_get'))
+            except (ZeroDivisionError, TypeError):
+                stats['hit_rate'] = 0
 
         stats['backend'] = cache_settings['BACKEND']
         stats['host'] = cache_settings.get('LOCATION', 'No location given')
 
=== modified file 'preflight/tests.py'
--- preflight/tests.py	2014-01-28 17:37:22 +0000
+++ preflight/tests.py	2014-07-14 19:46:56 +0000
@@ -5,6 +5,7 @@
 from cStringIO import StringIO
 
 from django.contrib.auth.models import AnonymousUser, User
+from django.conf import settings
 from django.core.management import call_command
 from django.core.urlresolvers import reverse
 from django.http import HttpResponse
@@ -31,6 +32,7 @@
     Preflight,
     authenticate,
     cleanse_setting,
+    gather_caches,
     gather_checks,
     gather_switches,
     gather_gargoyle,
@@ -230,6 +232,87 @@
         self.assertFalse(self.basic_preflight.authenticate(request))
 
 
+class GatherCaches(TestCase):
+
+    def setUp(self):
+        CACHES = {
+            'default': {
+                'BACKEND': ('django.core.cache.backends.'
+                            'memcached.MemcachedCache'),
+                'LOCATION': 'default_location',
+            },
+            'extra': {
+                'BACKEND': ('django.core.cache.backends.'
+                            'locmem.LocMemCache'),
+                'LOCATION': 'extra_location',
+            }
+        }
+        patcher = patch.multiple(settings, CACHES=CACHES)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def set_stats(self, stats):
+        patcher = patch('preflight.models.get_cache')
+        mock_get_cache = patcher.start()
+        self.addCleanup(patcher.stop)
+        MockCache = Mock()
+        if isinstance(stats, dict):
+            mock_stats = [[None, stats]]
+            MockCache._cache.get_stats.return_value = mock_stats
+        else:
+            mock_stats = []
+            for stat in stats:
+                mock_stats.append([[None, stat]])
+            MockCache._cache.get_stats.side_effect = mock_stats
+        mock_get_cache.return_value = MockCache
+
+    def test_not_raises_zero_division_error(self):
+        self.set_stats([{'get_hits': 10, 'cmd_get': 0},
+                        {'get_hits': 10, 'cmd_get': 10}])
+        caches = gather_caches()
+
+        expected = {
+            'default': {
+                'backend': settings.CACHES['default']['BACKEND'],
+                'host': settings.CACHES['default']['LOCATION'],
+                'hit_rate': 0,
+                'get_hits': 10,
+                'cmd_get': 0,
+            },
+            'extra': {
+                'backend': settings.CACHES['extra']['BACKEND'],
+                'host': settings.CACHES['extra']['LOCATION'],
+                'hit_rate': 100,
+                'get_hits': 10,
+                'cmd_get': 10,
+            }
+        }
+        self.assertEqual(caches, expected)
+
+    def test_not_raises_type_error(self):
+        self.set_stats([{'get_hits': 10, 'cmd_get': None},
+                        {'get_hits': None, 'cmd_get': 10}])
+        caches = gather_caches()
+
+        expected = {
+            'default': {
+                'backend': settings.CACHES['default']['BACKEND'],
+                'host': settings.CACHES['default']['LOCATION'],
+                'hit_rate': 0,
+                'get_hits': 10,
+                'cmd_get': None,
+            },
+            'extra': {
+                'backend': settings.CACHES['extra']['BACKEND'],
+                'host': settings.CACHES['extra']['LOCATION'],
+                'hit_rate': 0,
+                'get_hits': None,
+                'cmd_get': 10,
+            }
+        }
+        self.assertEqual(caches, expected)
+
+
 class GatherChecksTestCase(TestCase):
 
     def test(self):