Merge pull request #4533 from mboersma/fixup-controller-tests

ref(controller): remove REGISTRY_MODULE hack

Author: mboersma

controller/Makefile

@@ -74,7 +74,7 @@ setup-venv:
 	venv/bin/pip install --disable-pip-version-check -q -r requirements.txt -r dev_requirements.txt
 
 test-style: setup-venv
-	venv/bin/flake8
+	venv/bin/flake8 --show-pep8 --show-source
 	shellcheck $(SHELL_SCRIPTS)
 
 test-unit: setup-venv test-style

controller/api/models.py

@@ -867,8 +867,7 @@ def publish(self, source_version='latest'):
         source_tag = 'git-{}'.format(self.build.sha) if self.build.sha else source_version
         source_image = '{}:{}'.format(self.build.image, source_tag)
         # IOW, this image did not come from the builder
-        # FIXME: remove check for mock registry module
-        if not self.build.sha and 'mock' not in settings.REGISTRY_MODULE:
+        if not self.build.sha:
             # we assume that the image is not present on our registry,
             # so shell out a task to pull in the repository
             data = {

controller/api/tests/__init__.py

@@ -6,6 +6,7 @@
 from django.conf import settings
 from django.test.client import RequestFactory, Client
 from django.test.simple import DjangoTestSuiteRunner
+import requests
 
 
 # add patch support to built-in django test client
@@ -46,18 +47,25 @@ def run_tests(self, test_labels, extra_tests=None, **kwargs):
             test_labels, extra_tests, **kwargs)
 
 
+def mock_status_ok(*args, **kwargs):
+    resp = requests.Response()
+    resp.status_code = 200
+    resp._content_consumed = True
+    return resp
+
+
 from .test_api_middleware import *  # noqa
 from .test_app import *  # noqa
 from .test_auth import *  # noqa
 from .test_build import *  # noqa
-from .test_config import *  # noqa
-from .test_domain import *  # noqa
 from .test_certificate import *  # noqa
+from .test_config import *  # noqa
 from .test_container import *  # noqa
+from .test_domain import *  # noqa
 from .test_hooks import *  # noqa
 from .test_key import *  # noqa
+from .test_limits import *  # noqa
 from .test_perm import *  # noqa
 from .test_release import *  # noqa
 from .test_scheduler import *  # noqa
 from .test_users import *  # noqa
-from .test_limits import *  # noqa

controller/api/tests/test_app.py

@@ -10,21 +10,14 @@
 import logging
 import mock
 import os.path
-import requests
 
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.test import TestCase
 from rest_framework.authtoken.models import Token
 
 from api.models import App
-
-
-def mock_import_repository_task(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
+from . import mock_status_ok
 
 
 class AppTest(TestCase):
@@ -198,7 +191,7 @@ def test_app_structure_is_valid_json(self):
         self.assertIn('structure', response.data)
         self.assertEqual(response.data['structure'], {"web": 1})
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     @mock.patch('api.models.logger')
     def test_admin_can_manage_other_apps(self, mock_logger):
         """Administrators of Deis should be able to manage all applications.

controller/api/tests/test_build.py

@@ -7,23 +7,17 @@
 from __future__ import unicode_literals
 
 import json
-import mock
-import requests
 
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
+import mock
 from rest_framework.authtoken.models import Token
 
 from api.models import Build
+from . import mock_status_ok
 
 
-def mock_import_repository_task(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
-
-
+@mock.patch('api.models.publish_release', lambda *args: None)
 class BuildTest(TransactionTestCase):
 
     """Tests build notification from build system"""
@@ -34,7 +28,7 @@ def setUp(self):
         self.user = User.objects.get(username='autotest')
         self.token = Token.objects.get(user=self.user).key
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_build(self):
         """
         Test that a null build is created and that users can post new builds
@@ -82,7 +76,7 @@ def test_build(self):
         response = self.client.delete(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 405)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_response_data(self):
         """Test that the serialized response contains only relevant data."""
         body = {'id': 'test'}
@@ -108,7 +102,7 @@ def test_response_data(self):
         }
         self.assertDictContainsSubset(expected, response.data)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_build_default_containers(self):
         url = '/v1/apps'
         response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
@@ -194,7 +188,7 @@ def test_build_default_containers(self):
         self.assertEqual(container['type'], 'web')
         self.assertEqual(container['num'], 1)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_build_str(self):
         """Test the text representation of a build."""
         url = '/v1/apps'
@@ -211,7 +205,7 @@ def test_build_str(self):
         self.assertEqual(str(build), "{}-{}".format(
                          response.data['app'], response.data['uuid'][:7]))
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_admin_can_create_builds_on_other_apps(self):
         """If a user creates an application, an administrator should be able
         to push builds.
@@ -233,7 +227,7 @@ def test_admin_can_create_builds_on_other_apps(self):
         self.assertEqual(str(build), "{}-{}".format(
                          response.data['app'], response.data['uuid'][:7]))
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_unauthorized_user_cannot_modify_build(self):
         """
         An unauthorized user should not be able to modify other builds.
