From a7984779f7a40f37c54032c4a69e97c498d4393d Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sat, 13 Jun 2020 01:18:09 -0400 Subject: [PATCH 1/9] Testeando el codigo! --- .gitignore | 1 + fetcher/cache.py | 1 - fetcher/test.py | 217 +++++++++++++++++++++++++++++++++++++++++++++++ pre-commit.sh | 25 +++++- test.sh | 3 + 5 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 fetcher/test.py create mode 100755 test.sh diff --git a/.gitignore b/.gitignore index f9ceff8..5a3690b 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ __pycache__ /db.sqlite3 /static TODO.md +.coverage diff --git a/fetcher/cache.py b/fetcher/cache.py index 87202e1..bf6fa4e 100644 --- a/fetcher/cache.py +++ b/fetcher/cache.py @@ -24,7 +24,6 @@ def get_artist(mbid): def get_disc(mbid): _log.info('Intentando obtener disco %s desde redis', mbid) - with get_redis_connection() as redis: disc = redis.get(f'release_group:{mbid}') if not disc: diff --git a/fetcher/test.py b/fetcher/test.py new file mode 100644 index 0000000..8fa9e3a --- /dev/null +++ b/fetcher/test.py @@ -0,0 +1,217 @@ +import json +import logging +from unittest.mock import Mock, MagicMock, patch +from django.test import TestCase +from fetcher import cache + +# Parece que es molesto el logging en el output del test +logging.disable() + + +class CacheTest(TestCase): + @staticmethod + def mock_redis(mock_connection): + mock_redis = Mock() + mock_connection.return_value = Mock(__enter__=Mock(return_value=mock_redis), + __exit__=Mock(return_value=False)) + + return mock_redis + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_input_not_exists(self, mock_connection, mock_jobs): + """Testear get artist con un mbid no existente""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + response = cache.get_artist('mbid') + self.assertIsNone(response) + + mock_redis.get.assert_called_with('artist:mbid') + mock_jobs.load_artist_on_cache.delay.assert_called_once_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_input_exists(self, mock_connection, mock_jobs): + """Testear get artist con un mbid existente""" + payload = {'id': 'mbid', 'name': 'name'} + + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=json.dumps(payload)) + + response = cache.get_artist('mbid') + + self.assertEquals(payload, response) + + mock_redis.get.assert_called_with('artist:mbid') + mock_jobs.load_artist_on_cache.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_disc_input_not_exists(self, mock_connection, mock_jobs): + """Testear get disc con un mbid no existente""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + response = cache.get_disc('mbid') + self.assertIsNone(response) + + mock_redis.get.assert_called_with('release_group:mbid') + mock_jobs.load_artist_on_cache.delay.asert_called_once_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_disc_input_exists(self, mock_connection, mock_jobs): + """Testear get disc con un mbid existente""" + payload = {'id': 'mbid', 'title': 'oh no'} + + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=json.dumps(payload)) + + response = cache.get_disc('mbid') + self.assertEquals(payload, response) + + mock_redis.get.assert_called_with('release_group:mbid') + mock_jobs.load_artist_on_cache.delay.asert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_discs_of_artists_no_contains(self, mock_connection, mock_jobs): + """Testear get discs of artists con mbid que no esta en redis""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.__contains__ = Mock(return_value=False) + + result, count = cache.get_discs_of_artist('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(count, 0) + + mock_redis.__contains__.assert_called_with('artist:mbid:release_groups') + mock_jobs.load_artist_on_cache.delay.asert_called_once_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_discs_of_artists_bad_count(self, mock_connection, mock_jobs): + """Testear get discs of artists cuando el total no coincide con los almacenados""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=0) + + result, count = cache.get_discs_of_artist('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(count, 0) + + mock_redis.__contains__.assert_called_with('artist:mbid:release_groups') + mock_redis.get.assert_called_with('artist:mbid:release_groups:count') + mock_redis.zcard.assert_called_with('artist:mbid:release_groups') + mock_jobs.load_artist_on_cache.delay.asert_called_once_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_discs_of_artists_not_all_exists(self, mock_connection, mock_jobs): + """Testear get discs of artists cuando no estan guardados todos los discos""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=10) + mock_redis.zrange = Mock(return_value=range(10)) + mock_redis.exists = Mock(return_value=5) + + result, count = cache.get_discs_of_artist('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(count, 0) + + mock_redis.__contains__.assert_called_with('artist:mbid:release_groups') + mock_redis.get.assert_called_with('artist:mbid:release_groups:count') + mock_redis.zcard.assert_called_with('artist:mbid:release_groups') + mock_redis.zrange.assert_called_with('artist:mbid:release_groups', 0, 10) + mock_redis.exists.assert_called_with(*[f'release_group:{i}' for i in range(10)]) + mock_jobs.load_artist_on_cache.delay.asert_called_once_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_discs_of_artists_exists(self, mock_connection, mock_jobs): + """Testear get discs of artists cuando todo va bien""" + payload = [{'id': i} for i in range(10)] + + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=10) + mock_redis.zrange = Mock(return_value=range(10)) + mock_redis.exists = Mock(return_value=10) + mock_redis.mget = Mock(return_value=MagicMock()) + mock_redis.mget.return_value.__iter__.return_value = [json.dumps(item) for item in payload] + + result, count = cache.get_discs_of_artist('mbid', 10, 0) + self.assertEquals(result, payload) + self.assertEquals(count, 10) + + mock_redis.__contains__.assert_called_with('artist:mbid:release_groups') + mock_redis.get.assert_called_with('artist:mbid:release_groups:count') + mock_redis.zcard.assert_called_with('artist:mbid:release_groups') + mock_redis.zrange.assert_called_with('artist:mbid:release_groups', 0, 10) + mock_redis.exists.assert_called_with(*[f'release_group:{i}' for i in range(10)]) + mock_jobs.load_artist_on_cache.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_disc_input_not_exists(self, mock_connection, mock_jobs): + """Testear get artist of disc cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + result = cache.get_artist_of_disc('mbid') + self.assertEquals(result, None) + + mock_redis.get.assert_called_with('release_group:mbid:artist') + mock_jobs.load_entities_of_release_group.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_disc_input_exists(self, mock_connection, mock_jobs): + """Testear get artist of disc cuando existe""" + payload = {'id': 'artist_id', 'title': 'ohno'} + + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'release_group:mbid:artist': + return payload.get('id') + elif key == 'artist:artist_id': + return json.dumps(payload) + else: + self.fail('Key inesperada') + + mock_redis.get = get + + result = cache.get_artist_of_disc('mbid') + self.assertEquals(result, payload) + + mock_jobs.load_entities_of_release_group.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_release_input_not_exists(self, mock_connection, mock_jobs): + """Testear get release cuando no existe""" + payload = {'id': 'artist_id', 'title': 'ohno'} + + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'release_group:mbid:artist': + return payload.get('id') + elif key == 'artist:artist_id': + return json.dumps(payload) + else: + self.fail('Key inesperada') + + mock_redis.get = get + + result = cache.get_artist_of_disc('mbid') + self.assertEquals(result, payload) + + mock_jobs.load_entities_of_release_group.delay.assert_not_called() diff --git a/pre-commit.sh b/pre-commit.sh index 6da2438..f55782c 100755 --- a/pre-commit.sh +++ b/pre-commit.sh @@ -2,5 +2,28 @@ set -eu . venv/bin/activate + +STASH_NAME=pre-commit-$(date +%s) +git stash save -q --keep-index $STASH_NAME + flake8 . -exit $? +FLAKE_8=$? + +./test.sh +TEST=$? + +STASH_NUM=$(git stash list | grep $STASH_NAME | sed -re 's/stash@\{(.*)\}.*/\1/') +if [ -n "$STASH_NUM" ]; then + git stash pop -q stash@{$STASH_NUM} +fi + + +if [ $FLAKE_8 -ne 0 ]; then + exit $FLAKE_8 +fi + +if [ $TEST -ne 0 ]; then + exit $TEST +fi + +exit 0 diff --git a/test.sh b/test.sh new file mode 100755 index 0000000..9bd6c8b --- /dev/null +++ b/test.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +coverage run --source='.' --omit='venv/*' manage.py test From 45147618c16c96cad684e98fdec9c1aadc7a346b Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sat, 13 Jun 2020 01:31:14 -0400 Subject: [PATCH 2/9] htmlcov a gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5a3690b..fffc282 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ __pycache__ /static TODO.md .coverage +htmlcov/ From e3c32094338bd6e6af33f5933c533b9b11c67246 Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sat, 13 Jun 2020 01:56:56 -0400 Subject: [PATCH 3/9] Mas tests <3 --- fetcher/test.py | 109 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 97 insertions(+), 12 deletions(-) diff --git a/fetcher/test.py b/fetcher/test.py index 8fa9e3a..d180981 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -197,21 +197,106 @@ class CacheTest(TestCase): @patch('fetcher.cache.get_redis_connection') def test_get_release_input_not_exists(self, mock_connection, mock_jobs): """Testear get release cuando no existe""" - payload = {'id': 'artist_id', 'title': 'ohno'} + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + result = cache.get_release('mbid') + self.assertEquals(result, None) + + mock_redis.get.assert_called_with('release:mbid') + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_release_input_exists(self, mock_connection, mock_jobs): + """Testear get release cuando existe""" + payload = {'id': 'release_id', 'title': 'ohno'} mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=json.dumps(payload)) - def get(key): - if key == 'release_group:mbid:artist': - return payload.get('id') - elif key == 'artist:artist_id': - return json.dumps(payload) - else: - self.fail('Key inesperada') - - mock_redis.get = get - - result = cache.get_artist_of_disc('mbid') + result = cache.get_release('mbid') self.assertEquals(result, payload) + mock_redis.get.assert_called_with('release:mbid') + mock_jobs.load_entities_of_release.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_discs_not_exists(self, mock_connection, mock_jobs): + """Testear get releases of discs cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.__contains__ = Mock(return_value=False) + + result, total = cache.get_releases_of_disc('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_redis.__contains__.assert_called_with('release_group:mbid:releases') + mock_jobs.load_entities_of_release_group.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_discs_not_all_keys(self, mock_connection, mock_jobs): + """Testear get releases of discs cuando no estan todas las keys""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=5) + + result, total = cache.get_releases_of_disc('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_redis.__contains__.assert_called_with('release_group:mbid:releases') + mock_redis.get.assert_called_with('release_group:mbid:releases:count') + mock_redis.zcard.assert_called_with('release_group:mbid:releases') + mock_jobs.load_entities_of_release_group.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_discs_not_all_keys_loaded(self, mock_connection, mock_jobs): + """Testear get releases of discs cuando no estan todas las keys cargadas""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=10) + mock_redis.zrange = Mock(return_value=range(10)) + mock_redis.exists = Mock(return_value=5) + + result, total = cache.get_releases_of_disc('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_redis.__contains__.assert_called_with('release_group:mbid:releases') + mock_redis.get.assert_called_with('release_group:mbid:releases:count') + mock_redis.zcard.assert_called_with('release_group:mbid:releases') + mock_redis.zrange.assert_called_with('release_group:mbid:releases', 0, 10) + mock_redis.exists.assert_called_with(*[f'release:{id}' for id in range(10)]) + mock_jobs.load_entities_of_release_group.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_discs_all_god(self, mock_connection, mock_jobs): + """Testear get releases of discs cuando existe todo""" + payload = [{'id': i} for i in range(10)] + + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.__contains__ = Mock(return_value=True) + mock_redis.get = Mock(return_value=10) + mock_redis.zcard = Mock(return_value=10) + mock_redis.zrange = Mock(return_value=range(10)) + mock_redis.exists = Mock(return_value=10) + mock_redis.mget = Mock(return_value=MagicMock()) + mock_redis.mget.return_value.__iter__.return_value = [json.dumps(item) for item in payload] + + result, total = cache.get_releases_of_disc('mbid', 10, 0) + self.assertEquals(result, payload) + self.assertEquals(total, 10) + + mock_redis.__contains__.assert_called_with('release_group:mbid:releases') + mock_redis.get.assert_called_with('release_group:mbid:releases:count') + mock_redis.zcard.assert_called_with('release_group:mbid:releases') + mock_redis.zrange.assert_called_with('release_group:mbid:releases', 0, 10) + mock_redis.exists.assert_called_with(*[f'release:{id}' for id in range(10)]) mock_jobs.load_entities_of_release_group.delay.assert_not_called() From a4f3a5c87f854ac0d95e59ec8e90acd9dc905f13 Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 01:44:02 -0400 Subject: [PATCH 4/9] Test para get_artist_of_release --- fetcher/cache.py | 1 + fetcher/test.py | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/fetcher/cache.py b/fetcher/cache.py index bf6fa4e..6074a85 100644 --- a/fetcher/cache.py +++ b/fetcher/cache.py @@ -132,6 +132,7 @@ def get_artist_of_release(mbid): artist = redis.get(f'artist:{artist_id}') if not artist: _log.debug('El artista aun no se carga en redis') + jobs.load_entities_of_release.delay(mbid) return None _log.debug('Se encontro el artista') diff --git a/fetcher/test.py b/fetcher/test.py index d180981..870db9b 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -300,3 +300,63 @@ class CacheTest(TestCase): mock_redis.zrange.assert_called_with('release_group:mbid:releases', 0, 10) mock_redis.exists.assert_called_with(*[f'release:{id}' for id in range(10)]) mock_jobs.load_entities_of_release_group.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_release_input_no_exists(self, mock_connection, mock_jobs): + """Testear get artist of release cuando no existe""" + + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + result = cache.get_artist_of_release('mbid') + self.assertEquals(result, None) + + mock_redis.get.assert_called_with('release:mbid:artist') + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_release_input_not_saved(self, mock_connection, mock_jobs): + """Testear get artist of release cuando existe el id por aun no es guardado""" + + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'release:mbid:artist': + return 'artist_id' + if key == 'artist:artist_id': + return None + else: + self.fail('Key inesperada') + + mock_redis.get = get + + result = cache.get_artist_of_release('mbid') + self.assertEquals(result, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_release_exists(self, mock_connection, mock_jobs): + """Testear get artist of release cuando existe""" + + payload = {'id': 'artist_id', 'name': 'ohno'} + + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'release:mbid:artist': + return 'artist_id' + if key == 'artist:artist_id': + return json.dumps(payload) + else: + self.fail('Key inesperada') + + mock_redis.get = get + + result = cache.get_artist_of_release('mbid') + self.assertEquals(result, payload) + + mock_jobs.load_entities_of_release.delay.assert_not_called() From fb70b829a3a85f4314490a0aeef049426c5a2be0 Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 01:48:23 -0400 Subject: [PATCH 5/9] Tests para get_recording --- fetcher/test.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/fetcher/test.py b/fetcher/test.py index 870db9b..3b69db0 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -360,3 +360,31 @@ class CacheTest(TestCase): self.assertEquals(result, payload) mock_jobs.load_entities_of_release.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recording_not_exists(self, mock_connection, mock_jobs): + """Testear get recording cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=None) + + response = cache.get_recording('mbid') + self.assertEquals(response, None) + + mock_redis.get.assert_called_with('recording:mbid') + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recording_exists(self, mock_connection, mock_jobs): + """Testear get recording cuando existe""" + payload = {'id': 'mbid', 'title': 'ohno'} + + mock_redis = CacheTest.mock_redis(mock_connection) + mock_redis.get = Mock(return_value=json.dumps(payload)) + + response = cache.get_recording('mbid') + self.assertEquals(response, payload) + + mock_redis.get.assert_called_with('recording:mbid') + mock_jobs.load_entities_of_recording.delay.assert_not_called() From 823149bf5b2bee0602bf8a92ceb1376cb54a958c Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 03:32:44 -0400 Subject: [PATCH 6/9] Testeo de get_recordings of release --- fetcher/cache.py | 5 +- fetcher/test.py | 289 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 286 insertions(+), 8 deletions(-) diff --git a/fetcher/cache.py b/fetcher/cache.py index 6074a85..1da902b 100644 --- a/fetcher/cache.py +++ b/fetcher/cache.py @@ -165,7 +165,7 @@ def get_recordings_of_release(mbid): media_count = int(redis.get(f'{medias_key}:count')) if redis.zcard(medias_key) != media_count: - _log.debug('La cantidad de medias de la release no coinciden con el total', mbid) + _log.debug('La cantidad de medias de la release %s no coinciden con el total', mbid) jobs.load_entities_of_release.delay(mbid) return None @@ -179,7 +179,8 @@ def get_recordings_of_release(mbid): recordings_count = int(redis.get(f'{recordings_key}:count')) if redis.zcard(recordings_key) != recordings_count: - _log.debug('La cantidad de recordings de la media no coinciden con el total', mbid) + _log.debug('La cantidad de recordings de la media %s no coinciden con el total', + mbid) jobs.load_entities_of_release.delay(mbid) return None diff --git a/fetcher/test.py b/fetcher/test.py index 3b69db0..3059687 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -1,11 +1,10 @@ import json -import logging -from unittest.mock import Mock, MagicMock, patch -from django.test import TestCase -from fetcher import cache -# Parece que es molesto el logging en el output del test -logging.disable() +from unittest.mock import Mock, MagicMock, patch + +from django.test import TestCase + +from fetcher import cache class CacheTest(TestCase): @@ -388,3 +387,281 @@ class CacheTest(TestCase): mock_redis.get.assert_called_with('recording:mbid') mock_jobs.load_entities_of_recording.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_no_medias(self, mock_connection, mock_jobs): + """Testear get recordings of release cuando no hay ninguna media asociada a la release""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(self, key): + if key == 'release:mbid:media': + return False + else: + self.fail('Key inesperada en __contains__') + + mock_redis.__contains__ = __contains__ + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_not_all_medias(self, mock_connection, mock_jobs): + """Testear get recordings of release cuando no estan todos los ids de medias que + deberian""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'release:mbid:media': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'release:mbid:media:count': + return 2 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'release:mbid:media': + return 1 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_all_medias_no_recordings(self, mock_connection, mock_jobs): + """Testear get recordings of release estan las medias pero ninguna tiene recordings""" + + medias = [{'id': id, 'position': id} for id in range(1, 3)] + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'release:mbid:media': + return True + elif key == 'release:mbid:media:1:recordings': + return False + elif key == 'release:mbid:media:2:recordings': + return False + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'release:mbid:media:count': + return 2 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'release:mbid:media': + return 2 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, finish): + if key == 'release:mbid:media': + return [json.dumps(media) for media in medias] + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_all_medias_bad_recording_count(self, + mock_connection, + mock_jobs): + """Testear get recordings of release con las medias pero el conteo de grabaciones + de la primera, no coincide con el total""" + + medias = [{'id': id, 'position': id} for id in range(1, 3)] + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'release:mbid:media': + return True + elif key == 'release:mbid:media:1:recordings': + return True + elif key == 'release:mbid:media:2:recordings': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'release:mbid:media:count': + return 2 + if key == 'release:mbid:media:1:recordings:count': + return 10 + if key == 'release:mbid:media:2:recordings:count': + return 10 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'release:mbid:media': + return 2 + if key == 'release:mbid:media:1:recordings': + return 5 + if key == 'release:mbid:media:2:recordings': + return 5 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, finish): + if key == 'release:mbid:media': + return [json.dumps(media) for media in medias] + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_all_medias_no_exists_recording(self, + mock_connection, + mock_jobs): + """Testear get recordings of release con las medias pero no todos los recordings existen""" + medias = [{'id': id, 'position': id} for id in range(1, 3)] + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'release:mbid:media': + return True + elif key == 'release:mbid:media:1:recordings': + return True + elif key == 'release:mbid:media:2:recordings': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'release:mbid:media:count': + return 2 + if key == 'release:mbid:media:1:recordings:count': + return 10 + if key == 'release:mbid:media:2:recordings:count': + return 10 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'release:mbid:media': + return 2 + if key == 'release:mbid:media:1:recordings': + return 10 + if key == 'release:mbid:media:2:recordings': + return 10 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, finish): + if key == 'release:mbid:media': + return [json.dumps(media) for media in medias] + elif key == 'release:mbid:media:1:recordings': + return range(1, 11) + elif key == 'release:mbid:media:2:recordings': + return range(1, 11) + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + mock_redis.exists = Mock(return_value=8) + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_recordings_of_release_all_good(self, mock_connection, mock_jobs): + """Testear get recordings of release cuando esta todo como se espera""" + medias = [{'id': id, 'position': id} for id in range(1, 3)] + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'release:mbid:media': + return True + elif key == 'release:mbid:media:1:recordings': + return True + elif key == 'release:mbid:media:2:recordings': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'release:mbid:media:count': + return 2 + if key == 'release:mbid:media:1:recordings:count': + return 10 + if key == 'release:mbid:media:2:recordings:count': + return 10 + if key in [f'recording:{i}' for i in range(1, 11)]: + return json.dumps({'id': 'someid', 'title': 'sometitle'}) + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'release:mbid:media': + return 2 + if key == 'release:mbid:media:1:recordings': + return 10 + if key == 'release:mbid:media:2:recordings': + return 10 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, finish): + if key == 'release:mbid:media': + return [json.dumps(media) for media in medias] + elif key == 'release:mbid:media:1:recordings': + return range(1, 11) + elif key == 'release:mbid:media:2:recordings': + return range(1, 11) + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + mock_redis.exists = Mock(return_value=10) + + expected = [{'id': i, + 'position': i, + 'recordings': [{'id': 'someid', 'title': 'sometitle'} for i in range(10)]} + for i in range(1, 3)] + + response = cache.get_recordings_of_release('mbid') + self.assertEquals(response, expected) + + mock_jobs.load_entities_of_release.delay.assert_not_called() From 907cfe92d5b93e43583f3c4108d611a862cd30b4 Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 03:59:00 -0400 Subject: [PATCH 7/9] Testeados get_releases_of_recordings --- fetcher/test.py | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ test.sh | 2 +- 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/fetcher/test.py b/fetcher/test.py index 3059687..9c6a726 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -665,3 +665,153 @@ class CacheTest(TestCase): self.assertEquals(response, expected) mock_jobs.load_entities_of_release.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_recording_no_exists(self, mock_connection, mock_jobs): + """Testear get releases of recording cuando no existe""" + + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'recording:mbid:release': + return False + elif key == 'recording:mbid:release:count': + return False + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + result, total = cache.get_releases_of_recording('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_recording_not_all(self, mock_connection, mock_jobs): + """Testear get releases of recording cuando no coinciden las keys con la cantidad + almacenada""" + + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'recording:mbid:release': + return True + elif key == 'recording:mbid:release:count': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'recording:mbid:release:count': + return 10 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'recording:mbid:release': + return 5 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + result, total = cache.get_releases_of_recording('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_recording_all_but_no_exists(self, mock_connection, mock_jobs): + """Testear get releases of recording cuando estan todas las keys pero no existen todas""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'recording:mbid:release': + return True + elif key == 'recording:mbid:release:count': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'recording:mbid:release:count': + return 10 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'recording:mbid:release': + return 10 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, end): + if key == 'recording:mbid:release': + return range(10) + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + mock_redis.exists = Mock(return_value=8) + + result, total = cache.get_releases_of_recording('mbid', 10, 0) + self.assertEquals(result, None) + self.assertEquals(total, 0) + + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_releases_of_recording_all_good(self, mock_connection, mock_jobs): + """Testear get releases of recording cuando esta todo""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def __contains__(_, key): + if key == 'recording:mbid:release': + return True + elif key == 'recording:mbid:release:count': + return True + else: + self.fail(f'Key inesperada en __contains__: {key}') + mock_redis.__contains__ = __contains__ + + def get(key): + if key == 'recording:mbid:release:count': + return 10 + else: + self.fail(f'Key inesperada en get: {key}') + mock_redis.get = get + + def zcard(key): + if key == 'recording:mbid:release': + return 10 + else: + self.fail(f'Key inesperada en zcard: {key}') + mock_redis.zcard = zcard + + def zrange(key, start, end): + if key == 'recording:mbid:release': + return range(10) + else: + self.fail(f'Key inesperada en zrange: {key}') + mock_redis.zrange = zrange + + mock_redis.exists = Mock(return_value=10) + + releases = [{'id': i} for i in range(10)] + mock_redis.mget = Mock(return_value=[json.dumps(release) for release in releases]) + + result, total = cache.get_releases_of_recording('mbid', 10, 0) + self.assertEquals(result, releases) + self.assertEquals(total, 10) + + mock_jobs.load_entities_of_recording.delay.assert_not_called() diff --git a/test.sh b/test.sh index 9bd6c8b..e19e9d7 100755 --- a/test.sh +++ b/test.sh @@ -1,3 +1,3 @@ #!/bin/sh -coverage run --source='.' --omit='venv/*' manage.py test +coverage run --source='.' --omit='venv/*' manage.py test --failfast From 71fe9523a929d73722173b1f14eb104a2e4e92bc Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 04:04:16 -0400 Subject: [PATCH 8/9] Tests de get_artist_of_recording --- fetcher/cache.py | 1 + fetcher/test.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/fetcher/cache.py b/fetcher/cache.py index 1da902b..4912c5c 100644 --- a/fetcher/cache.py +++ b/fetcher/cache.py @@ -239,6 +239,7 @@ def get_artist_of_recording(mbid): artist = redis.get(f'artist:{artist_id}') if not artist: _log.debug('El artista aun no se carga en redis') + jobs.load_entities_of_recording.delay(mbid) return None _log.debug('Se encontro el artista') diff --git a/fetcher/test.py b/fetcher/test.py index 9c6a726..aa866e6 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -815,3 +815,64 @@ class CacheTest(TestCase): self.assertEquals(total, 10) mock_jobs.load_entities_of_recording.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_recordings_input_not_exists(self, mock_connection, mock_jobs): + """Testear get artist of recording cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'recording:mbid:artist': + return None + else: + self.fail(f'Key inesperada en get: {key}') + + mock_redis.get = get + + result = cache.get_artist_of_recording('mbid') + self.assertEquals(result, None) + + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_recordings_not_loaded(self, mock_connection, mock_jobs): + """Testear get artist of recording cuando existe id pero no esta cargado""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'recording:mbid:artist': + return 'artist_id' + elif key == 'artist:artist_id': + return None + else: + self.fail(f'Key inesperada en get: {key}') + + mock_redis.get = get + + result = cache.get_artist_of_recording('mbid') + self.assertEquals(result, None) + + mock_jobs.load_entities_of_recording.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_artist_of_recordings_exists(self, mock_connection, mock_jobs): + """Testear get artist of recording cuando existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + def get(key): + if key == 'recording:mbid:artist': + return 'artist_id' + elif key == 'artist:artist_id': + return json.dumps({'id': 'artist_id'}) + else: + self.fail(f'Key inesperada en get: {key}') + + mock_redis.get = get + + result = cache.get_artist_of_recording('mbid') + self.assertEquals(result, {'id': 'artist_id'}) + + mock_jobs.load_entities_of_recording.delay.assert_not_called() From a17d024393e597fa13c9996d46a1d5c37fce9e6a Mon Sep 17 00:00:00 2001 From: Daniel Cortes Date: Sun, 14 Jun 2020 04:08:18 -0400 Subject: [PATCH 9/9] Ultimos tests de cache --- fetcher/cache.py | 2 ++ fetcher/test.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/fetcher/cache.py b/fetcher/cache.py index 4912c5c..1fca240 100644 --- a/fetcher/cache.py +++ b/fetcher/cache.py @@ -252,6 +252,7 @@ def get_cover_art_disc(mbid): with get_redis_connection() as redis: covers = redis.get(f'release_group:{mbid}:cover_art') if not covers: + jobs.load_entities_of_release_group.delay(mbid) return None return json.loads(covers) @@ -263,6 +264,7 @@ def get_cover_art_release(mbid): with get_redis_connection() as redis: covers = redis.get(f'release:{mbid}:cover_art') if not covers: + jobs.load_entities_of_release.delay(mbid) return None return json.loads(covers) diff --git a/fetcher/test.py b/fetcher/test.py index aa866e6..6808c99 100644 --- a/fetcher/test.py +++ b/fetcher/test.py @@ -876,3 +876,55 @@ class CacheTest(TestCase): self.assertEquals(result, {'id': 'artist_id'}) mock_jobs.load_entities_of_recording.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_cover_art_disc_no_exists(self, mock_connection, mock_jobs): + """Testear get cover art of disc cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.get = Mock(return_value=None) + + result = cache.get_cover_art_disc('mbid') + self.assertEquals(result, None) + + mock_jobs.load_entities_of_release_group.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_cover_art_disc_exists(self, mock_connection, mock_jobs): + """Testear get cover art of disc cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.get = Mock(return_value=json.dumps({'id': 'mbid'})) + + result = cache.get_cover_art_disc('mbid') + self.assertEquals(result, {'id': 'mbid'}) + + mock_jobs.load_entities_of_release_group.delay.assert_not_called() + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_cover_art_release_no_exists(self, mock_connection, mock_jobs): + """Testear get cover art of release cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.get = Mock(return_value=None) + + result = cache.get_cover_art_release('mbid') + self.assertEquals(result, None) + + mock_jobs.load_entities_of_release.delay.assert_called_with('mbid') + + @patch('fetcher.cache.jobs') + @patch('fetcher.cache.get_redis_connection') + def test_get_cover_art_release_exists(self, mock_connection, mock_jobs): + """Testear get cover art of disc cuando no existe""" + mock_redis = CacheTest.mock_redis(mock_connection) + + mock_redis.get = Mock(return_value=json.dumps({'id': 'mbid'})) + + result = cache.get_cover_art_release('mbid') + self.assertEquals(result, {'id': 'mbid'}) + + mock_jobs.load_entities_of_release.delay.assert_not_called()