From c972fb253d5c2cc88ed54c65545497a3638df4fb Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Fri, 2 Nov 2018 15:55:37 -0500 Subject: [PATCH] Add test for build redirect --- readthedocs/builds/views.py | 31 ++++-- readthedocs/core/utils/__init__.py | 49 +++++---- readthedocs/rtd_tests/tests/test_views.py | 126 +++++++++++++++------- 3 files changed, 143 insertions(+), 63 deletions(-) diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index c44578b2863..d8a9e7d45c5 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- + """Views for builds app.""" from __future__ import ( @@ -35,8 +36,13 @@ class BuildBase(object): def get_queryset(self): self.project_slug = self.kwargs.get('project_slug', None) - self.project = get_object_or_404(Project.objects.protected(self.request.user),slug=self.project_slug,) - queryset = Build.objects.public(user=self.request.user, project=self.project) + self.project = get_object_or_404( + Project.objects.protected(self.request.user), + slug=self.project_slug, + ) + queryset = Build.objects.public( + user=self.request.user, project=self.project + ) return queryset @@ -57,10 +63,10 @@ def post(self, request, project_slug): slug=version_slug, ) - _, signature = trigger_build(project=project, version=version) - build_pk = signature.get('kwargs', {}).get('build_pk') + _, build = trigger_build(project=project, version=version) return HttpResponseRedirect( - reverse('builds_detail', args=[project.slug, build_pk])) + reverse('builds_detail', args=[project.slug, build.pk]), + ) class BuildList(BuildBase, BuildTriggerMixin, ListView): @@ -68,11 +74,14 @@ class BuildList(BuildBase, BuildTriggerMixin, ListView): def get_context_data(self, **kwargs): context = super(BuildList, self).get_context_data(**kwargs) - active_builds = self.get_queryset().exclude(state='finished').values('id') + active_builds = self.get_queryset().exclude(state='finished' + ).values('id') context['project'] = self.project context['active_builds'] = active_builds - context['versions'] = Version.objects.public(user=self.request.user, project=self.project) + context['versions'] = Version.objects.public( + user=self.request.user, project=self.project + ) context['build_qs'] = self.get_queryset() return context @@ -91,8 +100,12 @@ def get_context_data(self, **kwargs): def builds_redirect_list(request, project_slug): # pylint: disable=unused-argument - return HttpResponsePermanentRedirect(reverse('builds_project_list', args=[project_slug])) + return HttpResponsePermanentRedirect( + reverse('builds_project_list', args=[project_slug]) + ) def builds_redirect_detail(request, project_slug, pk): # pylint: disable=unused-argument - return HttpResponsePermanentRedirect(reverse('builds_detail', args=[project_slug, pk])) + return HttpResponsePermanentRedirect( + reverse('builds_detail', args=[project_slug, pk]) + ) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 6393ed8eba5..ea221c4149a 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- + """Common utilty functions.""" from __future__ import absolute_import @@ -14,13 +15,11 @@ from django.utils.functional import allow_lazy from django.utils.safestring import SafeText, mark_safe from django.utils.text import slugify as slugify_base -from future.backports.urllib.parse import urlparse from celery import group, chord from readthedocs.builds.constants import LATEST, BUILD_STATE_TRIGGERED from readthedocs.doc_builder.constants import DOCKER_LIMITS - log = logging.getLogger(__name__) SYNC_USER = getattr(settings, 'SYNC_USER', getpass.getuser()) @@ -40,9 +39,9 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable= kwargs = {} default_queue = getattr(settings, 'CELERY_DEFAULT_QUEUE', 'celery') if type in ['web', 'app']: - servers = getattr(settings, "MULTIPLE_APP_SERVERS", [default_queue]) + servers = getattr(settings, 'MULTIPLE_APP_SERVERS', [default_queue]) elif type in ['build']: - servers = getattr(settings, "MULTIPLE_BUILD_SERVERS", [default_queue]) + servers = getattr(settings, 'MULTIPLE_BUILD_SERVERS', [default_queue]) tasks = [] for server in servers: @@ -71,7 +70,12 @@ def cname_to_slug(host): def prepare_build( - project, version=None, record=True, force=False, immutable=True): + project, + version=None, + record=True, + force=False, + immutable=True, +): """ Prepare a build in a Celery task for project and version. @@ -132,11 +136,14 @@ def prepare_build( options['soft_time_limit'] = time_limit options['time_limit'] = int(time_limit * 1.2) - return update_docs_task.signature( - args=(project.pk,), - kwargs=kwargs, - options=options, - immutable=True, + return ( + update_docs_task.signature( + args=(project.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ), + build, ) @@ -153,7 +160,7 @@ def trigger_build(project, version=None, record=True, force=False): :param force: build the HTML documentation even if the files haven't changed :returns: A tuple (Celery AsyncResult promise, Task Signature from ``prepare_build``) """ - update_docs_task = prepare_build( + update_docs_task, build = prepare_build( project, version, record, @@ -165,11 +172,13 @@ def trigger_build(project, version=None, record=True, force=False): # Current project is skipped return None - return (update_docs_task.apply_async(), update_docs_task) + return (update_docs_task.apply_async(), build) -def send_email(recipient, subject, template, template_html, context=None, - request=None, from_email=None, **kwargs): # pylint: disable=unused-argument +def send_email( + recipient, subject, template, template_html, context=None, request=None, + from_email=None, **kwargs +): # pylint: disable=unused-argument """ Alter context passed in and call email send task. @@ -183,10 +192,14 @@ def send_email(recipient, subject, template, template_html, context=None, if context is None: context = {} context['uri'] = '{scheme}://{host}'.format( - scheme='https', host=settings.PRODUCTION_DOMAIN) - send_email_task.delay(recipient=recipient, subject=subject, template=template, - template_html=template_html, context=context, from_email=from_email, - **kwargs) + scheme='https', + host=settings.PRODUCTION_DOMAIN, + ) + send_email_task.delay( + recipient=recipient, subject=subject, template=template, + template_html=template_html, context=context, from_email=from_email, + **kwargs + ) def slugify(value, *args, **kwargs): diff --git a/readthedocs/rtd_tests/tests/test_views.py b/readthedocs/rtd_tests/tests/test_views.py index c1bcf92303e..cfe90e1dc4c 100644 --- a/readthedocs/rtd_tests/tests/test_views.py +++ b/readthedocs/rtd_tests/tests/test_views.py @@ -1,19 +1,26 @@ -from __future__ import absolute_import +# -*- coding: utf-8 -*- +from __future__ import ( + absolute_import, + division, + print_function, + unicode_literals, +) + +import mock from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.test import TestCase from django.utils.six.moves.urllib.parse import urlsplit -from django_dynamic_fixture import get -from django_dynamic_fixture import new +from django_dynamic_fixture import get, new from readthedocs.builds.constants import LATEST +from readthedocs.builds.models import Build from readthedocs.core.permissions import AdminPermission -from readthedocs.projects.models import ImportedFile -from readthedocs.projects.models import Project from readthedocs.projects.forms import UpdateProjectForm - +from readthedocs.projects.models import ImportedFile, Project class Testmaker(TestCase): + def setUp(self): self.eric = User(username='eric') self.eric.set_password('test') @@ -27,21 +34,24 @@ def test_imported_docs(self): self.assertEqual(r.status_code, 200) r = self.client.get('/dashboard/import/manual/', {}) self.assertEqual(r.status_code, 200) - form = UpdateProjectForm(data={ - 'name': 'Django Kong', - 'repo': 'https://github.com/ericholscher/django-kong', - 'repo_type': 'git', - 'description': 'OOHHH AH AH AH KONG SMASH', - 'language': 'en', - 'default_branch': '', - 'project_url': 'http://django-kong.rtfd.org', - 'default_version': LATEST, - 'privacy_level': 'public', - 'version_privacy_level': 'public', - 'python_interpreter': 'python', - 'documentation_type': 'sphinx', - 'csrfmiddlewaretoken': '34af7c8a5ba84b84564403a280d9a9be', - }, user=user) + form = UpdateProjectForm( + data={ + 'name': 'Django Kong', + 'repo': 'https://github.com/ericholscher/django-kong', + 'repo_type': 'git', + 'description': 'OOHHH AH AH AH KONG SMASH', + 'language': 'en', + 'default_branch': '', + 'project_url': 'http://django-kong.rtfd.org', + 'default_version': LATEST, + 'privacy_level': 'public', + 'version_privacy_level': 'public', + 'python_interpreter': 'python', + 'documentation_type': 'sphinx', + 'csrfmiddlewaretoken': '34af7c8a5ba84b84564403a280d9a9be', + }, + user=user, + ) _ = form.save() _ = Project.objects.get(slug='django-kong') @@ -111,11 +121,13 @@ def test_project_delete(self): def test_subprojects_delete(self): # This URL doesn't exist anymore, 404 response = self.client.get( - '/dashboard/pip/subprojects/delete/a-subproject/') + '/dashboard/pip/subprojects/delete/a-subproject/', + ) self.assertEqual(response.status_code, 404) # New URL response = self.client.get( - '/dashboard/pip/subprojects/a-subproject/delete/') + '/dashboard/pip/subprojects/a-subproject/delete/', + ) self.assertRedirectToLogin(response) def test_subprojects(self): @@ -143,7 +155,9 @@ def test_project_translations(self): self.assertRedirectToLogin(response) def test_project_translations_delete(self): - response = self.client.get('/dashboard/pip/translations/delete/a-translation/') + response = self.client.get( + '/dashboard/pip/translations/delete/a-translation/' + ) self.assertRedirectToLogin(response) def test_project_redirects(self): @@ -168,7 +182,8 @@ def setUp(self): slug='file', path='file.html', md5='abcdef', - commit='1234567890abcdef') + commit='1234567890abcdef', + ) def test_random_page_view_redirects(self): response = self.client.get('/random/') @@ -188,7 +203,9 @@ def test_404_for_with_no_imported_files(self): response = self.client.get('/random/pip/') self.assertEqual(response.status_code, 404) + class SubprojectViewTests(TestCase): + def setUp(self): self.user = new(User, username='test') self.user.set_password('test') @@ -201,10 +218,15 @@ def setUp(self): self.client.login(username='test', password='test') def test_deny_delete_for_non_project_admins(self): - response = self.client.get('/dashboard/my-mainproject/subprojects/delete/my-subproject/') + response = self.client.get( + '/dashboard/my-mainproject/subprojects/delete/my-subproject/' + ) self.assertEqual(response.status_code, 404) - self.assertTrue(self.subproject in [r.child for r in self.project.subprojects.all()]) + self.assertTrue( + self.subproject in + [r.child for r in self.project.subprojects.all()] + ) def test_admins_can_delete_subprojects(self): self.project.users.add(self.user) @@ -212,24 +234,56 @@ def test_admins_can_delete_subprojects(self): # URL doesn't exist anymore, 404 response = self.client.get( - '/dashboard/my-mainproject/subprojects/delete/my-subproject/') + '/dashboard/my-mainproject/subprojects/delete/my-subproject/', + ) self.assertEqual(response.status_code, 404) # This URL still doesn't accept GET, 405 response = self.client.get( - '/dashboard/my-mainproject/subprojects/my-subproject/delete/') + '/dashboard/my-mainproject/subprojects/my-subproject/delete/', + ) self.assertEqual(response.status_code, 405) - self.assertTrue(self.subproject in [r.child for r in self.project.subprojects.all()]) + self.assertTrue( + self.subproject in + [r.child for r in self.project.subprojects.all()] + ) # Test POST response = self.client.post( - '/dashboard/my-mainproject/subprojects/my-subproject/delete/') + '/dashboard/my-mainproject/subprojects/my-subproject/delete/', + ) self.assertEqual(response.status_code, 302) - self.assertTrue(self.subproject not in [r.child for r in self.project.subprojects.all()]) - - def test_project_admins_can_delete_subprojects_that_they_are_not_admin_of(self): + self.assertTrue( + self.subproject not in + [r.child for r in self.project.subprojects.all()] + ) + + def test_project_admins_can_delete_subprojects_that_they_are_not_admin_of( + self + ): self.project.users.add(self.user) self.assertFalse(AdminPermission.is_admin(self.user, self.subproject)) response = self.client.post( - '/dashboard/my-mainproject/subprojects/my-subproject/delete/') + '/dashboard/my-mainproject/subprojects/my-subproject/delete/', + ) self.assertEqual(response.status_code, 302) - self.assertTrue(self.subproject not in [r.child for r in self.project.subprojects.all()]) + self.assertTrue( + self.subproject not in + [r.child for r in self.project.subprojects.all()] + ) + + +class BuildViewTests(TestCase): + fixtures = ['eric', 'test_data'] + + def setUp(self): + self.client.login(username='eric', password='test') + + @mock.patch('readthedocs.projects.tasks.update_docs_task') + def test_build_redirect(self, mock): + r = self.client.post('/projects/pip/builds/', {'version_slug': '0.8.1'}) + build = Build.objects.filter(project__slug='pip').latest() + self.assertEqual(r.status_code, 302) + self.assertEqual( + r._headers['location'][1], + '/projects/pip/builds/%s/' % build.pk, + )