@@ -254,7 +248,7 @@ def test_unauthorized_user_cannot_modify_build(self):
                                     HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
         self.assertEqual(response.status_code, 403)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_new_build_does_not_scale_up_automatically(self):
         """
         After the first initial deploy, if the containers are scaled down to zero,

controller/api/tests/test_config.py

@@ -9,23 +9,17 @@
 
 import json
 import logging
-import mock
 import requests
 
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
 import etcd
+import mock
 from rest_framework.authtoken.models import Token
 
 import api.exceptions
 from api.models import App, Config
-
-
-def mock_status_ok(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
+from . import mock_status_ok
 
 
 def mock_status_not_found(*args, **kwargs):
@@ -52,6 +46,7 @@ def get(self, key, *args, **kwargs):
         return etcd.EtcdResult(None, node)
 
 
+@mock.patch('api.models.publish_release', lambda *args: None)
 class ConfigTest(TransactionTestCase):
 
     """Tests setting and updating config values"""

controller/api/tests/test_container.py

@@ -7,24 +7,18 @@
 from __future__ import unicode_literals
 
 import json
-import mock
-import requests
 
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
-from scheduler.states import TransitionError
+import mock
 from rest_framework.authtoken.models import Token
 
 from api.models import App, Build, Container, Release
+from scheduler.states import TransitionError
+from . import mock_status_ok
 
 
-def mock_import_repository_task(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
-
-
+@mock.patch('api.models.publish_release', lambda *args: None)
 class ContainerTest(TransactionTestCase):
     """Tests creation of containers on nodes"""
 
@@ -170,7 +164,7 @@ def test_container_api_heroku(self):
         response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 200)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_container_api_docker(self):
         url = '/v1/apps'
         response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
@@ -233,7 +227,7 @@ def test_container_api_docker(self):
         response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 200)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_container_release(self):
         url = '/v1/apps'
         response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))

controller/api/tests/test_hooks.py

@@ -7,22 +7,17 @@
 from __future__ import unicode_literals
 
 import json
-import mock
-import requests
 
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
+import mock
 from rest_framework.authtoken.models import Token
 
-
-def mock_import_repository_task(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
+from . import mock_status_ok
 
 
+@mock.patch('api.models.publish_release', lambda *args: None)
 class HookTest(TransactionTestCase):
 
     """Tests API hooks used to trigger actions from external components"""
@@ -97,7 +92,7 @@ def test_push_abuse(self):
                                     HTTP_X_DEIS_BUILDER_AUTH=settings.BUILDER_KEY)
         self.assertEqual(response.status_code, 403)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_build_hook(self):
         """Test creating a Build via an API Hook"""
         url = '/v1/apps'

controller/api/tests/test_release.py

@@ -7,23 +7,17 @@
 from __future__ import unicode_literals
 
 import json
-import mock
-import requests
 
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
+import mock
 from rest_framework.authtoken.models import Token
 
 from api.models import Release
+from . import mock_status_ok
 
 
-def mock_import_repository_task(*args, **kwargs):
-    resp = requests.Response()
-    resp.status_code = 200
-    resp._content_consumed = True
-    return resp
-
-
+@mock.patch('api.models.publish_release', lambda *args: None)
 class ReleaseTest(TransactionTestCase):
 
     """Tests push notification from build system"""
@@ -34,7 +28,7 @@ def setUp(self):
         self.user = User.objects.get(username='autotest')
         self.token = Token.objects.get(user=self.user).key
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_release(self):
         """
         Test that a release is created when an app is created, and
@@ -111,7 +105,7 @@ def test_release(self):
         self.assertEqual(response.status_code, 405)
         return release3
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_response_data(self):
         body = {'id': 'test'}
         response = self.client.post('/v1/apps', json.dumps(body),
@@ -136,7 +130,7 @@ def test_response_data(self):
         }
         self.assertDictContainsSubset(expected, response.data)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_release_rollback(self):
         url = '/v1/apps'
         response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
@@ -233,22 +227,22 @@ def test_release_rollback(self):
         self.assertIn('NEW_URL1', values)
         self.assertEqual('http://localhost:8080/', values['NEW_URL1'])
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_release_str(self):
         """Test the text representation of a release."""
         release3 = self.test_release()
         release = Release.objects.get(uuid=release3['uuid'])
         self.assertEqual(str(release), "{}-v3".format(release3['app']))
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_release_summary(self):
         """Test the text summary of a release."""
         release3 = self.test_release()
         release = Release.objects.get(uuid=release3['uuid'])
         # check that the release has push and env change messages
         self.assertIn('autotest deployed ', release.summary)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_admin_can_create_release(self):
         """If a non-user creates an app, an admin should be able to create releases."""
         user = User.objects.get(username='autotest2')
@@ -272,7 +266,7 @@ def test_admin_can_create_release(self):
         # account for the config release as well
         self.assertEqual(response.data['count'], 2)
 
-    @mock.patch('requests.post', mock_import_repository_task)
+    @mock.patch('requests.post', mock_status_ok)
     def test_unauthorized_user_cannot_modify_release(self):
         """
         An unauthorized user should not be able to modify other releases.

controller/api/tests/test_scheduler.py

@@ -11,11 +11,13 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
+import mock
 from rest_framework.authtoken.models import Token
 
 from scheduler import chaos
 
 
+@mock.patch('api.models.publish_release', lambda *args: None)
 class SchedulerTest(TransactionTestCase):
     """Tests creation of containers on nodes"""