From eb264eedcc15ee7891207b926ae8562dc3c28298 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 12:10:31 -0500 Subject: [PATCH 1/9] Add `type` field to VersionSerializer Since the `type` wasn't in the response when syncing version and retrieving this data the `APIVersion.type` was returning `UNKNOWN` and then the `APIVersion.commit_name` was incorrect. By adding the `type` to the API response the `Version` it's completed. https://github.com/rtfd/readthedocs.org/issues/3203 --- readthedocs/restapi/serializers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 63616ea19d8..7aac3dd18d2 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -73,6 +73,7 @@ class Meta(object): 'identifier', 'verbose_name', 'active', 'built', 'downloads', + 'type', ) From b982bf70515d7859d398b7da3b8d9f20942d09ca Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 12:36:58 -0500 Subject: [PATCH 2/9] Test case to check API Version detail response --- readthedocs/rtd_tests/tests/test_api.py | 56 +++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 81d7b05eaaa..f151a45846d 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -8,6 +8,7 @@ from builtins import str from django.test import TestCase from django.contrib.auth.models import User +from django.core.urlresolvers import reverse from django_dynamic_fixture import get from rest_framework import status from rest_framework.test import APIClient @@ -508,3 +509,58 @@ def test_generic_api_falls_back_to_token_auth(self, trigger_build): ) self.assertEqual(resp.status_code, 200) self.assertTrue(resp.data['build_triggered']) + + +class APIVersionTests(TestCase): + fixtures = ['eric', 'test_data'] + + def test_get_version_by_id(self): + pip = Project.objects.get(slug='pip') + version = pip.versions.get(slug='0.8') + + data = { + 'pk': version.pk, + } + resp = self.client.get( + reverse('version-detail', kwargs=data), + content_type='application/json', + HTTP_AUTHORIZATION='Basic %s' % eric_auth, + ) + self.assertEqual(resp.status_code, 200) + + version_data = { + 'type': 'tag', 'verbose_name': '0.8', 'built': False, 'id': 18, 'active': True, + 'project': { + 'analytics_code': None, + 'canonical_url': 'http://readthedocs.org/docs/pip/en/latest/', + 'cdn_enabled': False, + 'conf_py_file': '', + 'container_image': None, + 'container_mem_limit': None, + 'container_time_limit': None, + 'default_branch': None, + 'default_version': 'latest', + 'description': '', + 'documentation_type': 'sphinx', + 'enable_epub_build': True, + 'enable_pdf_build': True, + 'features': ['allow_deprecated_webhooks'], + 'id': 6, + 'install_project': False, + 'language': 'en', + 'name': 'Pip', + 'python_interpreter': 'python', + 'repo': 'https://github.com/pypa/pip', + 'repo_type': 'git', + 'requirements_file': None, + 'skip': False, + 'slug': 'pip', + 'suffix': '.rst', + 'use_system_packages': False, + 'users': [1]}, 'downloads': {}, 'identifier': '2404a34eba4ee9c48cc8bc4055b99a48354f4950', 'slug': '0.8' + } + + self.assertDictEqual( + resp.data, + version_data, + ) From a15cc81dd70e728aab4c794d593423c39a9e9110 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 13:15:53 -0500 Subject: [PATCH 3/9] Fix yapf for pre-commit to make effect --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2303dd3a139..f37e23c1a35 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,7 +35,7 @@ repos: - id: yapf exclude: 'migrations|settings|scripts' additional_dependencies: ['futures'] - args: ['--style=.style.yapf', '--parallel'] + args: ['--style=.style.yapf', '--parallel', '--in-place'] - repo: git@github.com:FalconSocial/pre-commit-python-sorter.git sha: b57843b0b874df1d16eb0bef00b868792cb245c2 From 36b7755f52cc9c2447adc95c721025394cedc85f Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 13:16:14 -0500 Subject: [PATCH 4/9] Update docformatter for pre-commit to proper version --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f37e23c1a35..732826af272 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,7 +44,7 @@ repos: args: ['--silent-overwrite'] - repo: git@github.com:humitos/mirrors-docformatter.git - sha: v0.8 + sha: v0.0.1 hooks: - id: docformatter args: ['--in-place', '--wrap-summaries=80', '--wrap-descriptions=80', '--pre-summary-newline', '--no-blank'] From 234356baa49510234a54074478932f788623325a Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 13:16:28 -0500 Subject: [PATCH 5/9] Ignore D403 since doesn't work for all the cases --- .flake8 | 2 +- prospector.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.flake8 b/.flake8 index 319e43e8d6d..8ff3a7c121f 100644 --- a/.flake8 +++ b/.flake8 @@ -1,3 +1,3 @@ [flake8] -ignore = E125,D100,D101,D102,D103,D105,D106,D107,D200,D202,D211,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54,MQ101,T000 +ignore = E125,D100,D101,D102,D103,D105,D106,D107,D200,D202,D211,D403,P101,FI15,FI16,FI12,FI11,FI17,FI50,FI53,FI54,MQ101,T000 max-line-length = 80 diff --git a/prospector.yml b/prospector.yml index f6101a32273..c19d23b9a10 100644 --- a/prospector.yml +++ b/prospector.yml @@ -40,6 +40,7 @@ pep257: - D106 # Missing docstring in public nested class # pydocstyle + - D403 # First word of the first line should be properly capitalized ('Github', not 'GitHub') - D406 # Section name should end with a newline ('Examples', not 'Examples::') - D407 # Missing dashed underline after section ('Examples') - D412 # No blank lines allowed between a section header and its content ('Examples') From 87fed4057037715e82561b5e5d414ad5ee5aa261 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 13:16:46 -0500 Subject: [PATCH 6/9] Autolint --- readthedocs/rtd_tests/tests/test_api.py | 277 ++++++++++++------------ 1 file changed, 143 insertions(+), 134 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index f151a45846d..3689f103a92 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -1,24 +1,25 @@ -from __future__ import absolute_import +# -*- coding: utf-8 -*- +from __future__ import ( + absolute_import, division, print_function, unicode_literals) -import json import base64 import datetime +import json +from builtins import str import mock -from builtins import str -from django.test import TestCase +from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django.core.urlresolvers import reverse +from django.test import TestCase from django_dynamic_fixture import get from rest_framework import status from rest_framework.test import APIClient -from allauth.socialaccount.models import SocialAccount from readthedocs.builds.models import Build, Version from readthedocs.integrations.models import Integration -from readthedocs.projects.models import Project, Feature -from readthedocs.oauth.models import RemoteRepository, RemoteOrganization - +from readthedocs.oauth.models import RemoteOrganization, RemoteRepository +from readthedocs.projects.models import Feature, Project super_auth = base64.b64encode(b'super:test').decode('utf-8') eric_auth = base64.b64encode(b'eric:test').decode('utf-8') @@ -28,9 +29,7 @@ class APIBuildTests(TestCase): fixtures = ['eric.json', 'test_data.json'] def test_make_build(self): - """ - Test that a superuser can use the API - """ + """Test that a superuser can use the API.""" client = APIClient() client.login(username='super', password='test') resp = client.post( @@ -43,7 +42,8 @@ def test_make_build(self): 'error': 'Test Error', 'state': 'cloning', }, - format='json') + format='json', + ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) build = resp.data self.assertEqual(build['state_display'], 'Cloning') @@ -55,7 +55,7 @@ def test_make_build(self): self.assertEqual(build['state_display'], 'Cloning') def test_make_build_without_permission(self): - """Ensure anonymous/non-staff users cannot write the build endpoint""" + """Ensure anonymous/non-staff users cannot write the build endpoint.""" client = APIClient() def _try_post(): @@ -68,7 +68,8 @@ def _try_post(): 'output': 'Test Output', 'error': 'Test Error', }, - format='json') + format='json', + ) self.assertEqual(resp.status_code, 403) _try_post() @@ -79,7 +80,7 @@ def _try_post(): _try_post() def test_update_build_without_permission(self): - """Ensure anonymous/non-staff users cannot update build endpoints""" + """Ensure anonymous/non-staff users cannot update build endpoints.""" client = APIClient() api_user = get(User, staff=False, password='test') client.force_authenticate(user=api_user) @@ -89,13 +90,15 @@ def test_update_build_without_permission(self): { 'project': 1, 'version': 1, - 'state': 'finished' + 'state': 'finished', }, - format='json') + format='json', + ) self.assertEqual(resp.status_code, 403) def test_make_build_protected_fields(self): - """Ensure build api view delegates correct serializer + """ + Ensure build api view delegates correct serializer. Super users should be able to read/write the `builder` property, but we don't expose this to end users via the API @@ -114,7 +117,7 @@ def test_make_build_protected_fields(self): self.assertIn('builder', resp.data) def test_make_build_commands(self): - """Create build and build commands""" + """Create build and build commands.""" client = APIClient() client.login(username='super', password='test') resp = client.post( @@ -124,7 +127,8 @@ def test_make_build_commands(self): 'version': 1, 'success': True, }, - format='json') + format='json', + ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) build = resp.data now = datetime.datetime.utcnow() @@ -138,7 +142,8 @@ def test_make_build_commands(self): 'start_time': str(now - datetime.timedelta(seconds=5)), 'end_time': str(now), }, - format='json') + format='json', + ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) resp = client.get('/api/v2/build/%s/' % build['id']) self.assertEqual(resp.status_code, 200) @@ -152,20 +157,24 @@ class APITests(TestCase): fixtures = ['eric.json', 'test_data.json'] def test_make_project(self): - """ - Test that a superuser can use the API - """ - post_data = {"name": "awesome-project", - "repo": "https://github.com/ericholscher/django-kong.git"} - resp = self.client.post('/api/v1/project/', - data=json.dumps(post_data), - content_type='application/json', - HTTP_AUTHORIZATION=u'Basic %s' % super_auth) + """Test that a superuser can use the API.""" + post_data = { + 'name': 'awesome-project', + 'repo': 'https://github.com/ericholscher/django-kong.git', + } + resp = self.client.post( + '/api/v1/project/', + data=json.dumps(post_data), + content_type='application/json', + HTTP_AUTHORIZATION='Basic %s' % super_auth, + ) self.assertEqual(resp.status_code, 201) - self.assertEqual(resp['location'], - '/api/v1/project/24/') - resp = self.client.get('/api/v1/project/24/', data={'format': 'json'}, - HTTP_AUTHORIZATION=u'Basic %s' % eric_auth) + self.assertEqual(resp['location'], '/api/v1/project/24/') + resp = self.client.get( + '/api/v1/project/24/', + data={'format': 'json'}, + HTTP_AUTHORIZATION='Basic %s' % eric_auth, + ) self.assertEqual(resp.status_code, 200) obj = json.loads(resp.content) self.assertEqual(obj['slug'], 'awesome-project') @@ -188,44 +197,41 @@ def test_user_doesnt_get_full_api_return(self): self.assertEqual(resp.data['conf_py_file'], 'foo') def test_invalid_make_project(self): - """ - Test that the authentication is turned on. - """ - post_data = {"user": "/api/v1/user/2/", - "name": "awesome-project-2", - "repo": "https://github.com/ericholscher/django-bob.git" - } + """Test that the authentication is turned on.""" + post_data = { + 'user': '/api/v1/user/2/', + 'name': 'awesome-project-2', + 'repo': 'https://github.com/ericholscher/django-bob.git', + } resp = self.client.post( - '/api/v1/project/', data=json.dumps(post_data), + '/api/v1/project/', + data=json.dumps(post_data), content_type='application/json', - HTTP_AUTHORIZATION=u'Basic %s' % base64.b64encode(b'tester:notapass').decode('utf-8') + HTTP_AUTHORIZATION='Basic %s' % + base64.b64encode(b'tester:notapass').decode('utf-8'), ) self.assertEqual(resp.status_code, 401) def test_make_project_dishonest_user(self): - """ - Test that you can't create a project for another user - """ + """Test that you can't create a project for another user.""" # represents dishonest data input, authentication happens for user 2 post_data = { - "users": ["/api/v1/user/1/"], - "name": "awesome-project-2", - "repo": "https://github.com/ericholscher/django-bob.git" + 'users': ['/api/v1/user/1/'], + 'name': 'awesome-project-2', + 'repo': 'https://github.com/ericholscher/django-bob.git', } resp = self.client.post( '/api/v1/project/', data=json.dumps(post_data), content_type='application/json', - HTTP_AUTHORIZATION=u'Basic %s' % base64.b64encode(b'tester:test').decode('utf-8') + HTTP_AUTHORIZATION='Basic %s' % + base64.b64encode(b'tester:test').decode('utf-8'), ) self.assertEqual(resp.status_code, 401) def test_ensure_get_unauth(self): - """ - Test that GET requests work without authenticating. - """ - - resp = self.client.get("/api/v1/project/", data={"format": "json"}) + """Test that GET requests work without authenticating.""" + resp = self.client.get('/api/v1/project/', data={'format': 'json'}) self.assertEqual(resp.status_code, 200) def test_project_features(self): @@ -234,7 +240,7 @@ def test_project_features(self): # One explicit, one implicit feature feature1 = get(Feature, projects=[project]) feature2 = get(Feature, projects=[], default_true=True) - feature3 = get(Feature, projects=[], default_true=False) + feature3 = get(Feature, projects=[], default_true=False) # noqa client = APIClient() client.force_authenticate(user=user) @@ -243,7 +249,7 @@ def test_project_features(self): self.assertIn('features', resp.data) self.assertEqual( resp.data['features'], - [feature1.feature_id, feature2.feature_id] + [feature1.feature_id, feature2.feature_id], ) def test_project_features_multiple_projects(self): @@ -257,20 +263,17 @@ def test_project_features_multiple_projects(self): resp = client.get('/api/v2/project/%s/' % (project1.pk)) self.assertEqual(resp.status_code, 200) self.assertIn('features', resp.data) - self.assertEqual( - resp.data['features'], - [feature.feature_id] - ) + self.assertEqual(resp.data['features'], [feature.feature_id]) class APIImportTests(TestCase): - """Import API endpoint tests""" + """Import API endpoint tests.""" fixtures = ['eric.json', 'test_data.json'] def test_permissions(self): - """Ensure user repositories aren't leaked to other users""" + """Ensure user repositories aren't leaked to other users.""" client = APIClient() account_a = get(SocialAccount, provider='github') @@ -280,33 +283,35 @@ def test_permissions(self): user_b = get(User, password='test', socialaccount_set=[account_b]) user_c = get(User, password='test', socialaccount_set=[account_c]) org_a = get(RemoteOrganization, users=[user_a], account=account_a) - repo_a = get(RemoteRepository, users=[user_a], organization=org_a, - account=account_a) - repo_b = get(RemoteRepository, users=[user_b], organization=None, - account=account_b) + repo_a = get( + RemoteRepository, + users=[user_a], + organization=org_a, + account=account_a, + ) + repo_b = get( + RemoteRepository, + users=[user_b], + organization=None, + account=account_b, + ) client.force_authenticate(user=user_a) - resp = client.get( - '/api/v2/remote/repo/', - format='json') + resp = client.get('/api/v2/remote/repo/', format='json') self.assertEqual(resp.status_code, status.HTTP_200_OK) repos = resp.data['results'] self.assertEqual(repos[0]['id'], repo_a.id) self.assertEqual(repos[0]['organization']['id'], org_a.id) self.assertEqual(len(repos), 1) - resp = client.get( - '/api/v2/remote/org/', - format='json') + resp = client.get('/api/v2/remote/org/', format='json') self.assertEqual(resp.status_code, status.HTTP_200_OK) orgs = resp.data['results'] self.assertEqual(orgs[0]['id'], org_a.id) self.assertEqual(len(orgs), 1) client.force_authenticate(user=user_b) - resp = client.get( - '/api/v2/remote/repo/', - format='json') + resp = client.get('/api/v2/remote/repo/', format='json') self.assertEqual(resp.status_code, status.HTTP_200_OK) repos = resp.data['results'] self.assertEqual(repos[0]['id'], repo_b.id) @@ -314,9 +319,7 @@ def test_permissions(self): self.assertEqual(len(repos), 1) client.force_authenticate(user=user_c) - resp = client.get( - '/api/v2/remote/repo/', - format='json') + resp = client.get('/api/v2/remote/repo/', format='json') self.assertEqual(resp.status_code, status.HTTP_200_OK) repos = resp.data['results'] self.assertEqual(len(repos), 0) @@ -325,7 +328,7 @@ def test_permissions(self): @mock.patch('readthedocs.core.views.hooks.trigger_build') class IntegrationsTests(TestCase): - """Integration for webhooks, etc""" + """Integration for webhooks, etc.""" fixtures = ['eric.json', 'test_data.json'] @@ -334,27 +337,25 @@ def setUp(self): self.version = get(Version, verbose_name='master', project=self.project) def test_github_webhook(self, trigger_build): - """GitHub webhook API""" + """GitHub webhook API.""" client = APIClient() - resp = client.post( + client.post( '/api/v2/webhook/github/{0}/'.format(self.project.slug), {'ref': 'master'}, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) - resp = client.post( + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) + client.post( '/api/v2/webhook/github/{0}/'.format(self.project.slug), {'ref': 'non-existent'}, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) def test_github_invalid_webhook(self, trigger_build): - """GitHub webhook unhandled event""" + """GitHub webhook unhandled event.""" client = APIClient() resp = client.post( '/api/v2/webhook/github/{0}/'.format(self.project.slug), @@ -366,27 +367,25 @@ def test_github_invalid_webhook(self, trigger_build): self.assertEqual(resp.data['detail'], 'Unhandled webhook event') def test_gitlab_webhook(self, trigger_build): - """GitLab webhook API""" + """GitLab webhook API.""" client = APIClient() - resp = client.post( + client.post( '/api/v2/webhook/gitlab/{0}/'.format(self.project.slug), {'object_kind': 'push', 'ref': 'master'}, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) - resp = client.post( + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) + client.post( '/api/v2/webhook/gitlab/{0}/'.format(self.project.slug), {'object_kind': 'push', 'ref': 'non-existent'}, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) def test_gitlab_invalid_webhook(self, trigger_build): - """GitLab webhook unhandled event""" + """GitLab webhook unhandled event.""" client = APIClient() resp = client.post( '/api/v2/webhook/gitlab/{0}/'.format(self.project.slug), @@ -397,50 +396,45 @@ def test_gitlab_invalid_webhook(self, trigger_build): self.assertEqual(resp.data['detail'], 'Unhandled webhook event') def test_bitbucket_webhook(self, trigger_build): - """Bitbucket webhook API""" + """Bitbucket webhook API.""" client = APIClient() - resp = client.post( + client.post( '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), { 'push': { 'changes': [{ 'new': { - 'name': 'master' - } - }] - } + 'name': 'master', + }, + }], + }, }, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) - resp = client.post( + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) + client.post( '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), { 'push': { - 'changes': [{ - 'new': { - 'name': 'non-existent' - } - }] - } + 'changes': [ + { + 'new': {'name': 'non-existent'}, + }, + ], + }, }, format='json', ) - trigger_build.assert_has_calls([ - mock.call(force=True, version=mock.ANY, project=self.project) - ]) + trigger_build.assert_has_calls( + [mock.call(force=True, version=mock.ANY, project=self.project)]) def test_bitbucket_invalid_webhook(self, trigger_build): - """Bitbucket webhook unhandled event""" + """Bitbucket webhook unhandled event.""" client = APIClient() resp = client.post( '/api/v2/webhook/bitbucket/{0}/'.format(self.project.slug), - {'foo': 'bar'}, - format='json', - HTTP_X_EVENT_KEY='pull_request' - ) + {'foo': 'bar'}, format='json', HTTP_X_EVENT_KEY='pull_request') self.assertEqual(resp.status_code, 200) self.assertEqual(resp.data['detail'], 'Unhandled webhook event') @@ -454,18 +448,21 @@ def test_generic_api_fails_without_auth(self, trigger_build): self.assertEqual(resp.status_code, 403) self.assertEqual( resp.data['detail'], - 'Authentication credentials were not provided.' + 'Authentication credentials were not provided.', ) def test_generic_api_respects_token_auth(self, trigger_build): client = APIClient() integration = Integration.objects.create( project=self.project, - integration_type=Integration.API_WEBHOOK + integration_type=Integration.API_WEBHOOK, ) self.assertIsNotNone(integration.token) resp = client.post( - '/api/v2/webhook/{0}/{1}/'.format(self.project.slug, integration.pk), + '/api/v2/webhook/{0}/{1}/'.format( + self.project.slug, + integration.pk, + ), {'token': integration.token}, format='json', ) @@ -473,7 +470,10 @@ def test_generic_api_respects_token_auth(self, trigger_build): self.assertTrue(resp.data['build_triggered']) # Test nonexistent branch resp = client.post( - '/api/v2/webhook/{0}/{1}/'.format(self.project.slug, integration.pk), + '/api/v2/webhook/{0}/{1}/'.format( + self.project.slug, + integration.pk, + ), {'token': integration.token, 'branches': 'nonexistent'}, format='json', ) @@ -498,12 +498,13 @@ def test_generic_api_falls_back_to_token_auth(self, trigger_build): user = get(User) client.force_authenticate(user=user) integration = Integration.objects.create( - project=self.project, - integration_type=Integration.API_WEBHOOK - ) + project=self.project, integration_type=Integration.API_WEBHOOK) self.assertIsNotNone(integration.token) resp = client.post( - '/api/v2/webhook/{0}/{1}/'.format(self.project.slug, integration.pk), + '/api/v2/webhook/{0}/{1}/'.format( + self.project.slug, + integration.pk, + ), {'token': integration.token}, format='json', ) @@ -529,7 +530,11 @@ def test_get_version_by_id(self): self.assertEqual(resp.status_code, 200) version_data = { - 'type': 'tag', 'verbose_name': '0.8', 'built': False, 'id': 18, 'active': True, + 'type': 'tag', + 'verbose_name': '0.8', + 'built': False, + 'id': 18, + 'active': True, 'project': { 'analytics_code': None, 'canonical_url': 'http://readthedocs.org/docs/pip/en/latest/', @@ -557,7 +562,11 @@ def test_get_version_by_id(self): 'slug': 'pip', 'suffix': '.rst', 'use_system_packages': False, - 'users': [1]}, 'downloads': {}, 'identifier': '2404a34eba4ee9c48cc8bc4055b99a48354f4950', 'slug': '0.8' + 'users': [1], + }, + 'downloads': {}, + 'identifier': '2404a34eba4ee9c48cc8bc4055b99a48354f4950', + 'slug': '0.8', } self.assertDictEqual( From e08dda44a4e00eead6bf86e3bb0cd11fd387aa39 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 15:29:39 -0500 Subject: [PATCH 7/9] Add log.error message to find out what are the causes of this --- readthedocs/builds/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index e960e430e72..e97f6002bbd 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -146,6 +146,7 @@ def commit_name(self): # If we came that far it's not a special version nor a branch or tag. # Therefore just return the identifier to make a safe guess. + log.error('TODO: Raise an exception here. Testing what cases it happens') return self.identifier def get_absolute_url(self): From 1b49e591bc53ad22db11e08c800aafcb85652786 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 15:30:06 -0500 Subject: [PATCH 8/9] Style --- readthedocs/rtd_tests/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 3689f103a92..69c8a86ae3b 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -525,7 +525,7 @@ def test_get_version_by_id(self): resp = self.client.get( reverse('version-detail', kwargs=data), content_type='application/json', - HTTP_AUTHORIZATION='Basic %s' % eric_auth, + HTTP_AUTHORIZATION='Basic {}'.format(eric_auth), ) self.assertEqual(resp.status_code, 200) From 2508f1b065b521697af10c5a1635ef5eed74c741 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 20 Dec 2017 15:32:16 -0500 Subject: [PATCH 9/9] Comment explaining why we are checking the full response in the test --- readthedocs/rtd_tests/tests/test_api.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 69c8a86ae3b..75e2a7e1f34 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -516,6 +516,12 @@ class APIVersionTests(TestCase): fixtures = ['eric', 'test_data'] def test_get_version_by_id(self): + """ + Test the full response of ``/api/v2/version/{pk}`` is what we expects. + + Allows us to notice changes in the fields returned by the endpoint + instead of let them pass silently. + """ pip = Project.objects.get(slug='pip') version = pip.versions.get(slug='0.8')