-
Notifications
You must be signed in to change notification settings - Fork 87
First pass at fixing tests from Py2->3 upgrade #3288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a1a3b6d
bc961a8
6fa64bf
b386116
d85cd29
b79a409
e6a731e
ed3282c
c011701
0f2c3f0
db54d58
b8a632a
d3b4fb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
|
|
||
| from io import StringIO | ||
| from io import BytesIO | ||
| from json import loads, dumps | ||
| from urllib.parse import urlparse | ||
|
|
||
|
|
@@ -14,6 +14,7 @@ | |
| import datetime | ||
| import psycopg2 | ||
| from unittest.case import skip | ||
| from django_tinsel.exceptions import HttpBadRequestException | ||
|
|
||
| from django.db import connection | ||
| from django.contrib.auth.models import AnonymousUser | ||
|
|
@@ -96,11 +97,11 @@ def send_json_body(url, body_object, client, method, user=None): | |
| are posting form data, so you need to manually setup the parameters | ||
| to override that default functionality. | ||
| """ | ||
| body_string = dumps(body_object) | ||
| body_stream = StringIO(body_string) | ||
| body_binary_string = dumps(body_object).encode() | ||
| body_stream = BytesIO(body_binary_string) | ||
| parsed_url = urlparse(url) | ||
| client_params = { | ||
| 'CONTENT_LENGTH': len(body_string), | ||
| 'CONTENT_LENGTH': len(body_binary_string), | ||
| 'CONTENT_TYPE': 'application/json', | ||
| 'PATH_INFO': _get_path(parsed_url), | ||
| 'QUERY_STRING': parsed_url[4], | ||
|
|
@@ -375,8 +376,8 @@ def test_locations_plots_endpoint_max_plots_param_must_be_a_number(self): | |
| API_PFX, self.instance.url_name)) | ||
| self.assertEqual(response.status_code, 400) | ||
| self.assertEqual(response.content, | ||
| 'The max_plots parameter must be ' | ||
| 'a number between 1 and 500') | ||
| b'The max_plots parameter must be ' | ||
| b'a number between 1 and 500') | ||
|
|
||
| def test_locations_plots_max_plots_param_cannot_be_greater_than_500(self): | ||
| response = get_signed( | ||
|
|
@@ -385,8 +386,8 @@ def test_locations_plots_max_plots_param_cannot_be_greater_than_500(self): | |
| API_PFX, self.instance.url_name)) | ||
| self.assertEqual(response.status_code, 400) | ||
| self.assertEqual(response.content, | ||
| 'The max_plots parameter must be ' | ||
| 'a number between 1 and 500') | ||
| b'The max_plots parameter must be ' | ||
| b'a number between 1 and 500') | ||
| response = get_signed( | ||
| self.client, | ||
| "%s/instance/%s/locations/0,0/plots?max_plots=500" % | ||
|
|
@@ -402,8 +403,8 @@ def test_locations_plots_endpoint_max_plots_param_cannot_be_less_than_1( | |
|
|
||
| self.assertEqual(response.status_code, 400) | ||
| self.assertEqual(response.content, | ||
| 'The max_plots parameter must be a ' | ||
| 'number between 1 and 500') | ||
| b'The max_plots parameter must be a ' | ||
| b'number between 1 and 500') | ||
| response = get_signed( | ||
| self.client, | ||
| "%s/instance/%s/locations/0,0/plots?max_plots=1" % | ||
|
|
@@ -419,7 +420,7 @@ def test_locations_plots_endpoint_distance_param_must_be_a_number(self): | |
|
|
||
| self.assertEqual(response.status_code, 400) | ||
| self.assertEqual(response.content, | ||
| 'The distance parameter must be a number') | ||
| b'The distance parameter must be a number') | ||
|
|
||
| response = get_signed( | ||
| self.client, | ||
|
|
@@ -472,7 +473,7 @@ def test_create_plot_with_tree(self): | |
| data, self.client, self.user) | ||
|
|
||
| self.assertEqual(200, response.status_code, | ||
| "Create failed:" + response.content) | ||
| "Create failed:" + response.content.decode()) | ||
|
|
||
| # Assert that a plot was added | ||
| self.assertEqual(plot_count + 1, Plot.objects.count()) | ||
|
|
@@ -509,7 +510,7 @@ def test_create_plot_with_invalid_tree_returns_400(self): | |
| self.assertEqual(400, | ||
| response.status_code, | ||
| "Expected creating a million foot " | ||
| "tall tree to return 400:" + response.content) | ||
| "tall tree to return 400:" + response.content.decode()) | ||
|
|
||
| body_dict = loads(response.content) | ||
| self.assertTrue('fieldErrors' in body_dict, | ||
|
|
@@ -548,7 +549,7 @@ def test_create_plot_with_geometry(self): | |
| data, self.client, self.user) | ||
|
|
||
| self.assertEqual(200, response.status_code, | ||
| "Create failed:" + response.content) | ||
| "Create failed:" + response.content.decode()) | ||
|
|
||
| # Assert that a plot was added | ||
| self.assertEqual(plot_count + 1, Plot.objects.count()) | ||
|
|
@@ -1334,8 +1335,8 @@ def setUp(self): | |
| 'sort_key': 'Date'} | ||
| ] | ||
| self.instance.save() | ||
| self.instance.logo.save(Instance.test_png_name, | ||
| File(open(Instance.test_png_path, 'r'))) | ||
| with open(Instance.test_png_path, 'rb') as f: | ||
| self.instance.logo.save(Instance.test_png_name, f) | ||
|
|
||
| def test_returns_config_colors(self): | ||
| request = sign_request_as_user(make_request(), self.user) | ||
|
|
@@ -1568,7 +1569,7 @@ def _test_post_photo(self, path): | |
| self.instance.url_name, | ||
| plot_id) | ||
|
|
||
| with open(path) as img: | ||
| with open(path, 'rb') as img: | ||
| req = self.factory.post( | ||
| url, {'name': 'afile', 'file': img}) | ||
|
|
||
|
|
@@ -1621,7 +1622,7 @@ def testUploadPhoto(self): | |
| url = reverse('update_user_photo', kwargs={'version': 3, | ||
| 'user_id': peon.pk}) | ||
|
|
||
| with open(TreePhotoTest.test_jpeg_path) as img: | ||
| with open(TreePhotoTest.test_jpeg_path, 'rb') as img: | ||
| req = self.factory.post( | ||
| url, {'name': 'afile', 'file': img}) | ||
|
|
||
|
|
@@ -1648,7 +1649,7 @@ def testCanOnlyUploadAsSelf(self): | |
| grunt = make_user(username='grunt', password='pw') | ||
| grunt.save() | ||
|
|
||
| with open(TreePhotoTest.test_jpeg_path) as img: | ||
| with open(TreePhotoTest.test_jpeg_path, 'rb') as img: | ||
| req = self.factory.post( | ||
| url, {'name': 'afile', 'file': img}) | ||
|
|
||
|
|
@@ -1883,7 +1884,7 @@ def testTimestampVoidsSignature(self): | |
| acred = APIAccessCredential.create() | ||
| url = ('http://testserver.com/test/blah?' | ||
| 'timestamp=%%s&' | ||
| 'k1=4&k2=a&access_key=%s' % acred.access_key) | ||
| 'k1=4&k2=a&access_key=%s' % acred.access_key.decode()) | ||
|
|
||
| curtime = datetime.datetime.now() | ||
| invalid = curtime - datetime.timedelta(minutes=100) | ||
|
|
@@ -1902,7 +1903,7 @@ def testPOSTBodyChangesSig(self): | |
| url = "%s/i/plots/1/tree/photo" % API_PFX | ||
|
|
||
| def get_sig(path): | ||
| with open(path) as img: | ||
| with open(path, 'rb') as img: | ||
| req = self.factory.post( | ||
| url, {'name': 'afile', 'file': img}) | ||
|
|
||
|
|
@@ -1949,14 +1950,14 @@ def testMalformedTimestamp(self): | |
|
|
||
| url = ('http://testserver.com/test/blah?' | ||
| 'timestamp=%%s&' | ||
| 'k1=4&k2=a&access_key=%s' % acred.access_key) | ||
| 'k1=4&k2=a&access_key=%s' % acred.access_key.decode()) | ||
|
|
||
| req = self.sign_and_send(url % ('%sFAIL' % timestamp), | ||
| acred.secret_key) | ||
|
|
||
| self.assertEqual(req.status_code, 400) | ||
|
|
||
| req = self.sign_and_send(url % timestamp, acred.secret_key) | ||
| req = self.sign_and_send(url % timestamp, acred.secret_key.decode()) | ||
|
|
||
| self.assertRequestWasSuccess(req) | ||
|
|
||
|
|
@@ -1972,7 +1973,7 @@ def testMissingAccessKey(self): | |
|
|
||
| self.assertEqual(req.status_code, 400) | ||
|
|
||
| req = self.sign_and_send('%s&access_key=%s' % (url, acred.access_key), | ||
| req = self.sign_and_send('%s&access_key=%s' % (url, acred.access_key.decode()), | ||
| acred.secret_key) | ||
|
|
||
| self.assertRequestWasSuccess(req) | ||
|
|
@@ -1987,9 +1988,8 @@ def testAuthenticatesAsUser(self): | |
| req = self.sign_and_send('http://testserver.com/test/blah?' | ||
| 'timestamp=%s&' | ||
| 'k1=4&k2=a&access_key=%s' % | ||
| (timestamp, acred.access_key), | ||
| acred.secret_key) | ||
|
|
||
| (timestamp, acred.access_key.decode()), | ||
| acred.secret_key.decode()) | ||
| self.assertEqual(req.user.pk, peon.pk) | ||
|
|
||
|
|
||
|
|
@@ -2003,7 +2003,7 @@ def test_401(self): | |
| self.assertEqual(ret.status_code, 401) | ||
|
|
||
| def test_ok(self): | ||
| auth = base64.b64encode("jim:password") | ||
| auth = base64.b64encode(b"jim:password") | ||
| withauth = {"HTTP_AUTHORIZATION": "Basic %s" % auth} | ||
|
|
||
| ret = get_signed(self.client, "%s/user" % API_PFX, **withauth) | ||
|
|
@@ -2015,14 +2015,14 @@ def test_malformed_auth(self): | |
| ret = get_signed(self.client, "%s/user" % API_PFX, **withauth) | ||
| self.assertEqual(ret.status_code, 401) | ||
|
|
||
| auth = base64.b64encode("foobar") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider updating the commit message to explain why we are removing this base 64 encoding. It is my understanding that basic auth requires base 64.
from https://en.wikipedia.org/wiki/Basic_access_authentication
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks so much for this detail. I took another dive in with this better baseline understanding, and get what is going on 100% now. In py3, there are ripple effect from having to have the initial credentials start as bytes. This complication appears on these lines: https://github.com/OpenTreeMap/otm-core/pull/3288/files#diff-c7aac248bb6ed915db70251295a116a3R96-R103. I pushed up changes to this effect. |
||
| auth = base64.b64encode(b"foobar") | ||
| withauth = {"HTTP_AUTHORIZATION": "Basic %s" % auth} | ||
|
|
||
| ret = get_signed(self.client, "%s/user" % API_PFX, **withauth) | ||
| self.assertEqual(ret.status_code, 401) | ||
|
|
||
| def test_bad_cred(self): | ||
| auth = base64.b64encode("jim:passwordz") | ||
| auth = base64.b64encode(b"jim:passwordz") | ||
| withauth = {"HTTP_AUTHORIZATION": "Basic %s" % auth} | ||
|
|
||
| ret = get_signed(self.client, "%s/user" % API_PFX, **withauth) | ||
|
|
@@ -2034,7 +2034,7 @@ def test_user_has_rep(self): | |
| ijim.reputation = 1001 | ||
| ijim.save() | ||
|
|
||
| auth = base64.b64encode("jim:password") | ||
| auth = base64.b64encode(b"jim:password") | ||
| withauth = dict(list(self.sign.items()) + | ||
| [("HTTP_AUTHORIZATION", "Basic %s" % auth)]) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.