From f259a911c7c7de1857d713227275d3c97ad6517c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 18 Sep 2019 17:25:58 -0400 Subject: [PATCH 01/18] BOM-618 Decode content we get from test requests. --- common/djangoapps/student/tests/test_email.py | 4 ++-- .../third_party_auth/tests/specs/base.py | 2 +- .../verify_student/tests/test_views.py | 20 +++++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 25d6bc8265..76a9848b51 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -215,7 +215,7 @@ class ReactivationEmailTests(EmailTestMixin, CacheIsolationTestCase): Send the reactivation email to the specified user, and return the response as json data. """ - return json.loads(send_reactivation_email_for_user(user).content) + return json.loads(send_reactivation_email_for_user(user).content.decode('utf-8')) def assertReactivateEmailSent(self, email_user): """ @@ -480,7 +480,7 @@ class EmailChangeConfirmationTests(EmailTestMixin, EmailTemplateTagMixin, CacheI self.assertEqual(response.status_code, 200) self.assertEquals( mock_render_to_response(expected_template, expected_context).content, - response.content + response.content.decode('utf-8') ) def assertChangeEmailSent(self, test_body_type): diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index a85eadcf36..c8c9d6bf86 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -935,7 +935,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): strategy.request.POST['password'] = 'bad_' + password if success is False else password self.assert_pipeline_running(strategy.request) - payload = json.loads(login_user(strategy.request).content) + payload = json.loads(login_user(strategy.request).content.decode('utf-8')) if success is None: # Request malformed -- just one of email/password given. diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 4e51ad86af..98b4b1f4dd 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -1340,7 +1340,7 @@ class TestCreateOrderView(ModuleStoreTestCase): @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_invalid_amount(self): response = self._create_order('1.a', self.course_id, expect_status_code=400) - self.assertIn('Selected price is not valid number.', response.content) + self.assertIn('Selected price is not valid number.', response.content.decode('utf-8')) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_invalid_mode(self): @@ -1348,7 +1348,7 @@ class TestCreateOrderView(ModuleStoreTestCase): course_id = 'Fake/999/Test_Course' CourseFactory.create(org='Fake', number='999', display_name='Test Course') response = self._create_order('50', course_id, expect_status_code=400) - self.assertIn('This course doesn\'t support paid certificates', response.content) + self.assertIn('This course doesn\'t support paid certificates', response.content.decode('utf-8')) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_create_order_fail_with_get(self): @@ -1672,7 +1672,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): HTTP_AUTHORIZATION='test BBBBBBBBBBBBBBBBBBBB: testing', HTTP_DATE='testdate' ) - self.assertIn('Invalid JSON', response.content) + self.assertIn('Invalid JSON', response.content.decode('utf-8')) self.assertEqual(response.status_code, 400) def test_invalid_dict(self): @@ -1687,7 +1687,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): HTTP_AUTHORIZATION='test BBBBBBBBBBBBBBBBBBBB:testing', HTTP_DATE='testdate' ) - self.assertIn('JSON should be dict', response.content) + self.assertIn('JSON should be dict', response.content.decode('utf-8')) self.assertEqual(response.status_code, 400) @patch( @@ -1712,7 +1712,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): HTTP_AUTHORIZATION='test testing:testing', HTTP_DATE='testdate' ) - self.assertIn('Access key invalid', response.content) + self.assertIn('Access key invalid', response.content.decode('utf-8')) self.assertEqual(response.status_code, 400) @patch( @@ -1737,7 +1737,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): HTTP_AUTHORIZATION='test BBBBBBBBBBBBBBBBBBBB:testing', HTTP_DATE='testdate' ) - self.assertIn('edX ID Invalid-Id not found', response.content) + self.assertIn('edX ID Invalid-Id not found', response.content.decode('utf-8')) self.assertEqual(response.status_code, 400) @patch( @@ -1780,7 +1780,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): self.assertEqual(attempt.expiry_date.date(), expiry_date.date()) self.assertIsNone(old_verification.expiry_date) self.assertIsNone(old_verification.expiry_email_date) - self.assertEquals(response.content, 'OK!') + self.assertEquals(response.content.decode('utf-8'), 'OK!') self.assertEqual(len(mail.outbox), 1) @patch( @@ -1814,7 +1814,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): attempt = SoftwareSecurePhotoVerification.objects.get(receipt_id=self.receipt_id) self.assertEqual(attempt.status, u'approved') self.assertEqual(attempt.expiry_date.date(), expiry_date.date()) - self.assertEquals(response.content, 'OK!') + self.assertEquals(response.content.decode('utf-8'), 'OK!') self.assertEqual(len(mail.outbox), 1) @patch( @@ -1872,7 +1872,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): self.assertEqual(attempt.status, u'must_retry') self.assertEqual(attempt.error_code, u'You must retry the verification.') self.assertEqual(attempt.error_msg, u'"Memory overflow"') - self.assertEquals(response.content, 'OK!') + self.assertEquals(response.content.decode('utf-8'), 'OK!') @patch( 'lms.djangoapps.verify_student.ssencrypt.has_valid_signature', @@ -1896,7 +1896,7 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): HTTP_AUTHORIZATION='test BBBBBBBBBBBBBBBBBBBB:testing', HTTP_DATE='testdate' ) - self.assertIn('Result Unknown not understood', response.content) + self.assertIn('Result Unknown not understood', response.content.decode('utf-8')) class TestReverifyView(TestCase): From 835a84f33c3f78fe6050f7a48bde31e9409b43e6 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 18 Sep 2019 17:38:55 -0400 Subject: [PATCH 02/18] Use bytestrings to create SimpleUploadedFiles. BOM-616 --- .../third_party_auth/tests/test_admin.py | 2 +- .../certificates/tests/test_models.py | 6 +-- lms/djangoapps/instructor/tests/test_api.py | 48 +++++++++---------- .../instructor/tests/test_certificates.py | 18 +++---- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_admin.py b/common/djangoapps/third_party_auth/tests/test_admin.py index 719dc0d0e6..dd5985da44 100644 --- a/common/djangoapps/third_party_auth/tests/test_admin.py +++ b/common/djangoapps/third_party_auth/tests/test_admin.py @@ -50,7 +50,7 @@ class Oauth2ProviderConfigAdminTest(testutil.TestCase): provider1 = self.configure_dummy_provider( enabled=True, icon_class='', - icon_image=SimpleUploadedFile('icon.svg', ''), + icon_image=SimpleUploadedFile('icon.svg', b''), ) # Get the provider instance with active flag diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index cecd3d1cae..58ef5b4871 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -180,17 +180,17 @@ class CertificateTemplateAssetTest(TestCase): """ CertificateTemplateAsset(description='test description', asset=SimpleUploadedFile( 'picture1.jpg', - 'these are the file contents!')).save() + b'these are the file contents!')).save() certificate_template_asset = CertificateTemplateAsset.objects.get(id=1) self.assertEqual(certificate_template_asset.asset, 'certificate_template_assets/1/picture1.jpg') # Now save asset with same file again, New file will be uploaded after deleting the old one with the same name. - certificate_template_asset.asset = SimpleUploadedFile('picture1.jpg', 'file contents') + certificate_template_asset.asset = SimpleUploadedFile('picture1.jpg', b'file contents') certificate_template_asset.save() self.assertEqual(certificate_template_asset.asset, 'certificate_template_assets/1/picture1.jpg') # Now replace the asset with another file - certificate_template_asset.asset = SimpleUploadedFile('picture2.jpg', 'file contents') + certificate_template_asset.asset = SimpleUploadedFile('picture2.jpg', b'file contents') certificate_template_asset.save() certificate_template_asset = CertificateTemplateAsset.objects.get(id=1) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index c12da705ff..69c5669ad6 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -675,7 +675,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas """ Happy path test to create a single new user """ - csv_content = "test_student@example.com,test_student_1,tester1,USA" + csv_content = b"test_student@example.com,test_student_1,tester1,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) self.assertEqual(response.status_code, 200) @@ -718,8 +718,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas If the email address and username already exists and the user is enrolled in the course, do nothing (including no email gets sent out) """ - csv_content = "test_student@example.com,test_student_1,tester1,USA\n" \ - "test_student@example.com,test_student_1,tester2,US" + csv_content = b"test_student@example.com,test_student_1,tester1,USA\n" \ + b"test_student@example.com,test_student_1,tester2,US" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) self.assertEqual(response.status_code, 200) @@ -771,7 +771,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas """ Try uploading a CSV file which does not have the exact four columns of data """ - csv_content = "test_student@example.com,test_student_1\n" + csv_content = b"test_student@example.com,test_student_1\n" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) self.assertEqual(response.status_code, 200) @@ -788,8 +788,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas """ Test failure case of a poorly formatted email field """ - csv_content = "test_student.example.com,test_student_1,tester1,USA" - + csv_content = b"test_student.example.com,test_student_1,tester1,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) data = json.loads(response.content.decode('utf-8')) @@ -808,8 +807,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas If the email address and username already exists and the user is not enrolled in the course, enrolled him/her and iterate to next one. """ - csv_content = "nonenrolled@test.com,NotEnrolledStudent,tester1,USA" - + csv_content = b"nonenrolled@test.com,NotEnrolledStudent,tester1,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) self.assertEqual(response.status_code, 200) @@ -827,8 +825,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas If the email address already exists, but the username is different, assume it is the correct user and just register the user in the course. """ - csv_content = "test_student@example.com,test_student_1,tester1,USA\n" \ - "test_student@example.com,test_student_2,tester2,US" + csv_content = b"test_student@example.com,test_student_1,tester1,USA\n" \ + b"test_student@example.com,test_student_2,tester2,US" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) @@ -862,7 +860,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas user.is_active = False user.save() - csv_content = "{email},{username},tester,USA".format(email=conflicting_email, username='new_test_student') + csv_content = b"{email},{username},tester,USA".format(email=conflicting_email, username='new_test_student') uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) @@ -880,8 +878,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas If the username already exists (but not the email), assume it is a different user and fail to create the new account. """ - csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ - "test_student2@example.com,test_student_1,tester2,US" + csv_content = b"test_student1@example.com,test_student_1,tester1,USA\n" \ + b"test_student2@example.com,test_student_1,tester2,US" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) @@ -895,8 +893,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas """ Test when the user does not attach a file """ - csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ - "test_student2@example.com,test_student_1,tester2,US" + csv_content = b"test_student1@example.com,test_student_1,tester1,USA\n" \ + b"test_student2@example.com,test_student_1,tester2,US" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) @@ -913,8 +911,8 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas """ Test that exceptions are handled well """ - csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ - "test_student2@example.com,test_student_1,tester2,US" + csv_content = b"test_student1@example.com,test_student_1,tester1,USA\n" \ + b"test_student2@example.com,test_student_1,tester2,US" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) with patch('lms.djangoapps.instructor.views.api.create_manual_course_enrollment') as mock: @@ -947,10 +945,10 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas user.is_active = False user.save() - csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ - "test_student3@example.com,test_student_1,tester3,CA\n" \ - "test_student4@example.com,test_student_4,tester4,USA\n" \ - "test_student2@example.com,test_student_2,tester2,USA" + csv_content = b"test_student1@example.com,test_student_1,tester1,USA\n" \ + b"test_student3@example.com,test_student_1,tester3,CA\n" \ + b"test_student4@example.com,test_student_4,tester4,USA\n" \ + b"test_student2@example.com,test_student_2,tester2,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) @@ -985,7 +983,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas @patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': False}) def test_allow_automated_signups_flag_not_set(self): - csv_content = "test_student1@example.com,test_student_1,tester1,USA" + csv_content = b"test_student1@example.com,test_student_1,tester1,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.url, {'students_list': uploaded_file}) self.assertEquals(response.status_code, 403) @@ -1001,7 +999,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas # Login Audit Course instructor self.client.login(username=self.audit_course_instructor.username, password='test') - csv_content = "test_student_wl@example.com,test_student_wl,Test Student,USA" + csv_content = b"test_student_wl@example.com,test_student_wl,Test Student,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.audit_course_url, {'students_list': uploaded_file}) @@ -1032,7 +1030,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas # Login Audit Course instructor self.client.login(username=self.white_label_course_instructor.username, password='test') - csv_content = "test_student_wl@example.com,test_student_wl,Test Student,USA" + csv_content = b"test_student_wl@example.com,test_student_wl,Test Student,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.white_label_course_url, {'students_list': uploaded_file}) @@ -1058,7 +1056,7 @@ class TestInstructorAPIBulkAccountCreationAndEnrollment(SharedModuleStoreTestCas # Login white label course instructor self.client.login(username=self.white_label_course_instructor.username, password='test') - csv_content = "test_student_wl@example.com,test_student_wl,Test Student,USA" + csv_content = b"test_student_wl@example.com,test_student_wl,Test Student,USA" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) response = self.client.post(self.white_label_course_url, {'students_list': uploaded_file}) diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 96c8118bb6..6d3104823d 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -928,8 +928,8 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest """ Happy path test to create a single new white listed record """ - csv_content = "test_student1@example.com,dummy_notes\n" \ - "test_student2@example.com,dummy_notes" + csv_content = b"test_student1@example.com,dummy_notes\n" \ + b"test_student2@example.com,dummy_notes" data = self.upload_file(csv_content=csv_content) self.assertEquals(len(data['general_errors']), 0) self.assertEquals(len(data['row_errors']['data_format_error']), 0) @@ -943,8 +943,8 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest """ Try uploading a CSV file with invalid data formats and verify the errors. """ - csv_content = "test_student1@example.com,test,1,USA\n" \ - "test_student2@example.com,test,1" + csv_content = b"test_student1@example.com,test,1,USA\n" \ + b"test_student2@example.com,test,1" data = self.upload_file(csv_content=csv_content) self.assertEquals(len(data['row_errors']['data_format_error']), 2) @@ -979,7 +979,7 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest """ Test failure case of a poorly formatted email field """ - csv_content = "test_student.example.com,dummy_notes" + csv_content = b"test_student.example.com,dummy_notes" data = self.upload_file(csv_content=csv_content) self.assertEquals(len(data['row_errors']['user_not_exist']), 1) @@ -990,7 +990,7 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest """ If the user is not enrolled in the course then there should be a user_not_enrolled error. """ - csv_content = "nonenrolled@test.com,dummy_notes" + csv_content = b"nonenrolled@test.com,dummy_notes" data = self.upload_file(csv_content=csv_content) self.assertEquals(len(data['row_errors']['user_not_enrolled']), 1) @@ -1007,7 +1007,7 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest whitelist=True, notes='' ) - csv_content = "test_student1@example.com,dummy_notes" + csv_content = b"test_student1@example.com,dummy_notes" data = self.upload_file(csv_content=csv_content) self.assertEquals(len(data['row_errors']['user_already_white_listed']), 1) self.assertEquals(len(data['general_errors']), 0) @@ -1018,8 +1018,8 @@ class TestCertificatesInstructorApiBulkWhiteListExceptions(SharedModuleStoreTest """ Test when the user does not attach a file """ - csv_content = "test_student1@example.com,dummy_notes\n" \ - "test_student2@example.com,dummy_notes" + csv_content = b"test_student1@example.com,dummy_notes\n" \ + b"test_student2@example.com,dummy_notes" uploaded_file = SimpleUploadedFile("temp.csv", csv_content) From 1b6e6634cbbd05f4f0e566eb887f7eb4233d0dc3 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Thu, 19 Sep 2019 09:43:14 -0400 Subject: [PATCH 03/18] Fix edxnotes stub tests BOM-633 --- common/djangoapps/terrain/stubs/edxnotes.py | 8 ++++---- common/djangoapps/terrain/stubs/http.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/terrain/stubs/edxnotes.py b/common/djangoapps/terrain/stubs/edxnotes.py index 027fdd23ea..08c1d60d78 100644 --- a/common/djangoapps/terrain/stubs/edxnotes.py +++ b/common/djangoapps/terrain/stubs/edxnotes.py @@ -121,7 +121,7 @@ class StubEdxNotesServiceHandler(StubHttpRequestHandler): } if status_code < 400 and content: headers["Content-Type"] = "application/json" - content = json.dumps(content) + content = json.dumps(content).encode('utf-8') else: headers["Content-Type"] = "text/html" @@ -131,7 +131,7 @@ class StubEdxNotesServiceHandler(StubHttpRequestHandler): """ Create a note, assign id, annotator_schema_version, created and updated dates. """ - note = json.loads(self.request_content) + note = json.loads(self.request_content.decode('utf-8')) note.update({ "id": uuid4().hex, "annotator_schema_version": "v1.0", @@ -146,7 +146,7 @@ class StubEdxNotesServiceHandler(StubHttpRequestHandler): The same as self._create, but it works a list of notes. """ try: - notes = json.loads(self.request_content) + notes = json.loads(self.request_content.decode('utf-8')) except ValueError: self.respond(400, "Bad Request") return @@ -181,7 +181,7 @@ class StubEdxNotesServiceHandler(StubHttpRequestHandler): """ Update the note by note id. """ - note = self.server.update_note(note_id, json.loads(self.request_content)) + note = self.server.update_note(note_id, json.loads(self.request_content.decode('utf-8'))) if note: self.respond(content=note) else: diff --git a/common/djangoapps/terrain/stubs/http.py b/common/djangoapps/terrain/stubs/http.py index dfd2902ca5..1213a1ef22 100644 --- a/common/djangoapps/terrain/stubs/http.py +++ b/common/djangoapps/terrain/stubs/http.py @@ -92,7 +92,7 @@ class StubHttpRequestHandler(BaseHTTPRequestHandler, object): Retrieve the content of the request. """ try: - length = int(self.headers.getheader('content-length')) + length = int(self.headers.get('content-length')) except (TypeError, ValueError): return "" From 23870802c65f0877da1edb0e85ea98e866ba589f Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Thu, 19 Sep 2019 09:52:42 -0400 Subject: [PATCH 04/18] Replace usage of UUID.get_hex() - BOM-672 --- .../tests/discussion/test_cohort_management.py | 10 +++++----- .../acceptance/tests/lms/test_lms_course_discovery.py | 2 +- .../tests/studio/test_studio_course_create.py | 2 +- openedx/core/lib/tests/test_xblock_utils.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py index fd960695da..b8f4fb30ab 100644 --- a/common/test/acceptance/tests/discussion/test_cohort_management.py +++ b/common/test/acceptance/tests/discussion/test_cohort_management.py @@ -346,7 +346,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then the cohort has 1 user And appropriate events have been emitted """ - cohort_name = str(uuid.uuid4().get_hex()[0:20]) + cohort_name = str(uuid.uuid4().hex[0:20]) self._verify_cohort_settings(cohort_name=cohort_name, assignment_type=None) def test_add_new_cohort_with_manual_assignment_type(self): @@ -361,7 +361,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then the cohort has 1 user And appropriate events have been emitted """ - cohort_name = str(uuid.uuid4().get_hex()[0:20]) + cohort_name = str(uuid.uuid4().hex[0:20]) self._verify_cohort_settings(cohort_name=cohort_name, assignment_type='manual') def test_add_new_cohort_with_random_assignment_type(self): @@ -376,7 +376,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin Then the cohort has 1 user And appropriate events have been emitted """ - cohort_name = str(uuid.uuid4().get_hex()[0:20]) + cohort_name = str(uuid.uuid4().hex[0:20]) self._verify_cohort_settings(cohort_name=cohort_name, assignment_type='random') def test_update_existing_cohort_settings(self): @@ -396,7 +396,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin And cohort with new name is present in cohorts dropdown list And cohort assignment type should be "manual" """ - cohort_name = str(uuid.uuid4().get_hex()[0:20]) + cohort_name = str(uuid.uuid4().hex[0:20]) new_cohort_name = '{old}__NEW'.format(old=cohort_name) self._verify_cohort_settings( cohort_name=cohort_name, @@ -422,7 +422,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin And I click on Save button Then I should see an error message """ - cohort_name = str(uuid.uuid4().get_hex()[0:20]) + cohort_name = str(uuid.uuid4().hex[0:20]) new_cohort_name = '' self._verify_cohort_settings( cohort_name=cohort_name, diff --git a/common/test/acceptance/tests/lms/test_lms_course_discovery.py b/common/test/acceptance/tests/lms/test_lms_course_discovery.py index 903562a2f7..a1f52df4aa 100644 --- a/common/test/acceptance/tests/lms/test_lms_course_discovery.py +++ b/common/test/acceptance/tests/lms/test_lms_course_discovery.py @@ -41,7 +41,7 @@ class CourseDiscoveryTest(AcceptanceTest): for i in range(12): org = 'test_org' - number = "{}{}".format(str(i), str(uuid.uuid4().get_hex().upper()[0:6])) + number = "{}{}".format(str(i), str(uuid.uuid4().hex.upper()[0:6])) run = "test_run" name = "test course" if i < 10 else "grass is always greener" settings = {'enrollment_start': datetime.datetime(1970, 1, 1).isoformat()} diff --git a/common/test/acceptance/tests/studio/test_studio_course_create.py b/common/test/acceptance/tests/studio/test_studio_course_create.py index 7df2bd6f03..75b9577486 100644 --- a/common/test/acceptance/tests/studio/test_studio_course_create.py +++ b/common/test/acceptance/tests/studio/test_studio_course_create.py @@ -31,7 +31,7 @@ class CreateCourseTest(AcceptanceTest): self.dashboard_page = DashboardPage(self.browser) self.course_name = "New Course Name" self.course_org = "orgX" - self.course_number = str(uuid.uuid4().get_hex().upper()[0:6]) + self.course_number = str(uuid.uuid4().hex.upper()[0:6]) self.course_run = "2015_T2" def test_create_course_with_non_existing_org(self): diff --git a/openedx/core/lib/tests/test_xblock_utils.py b/openedx/core/lib/tests/test_xblock_utils.py index 1e75368f7d..bc910cb607 100644 --- a/openedx/core/lib/tests/test_xblock_utils.py +++ b/openedx/core/lib/tests/test_xblock_utils.py @@ -108,7 +108,7 @@ class TestXblockUtils(SharedModuleStoreTestCase): frag=fragment, context={"wrap_xblock_data": {"custom-attribute": "custom-value"}}, usage_id_serializer=lambda usage_id: quote_slashes(six.text_type(usage_id)), - request_token=uuid.uuid1().get_hex() + request_token=uuid.uuid1().hex ) self.assertIsInstance(test_wrap_output, Fragment) self.assertIn('xblock-baseview', test_wrap_output.content) From a2b2431944f7bc23e90eca7ca208f89534289fec Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 17 Sep 2019 16:12:45 -0400 Subject: [PATCH 05/18] make upgrade --- requirements/edx/development.txt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 58e9b77fd9..bffd225550 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -58,8 +58,8 @@ click-log==0.3.2 click==7.0 code-annotations==0.3.2 colorama==0.4.1 -commonmark==0.9.0 # via recommonmark configparser==4.0.2 +commonmark==0.9.0 # via recommonmark contextlib2==0.5.5 cookies==2.2.1 coreapi==2.3.3 @@ -189,6 +189,7 @@ jmespath==0.9.4 jsondiff==1.1.1 jsonfield==2.0.2 jsonpickle==1.2 +jsonschema==3.0.2 kombu==3.0.37 laboratory==1.0.2 lazy-object-proxy==1.4.2 @@ -198,6 +199,7 @@ libsass==0.10.0 loremipsum==1.0.5 git+https://github.com/edx/xblock-lti-consumer.git@v1.1.8#egg=lti_consumer-xblock==1.1.8 lxml==3.8.0 +m2r==0.2.1 mailsnake==1.6.4 mako==1.0.2 mando==0.6.4 @@ -206,6 +208,7 @@ markey==0.8 markupsafe==1.1.1 maxminddb==1.4.1 mccabe==0.6.1 +mistune==0.8.4 # via m2r mock==1.0.1 modernize==0.7 git+https://github.com/edx/MongoDBProxy.git@d92bafe9888d2940f647a7b2b2383b29c752f35a#egg=MongoDBProxy==0.1.0+edx.2 @@ -259,6 +262,7 @@ pymongo==2.9.1 pynliner==0.8.0 pyparsing==2.2.0 pyquery==1.4.0 +pyrsistent==0.15.4 # via jsonschema pysqlite==2.8.3 ; python_version == "2.7" pysrt==1.1.1 pytest-attrib==0.1.3 @@ -311,6 +315,7 @@ sorl-thumbnail==12.3 sortedcontainers==2.1.0 soupsieve==1.9.3 sphinx==1.8.5 +sphinxcontrib-httpdomain==1.7.0 sphinxcontrib-websupport==1.1.2 # via sphinx sqlparse==0.3.0 staff-graded-xblock==0.5 @@ -351,4 +356,4 @@ xss-utils==0.1.1 zipp==0.6.0 # The following packages are considered to be unsafe in a requirements file: -# setuptools==41.2.0 # via caniusepython3, fs, lazy, pytest, python-levenshtein, sphinx +# setuptools==41.2.0 # via caniusepython3, fs, jsonschema, lazy, pytest, python-levenshtein, sphinx From a8a88c534aff0ac9498268644113c070f1c94c29 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 17 Sep 2019 17:37:20 -0400 Subject: [PATCH 06/18] make upgrade --- requirements/edx/development.txt | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index bffd225550..58e9b77fd9 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -58,8 +58,8 @@ click-log==0.3.2 click==7.0 code-annotations==0.3.2 colorama==0.4.1 -configparser==4.0.2 commonmark==0.9.0 # via recommonmark +configparser==4.0.2 contextlib2==0.5.5 cookies==2.2.1 coreapi==2.3.3 @@ -189,7 +189,6 @@ jmespath==0.9.4 jsondiff==1.1.1 jsonfield==2.0.2 jsonpickle==1.2 -jsonschema==3.0.2 kombu==3.0.37 laboratory==1.0.2 lazy-object-proxy==1.4.2 @@ -199,7 +198,6 @@ libsass==0.10.0 loremipsum==1.0.5 git+https://github.com/edx/xblock-lti-consumer.git@v1.1.8#egg=lti_consumer-xblock==1.1.8 lxml==3.8.0 -m2r==0.2.1 mailsnake==1.6.4 mako==1.0.2 mando==0.6.4 @@ -208,7 +206,6 @@ markey==0.8 markupsafe==1.1.1 maxminddb==1.4.1 mccabe==0.6.1 -mistune==0.8.4 # via m2r mock==1.0.1 modernize==0.7 git+https://github.com/edx/MongoDBProxy.git@d92bafe9888d2940f647a7b2b2383b29c752f35a#egg=MongoDBProxy==0.1.0+edx.2 @@ -262,7 +259,6 @@ pymongo==2.9.1 pynliner==0.8.0 pyparsing==2.2.0 pyquery==1.4.0 -pyrsistent==0.15.4 # via jsonschema pysqlite==2.8.3 ; python_version == "2.7" pysrt==1.1.1 pytest-attrib==0.1.3 @@ -315,7 +311,6 @@ sorl-thumbnail==12.3 sortedcontainers==2.1.0 soupsieve==1.9.3 sphinx==1.8.5 -sphinxcontrib-httpdomain==1.7.0 sphinxcontrib-websupport==1.1.2 # via sphinx sqlparse==0.3.0 staff-graded-xblock==0.5 @@ -356,4 +351,4 @@ xss-utils==0.1.1 zipp==0.6.0 # The following packages are considered to be unsafe in a requirements file: -# setuptools==41.2.0 # via caniusepython3, fs, jsonschema, lazy, pytest, python-levenshtein, sphinx +# setuptools==41.2.0 # via caniusepython3, fs, lazy, pytest, python-levenshtein, sphinx From 91c87fd65beaa139420518e16be43904fd8672d2 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Thu, 19 Sep 2019 10:38:49 -0400 Subject: [PATCH 07/18] Fixed last lms/djangoapps/discussion test failures - BOM-680 --- lms/djangoapps/discussion/notification_prefs/tests.py | 2 +- .../django_comment_common/comment_client/utils.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/discussion/notification_prefs/tests.py b/lms/djangoapps/discussion/notification_prefs/tests.py index b959884056..814bc1bcf0 100644 --- a/lms/djangoapps/discussion/notification_prefs/tests.py +++ b/lms/djangoapps/discussion/notification_prefs/tests.py @@ -28,7 +28,7 @@ from util.testing import UrlResetMixin @override_settings(SECRET_KEY="test secret key") class NotificationPrefViewTest(UrlResetMixin, TestCase): - INITIALIZATION_VECTOR = "\x00" * 16 + INITIALIZATION_VECTOR = b"\x00" * 16 @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py index d598ade307..29357fca2a 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py @@ -73,17 +73,18 @@ def perform_request(method, url, data_or_params=None, raw=False, ) metric_tags.append(u'status_code:{}'.format(response.status_code)) - if response.status_code > 200: + status_code = int(response.status_code) + if status_code > 200: metric_tags.append(u'result:failure') else: metric_tags.append(u'result:success') - if 200 < response.status_code < 500: + if 200 < status_code < 500: raise CommentClientRequestError(response.text, response.status_code) # Heroku returns a 503 when an application is in maintenance mode - elif response.status_code == 503: + elif status_code == 503: raise CommentClientMaintenanceError(response.text) - elif response.status_code == 500: + elif status_code == 500: raise CommentClient500Error(response.text) else: if raw: From 3a837609091457ae7b9a4eb021e3144a1053e38f Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 10:54:28 -0400 Subject: [PATCH 08/18] Run `make upgrade` --- requirements/edx/base.txt | 4 ++-- requirements/edx/development.txt | 6 +++--- requirements/edx/testing.txt | 6 +++--- scripts/xblock/requirements.txt | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index efe7ee039c..23c01dd3e3 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -97,7 +97,7 @@ docutils==0.15.2 # via botocore drf-yasg==1.16 edx-ace==0.1.10 edx-analytics-data-api-client==0.15.3 -edx-bulk-grades==0.6.0 +edx-bulk-grades==0.6.1 edx-ccx-keys==1.0.0 edx-celeryutils==0.3.0 edx-completion==2.0.0 @@ -253,7 +253,7 @@ wrapt==1.10.5 git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.2.4#egg=xblock-drag-and-drop-v2==2.2.4 git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536fff6d941#egg=xblock-poll==1.5.1 xblock-utils==1.2.2 -xblock==1.2.4 +xblock==1.2.5 xmlsec==1.3.3 # via python3-saml xss-utils==0.1.1 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 58e9b77fd9..f3dd3d5314 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -121,7 +121,7 @@ docutils==0.15.2 drf-yasg==1.16 edx-ace==0.1.10 edx-analytics-data-api-client==0.15.3 -edx-bulk-grades==0.6.0 +edx-bulk-grades==0.6.1 edx-ccx-keys==1.0.0 edx-celeryutils==0.3.0 edx-completion==2.0.0 @@ -339,12 +339,12 @@ web-fragments==0.3.0 webencodings==0.5.1 webob==1.8.5 websocket-client==0.56.0 -werkzeug==0.15.6 +werkzeug==0.16.0 wrapt==1.10.5 git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.2.4#egg=xblock-drag-and-drop-v2==2.2.4 git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536fff6d941#egg=xblock-poll==1.5.1 xblock-utils==1.2.2 -xblock==1.2.4 +xblock==1.2.5 xmlsec==1.3.3 xmltodict==0.12.0 xss-utils==0.1.1 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index c047db6bc3..2a4abe1b88 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -117,7 +117,7 @@ docutils==0.15.2 drf-yasg==1.16 edx-ace==0.1.10 edx-analytics-data-api-client==0.15.3 -edx-bulk-grades==0.6.0 +edx-bulk-grades==0.6.1 edx-ccx-keys==1.0.0 edx-celeryutils==0.3.0 edx-completion==2.0.0 @@ -325,12 +325,12 @@ web-fragments==0.3.0 webencodings==0.5.1 webob==1.8.5 websocket-client==0.56.0 # via docker -werkzeug==0.15.6 # via moto +werkzeug==0.16.0 # via moto wrapt==1.10.5 git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.2.4#egg=xblock-drag-and-drop-v2==2.2.4 git+https://github.com/open-craft/xblock-poll@add89e14558c30f3c8dc7431e5cd6536fff6d941#egg=xblock-poll==1.5.1 xblock-utils==1.2.2 -xblock==1.2.4 +xblock==1.2.5 xmlsec==1.3.3 xmltodict==0.12.0 # via moto xss-utils==0.1.1 diff --git a/scripts/xblock/requirements.txt b/scripts/xblock/requirements.txt index dcf4970ad6..5de4674661 100644 --- a/scripts/xblock/requirements.txt +++ b/scripts/xblock/requirements.txt @@ -8,4 +8,4 @@ certifi==2019.9.11 # via requests chardet==3.0.4 # via requests idna==2.8 # via requests requests==2.22.0 -urllib3==1.25.3 # via requests +urllib3==1.25.4 # via requests From cbf35593acbe54835ae222b373ec4de45c237223 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 11:15:06 -0400 Subject: [PATCH 09/18] BOM-618 Decode to utf-8 before loading as json in tests. --- .../djangoapps/student/tests/test_reset_password.py | 6 +++--- .../djangoapps/api_admin/api/v1/tests/test_views.py | 2 +- .../core/djangoapps/enrollments/tests/test_views.py | 12 ++++++++---- .../djangoapps/programs/tasks/v1/tests/test_tasks.py | 6 ++++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 0ca7b6f21c..76b0a4dbac 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -76,7 +76,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): bad_pwd_resp = password_reset(bad_pwd_req) # If they've got an unusable password, we return a successful response code self.assertEquals(bad_pwd_resp.status_code, 200) - obj = json.loads(bad_pwd_resp.content) + obj = json.loads(bad_pwd_resp.content.decode('utf-8')) self.assertEquals(obj, { 'success': True, 'value': "('registration/password_reset_done.html', [])", @@ -95,7 +95,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): # This prevents someone potentially trying to "brute-force" find out which # emails are and aren't registered with edX self.assertEquals(bad_email_resp.status_code, 200) - obj = json.loads(bad_email_resp.content) + obj = json.loads(bad_email_resp.content.decode('utf-8')) self.assertEquals(obj, { 'success': True, 'value': "('registration/password_reset_done.html', [])", @@ -145,7 +145,7 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.assertFalse(dop_models.RefreshToken.objects.filter(user=self.user).exists()) self.assertFalse(dot_models.AccessToken.objects.filter(user=self.user).exists()) self.assertFalse(dot_models.RefreshToken.objects.filter(user=self.user).exists()) - obj = json.loads(good_resp.content) + obj = json.loads(good_resp.content.decode('utf-8')) self.assertTrue(obj['success']) self.assertIn('e-mailed you instructions for setting your password', obj['value']) diff --git a/openedx/core/djangoapps/api_admin/api/v1/tests/test_views.py b/openedx/core/djangoapps/api_admin/api/v1/tests/test_views.py index 8916785905..2496963ea7 100644 --- a/openedx/core/djangoapps/api_admin/api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/api_admin/api/v1/tests/test_views.py @@ -46,7 +46,7 @@ class ApiAccessRequestViewTests(TestCase): """ Assert API response on `API Access Request` endpoint. """ - json_content = json.loads(api_response.content) + json_content = json.loads(api_response.content.decode('utf-8')) self.assertEqual(api_response.status_code, 200) self.assertEqual(json_content['count'], expected_results_count) diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index 42935a750f..8d1e059745 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -547,8 +547,12 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente def test_enrollment_already_enrolled(self): response = self.assert_enrollment_status() + response_json = json.loads(response.content.decode('utf-8')) + repeat_response = self.assert_enrollment_status(expected_status=status.HTTP_200_OK) - self.assertEqual(json.loads(response.content.decode('utf-8')), json.loads(repeat_response.content)) + repeat_json = json.loads(repeat_response.content.decode('utf-8')) + + self.assertEqual(response_json, repeat_json) def test_get_enrollment_with_invalid_key(self): resp = self.client.post( @@ -562,7 +566,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente format='json' ) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIn("No course ", resp.content) + self.assertIn("No course ", resp.content.decode('utf-8')) def test_enrollment_throttle_for_user(self): """Make sure a user requests do not exceed the maximum number of requests""" @@ -655,7 +659,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente ), {'include_expired': True}, ) - v_data = json.loads(v_response.content) + v_data = json.loads(v_response.content.decode('utf-8')) # Ensure that both course modes are returned self.assertEqual(len(v_data['course_modes']), 2) @@ -664,7 +668,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase, Ente h_response = self.client.get( reverse('courseenrollmentdetails', kwargs={"course_id": six.text_type(self.course.id)}), ) - h_data = json.loads(h_response.content) + h_data = json.loads(h_response.content.decode('utf-8')) # Ensure that only one course mode is returned and that it is honor self.assertEqual(len(h_data['course_modes']), 1) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 2ad18790aa..d9da31b8b9 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -109,7 +109,8 @@ class AwardProgramCertificateTestCase(TestCase): } ] } - self.assertEqual(json.loads(httpretty.last_request().body), expected_body) + last_request_body = httpretty.last_request().body.decode('utf-8') + self.assertEqual(json.loads(last_request_body), expected_body) @skip_unless_lms @@ -471,7 +472,8 @@ class PostCourseCertificateTestCase(TestCase): 'value': visible_date.strftime('%Y-%m-%dT%H:%M:%SZ') # text representation of date }] } - self.assertEqual(json.loads(httpretty.last_request().body), expected_body) + last_request_body = httpretty.last_request().body.decode('utf-8') + self.assertEqual(json.loads(last_request_body), expected_body) @skip_unless_lms From fc57bf976303a16017cd460a8a844f0071a55bb8 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Thu, 19 Sep 2019 11:16:38 -0400 Subject: [PATCH 10/18] Use bytes with GridFS when not specifying an encoding - BOM-157 --- .../management/commands/tests/test_delete_course.py | 4 ++-- lms/djangoapps/courseware/tests/test_video_mongo.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py b/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py index 8d0388aa14..d5b915a7c8 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py @@ -40,7 +40,7 @@ class DeleteCourseTests(ModuleStoreTestCase): store = contentstore() asset_key = course_run.id.make_asset_key('asset', 'test.txt') - content = StaticContent(asset_key, 'test.txt', 'plain/text', 'test data') + content = StaticContent(asset_key, 'test.txt', 'plain/text', b'test data') store.save(content) __, asset_count = store.get_all_content_for_course(course_run.id) assert asset_count == 1 @@ -69,7 +69,7 @@ class DeleteCourseTests(ModuleStoreTestCase): store = contentstore() asset_key = course_run.id.make_asset_key('asset', 'test.txt') - content = StaticContent(asset_key, 'test.txt', 'plain/text', 'test data') + content = StaticContent(asset_key, 'test.txt', 'plain/text', b'test data') store.save(content) __, asset_count = store.get_all_content_for_course(course_run.id) assert asset_count == 1 diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index fbc83780af..b6639beaa7 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -1310,7 +1310,7 @@ class TestEditorSavedMethod(BaseTestVideoXBlock): self.MODULESTORE = MODULESTORES[default_store] # pylint: disable=invalid-name self.initialize_block(metadata=self.metadata) item = self.store.get_item(self.item_descriptor.location) - with open(self.file_path, "r") as myfile: + with open(self.file_path, "rb") as myfile: save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location) item.sub = "3_yD_cEKoCk" # subs_video.srt.sjson does not exist before calling editor_saved function @@ -1330,7 +1330,7 @@ class TestEditorSavedMethod(BaseTestVideoXBlock): self.MODULESTORE = MODULESTORES[default_store] self.initialize_block(metadata=self.metadata) item = self.store.get_item(self.item_descriptor.location) - with open(self.file_path, "r") as myfile: + with open(self.file_path, "rb") as myfile: save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location) save_to_store(myfile.read(), 'subs_video.srt.sjson', 'text/sjson', item.location) item.sub = "3_yD_cEKoCk" From ae2c3b8edf294618793be820406c7d8c16f09383 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Thu, 19 Sep 2019 13:53:37 -0400 Subject: [PATCH 11/18] Fix mocks of open() - BOM-597 --- .../xmodule/modulestore/tests/test_xml_importer.py | 8 +++++++- pavelib/paver_tests/test_paver_quality.py | 14 ++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index 070e4163fc..0cdfb2bf52 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -9,6 +9,7 @@ import unittest from uuid import uuid4 import mock +import six from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from path import Path as path @@ -22,6 +23,11 @@ from xmodule.modulestore.xml_importer import StaticContentImporter, _update_and_ from xmodule.tests import DATA_DIR from xmodule.x_module import XModuleMixin +if six.PY2: + OPEN_BUILTIN = '__builtin__.open' +else: + OPEN_BUILTIN = 'builtins.open' + class ModuleStoreNoSettings(unittest.TestCase): """ @@ -379,7 +385,7 @@ class StaticContentImporterTest(unittest.TestCase): base_dir = path('/path/to/dir') full_file_path = os.path.join(base_dir, 'static/some_file.txt') self.mocked_content_store.generate_thumbnail.return_value = (None, None) - with mock.patch("__builtin__.open", mock.mock_open(read_data="data")) as mock_file: + with mock.patch(OPEN_BUILTIN, mock.mock_open(read_data=b"data")) as mock_file: self.static_content_importer.import_static_file( full_file_path=full_file_path, base_dir=base_dir diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 44309b5d89..0d558dd0ff 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -9,6 +9,7 @@ import tempfile import textwrap import unittest +import six from ddt import data, ddt, file_data, unpack from mock import MagicMock, mock_open, patch from path import Path as path @@ -17,6 +18,11 @@ from paver.easy import BuildFailure import pavelib.quality from pavelib.paver_tests.utils import PaverTestCase, fail_on_eslint +if six.PY2: + OPEN_BUILTIN = '__builtin__.open' +else: + OPEN_BUILTIN = 'builtins.open' + @ddt class TestPaverQualityViolations(unittest.TestCase): @@ -287,7 +293,7 @@ class TestPaverRunQuality(PaverTestCase): self.addCleanup(shutil.rmtree, self.report_dir) self.addCleanup(report_dir_patcher.stop) - @patch('__builtin__.open', mock_open()) + @patch(OPEN_BUILTIN, mock_open()) def test_failure_on_diffquality_pylint(self): """ If diff-quality fails on pylint, the paver task should also fail, but @@ -308,7 +314,7 @@ class TestPaverRunQuality(PaverTestCase): # of a diff-quality pylint failure, eslint is still called. self.assertEqual(self._mock_paver_sh.call_count, 2) - @patch('__builtin__.open', mock_open()) + @patch(OPEN_BUILTIN, mock_open()) def test_failure_on_diffquality_eslint(self): """ If diff-quality fails on eslint, the paver task should also fail @@ -329,7 +335,7 @@ class TestPaverRunQuality(PaverTestCase): # and once for diff quality with eslint self.assertEqual(self._mock_paver_sh.call_count, 4) - @patch('__builtin__.open', mock_open()) + @patch(OPEN_BUILTIN, mock_open()) def test_other_exception(self): """ If diff-quality fails for an unknown reason on the first run, then @@ -342,7 +348,7 @@ class TestPaverRunQuality(PaverTestCase): # Test that pylint is NOT called by counting calls self.assertEqual(self._mock_paver_sh.call_count, 1) - @patch('__builtin__.open', mock_open()) + @patch(OPEN_BUILTIN, mock_open()) def test_no_diff_quality_failures(self): # Assert nothing is raised pavelib.quality.run_quality("") From 24c0974f786aac493c282e1e151a1691020303ab Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 14:13:43 -0400 Subject: [PATCH 12/18] BOM-619 Fix csv encoding/decoding in management command test. --- .../management/commands/email_opt_in_list.py | 14 ++++++++++---- .../management/tests/test_email_opt_in_list.py | 11 ++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py b/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py index 474c91274b..41f73cb2a4 100644 --- a/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py +++ b/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py @@ -33,6 +33,7 @@ from django.core.management.base import BaseCommand, CommandError from django.db import connections from django.utils import timezone from opaque_keys.edx.keys import CourseKey +import six from six import text_type from six.moves import range @@ -251,14 +252,19 @@ class Command(BaseCommand): else: pref_set_datetime = self.DEFAULT_DATETIME_STR + + if not full_name: + full_name = "" + + # Only encode to utf-8 in python2 because python3's csv writer can handle unicode. writer.writerow({ "user_id": user_id, - "username": username.encode('utf-8'), - "email": email.encode('utf-8'), + "username": username.encode('utf-8') if six.PY2 else username, + "email": email.encode('utf-8') if six.PY2 else email, # There should not be a case where users are without full_names. We only need this safe check because # of ECOM-1995. - "full_name": full_name.encode('utf-8') if full_name else '', - "course_id": course_id.encode('utf-8'), + "full_name": full_name.encode('utf-8') if six.PY2 else full_name, + "course_id": course_id.encode('utf-8') if six.PY2 else course_id, "is_opted_in_for_email": is_opted_in if is_opted_in else "True", "preference_set_datetime": pref_set_datetime, }) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py index 9a64a50282..34fdc188b3 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py @@ -9,6 +9,7 @@ import tempfile from collections import defaultdict import ddt +import six from django.contrib.auth.models import User from django.core.management import call_command from django.core.management.base import CommandError @@ -213,7 +214,15 @@ class EmailOptInListTest(ModuleStoreTestCase): # Execute the command, but exclude the second course from the list output = self._run_command(self.TEST_ORG, chunk_size=2) - course_ids = [row['course_id'].strip().decode('utf-8') for row in output] + course_ids = [] + for row in output: + course_id = row['course_id'].strip() + # Python3 takes care of the decoding in the csv object + # but python 2 doesn't + if six.PY2: + course_id = course_id.decode('utf-8') + course_ids.append(course_id) + for course in self.courses: assert text_type(course.id) in course_ids From 9740e2e0779dd82bf0c9e698b3c668a4af274650 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 14:14:17 -0400 Subject: [PATCH 13/18] BOM-618 Don't decode string objects. --- common/lib/xmodule/xmodule/lti_module.py | 2 +- common/lib/xmodule/xmodule/tests/__init__.py | 2 +- .../courseware/management/commands/tests/test_dump_course.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 01753cb0d0..302c65413a 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -657,7 +657,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'} # so '='' becomes '%3D'. # We send form via browser, so browser will encode it again, # So we need to decode signature back: - params[u'oauth_signature'] = six.moves.urllib.parse.unquote(params[u'oauth_signature']).decode('utf8') + params[u'oauth_signature'] = six.moves.urllib.parse.unquote(params[u'oauth_signature']).encode('utf-8').decode('utf8') # Add LTI parameters to OAuth parameters for sending in form. params.update(body) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 6aab74ede0..2113f76678 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -176,7 +176,7 @@ def mock_render_template(*args, **kwargs): Allows us to not depend on any actual template rendering mechanism, while still returning a unicode object """ - return pprint.pformat((args, kwargs)).decode() + return pprint.pformat((args, kwargs)).encode().decode() class ModelsTest(unittest.TestCase): diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py index 34d50f7021..ebf1c24329 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py @@ -88,7 +88,7 @@ class CommandsTestBase(SharedModuleStoreTestCase): def test_dump_course_ids(self): output = self.call_command('dump_course_ids') - dumped_courses = output.decode('utf-8').strip().split('\n') + dumped_courses = output.strip().split('\n') course_ids = {text_type(course_id) for course_id in self.loaded_courses} dumped_ids = set(dumped_courses) self.assertEqual(course_ids, dumped_ids) From 4eaf875741a0ccaa6335aa0e25de1724615c807d Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 15:53:39 -0400 Subject: [PATCH 14/18] BOM-621 Fix hashing in a bunch of places. --- common/djangoapps/student/tests/test_models.py | 2 +- common/djangoapps/third_party_auth/pipeline.py | 8 ++++---- .../third_party_auth/tests/specs/test_google.py | 6 +++--- .../user_api/accounts/tests/test_image_helpers.py | 2 +- .../core/djangoapps/user_api/accounts/tests/test_views.py | 2 +- openedx/features/enterprise_support/utils.py | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index e6b7d482e8..e3d83784fd 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -63,7 +63,7 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): self.assertIsNone(CourseEnrollment.generate_enrollment_status_hash(AnonymousUser())) # No enrollments - expected = hashlib.md5(self.user.username).hexdigest() + expected = hashlib.md5(self.user.username.encode('utf-8')).hexdigest() self.assertEqual(CourseEnrollment.generate_enrollment_status_hash(self.user), expected) self.assert_enrollment_status_hash_cached(self.user, expected) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 03a9691a3b..55008258b5 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -503,17 +503,17 @@ def redirect_to_custom_form(request, auth_entry, details, kwargs): if isinstance(secret_key, six.text_type): secret_key = secret_key.encode('utf-8') custom_form_url = form_info['url'] - data_str = json.dumps({ + data_bytes = json.dumps({ "auth_entry": auth_entry, "backend_name": backend_name, "provider_id": provider_id, "user_details": details, - }) - digest = hmac.new(secret_key, msg=data_str, digestmod=hashlib.sha256).digest() + }).encode('utf-8') + digest = hmac.new(secret_key, msg=data_bytes, digestmod=hashlib.sha256).digest() # Store the data in the session temporarily, then redirect to a page that will POST it to # the custom login/register page. request.session['tpa_custom_auth_entry_data'] = { - 'data': base64.b64encode(data_str), + 'data': base64.b64encode(data_bytes), 'hmac': base64.b64encode(digest), 'post_url': custom_form_url, } diff --git a/common/djangoapps/third_party_auth/tests/specs/test_google.py b/common/djangoapps/third_party_auth/tests/specs/test_google.py index db8661810e..9562c0a9ac 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_google.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_google.py @@ -78,8 +78,8 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): response = self.client.get(response['Location']) self.assertEqual(response.status_code, 200) - self.assertIn('action="/misc/my-custom-registration-form" method="post"', response.content) - data_decoded = base64.b64decode(response.context['data']) + self.assertIn('action="/misc/my-custom-registration-form" method="post"', response.content.decode('utf-8')) + data_decoded = base64.b64decode(response.context['data']).decode('utf-8') data_parsed = json.loads(data_decoded) # The user's details get passed to the custom page as a base64 encoded query parameter: self.assertEqual(data_parsed, { @@ -96,7 +96,7 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): }) # Check the hash that is used to confirm the user's data in the GET parameter is correct secret_key = settings.THIRD_PARTY_AUTH_CUSTOM_AUTH_FORMS['custom1']['secret_key'] - hmac_expected = hmac.new(secret_key, msg=data_decoded, digestmod=hashlib.sha256).digest() + hmac_expected = hmac.new(secret_key.encode('utf-8'), msg=data_decoded.encode('utf-8'), digestmod=hashlib.sha256).digest() self.assertEqual(base64.b64decode(response.context['hmac']), hmac_expected) # Now our custom registration form creates or logs in the user: diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py b/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py index b2e04e16f1..bb867c952b 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py @@ -73,7 +73,7 @@ class ProfileImageUrlTestCase(TestCase): """ self.user.profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOAD_DT self.user.profile.save() - expected_name = hashlib.md5('secret' + self.user.username).hexdigest() + expected_name = hashlib.md5('secret' + self.user.username.encode('utf-8')).hexdigest() actual_urls = get_profile_image_urls_for_user(self.user) self.verify_urls(actual_urls, expected_name, is_default=False) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 683651a129..0812282ee8 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -126,7 +126,7 @@ class UserAPITestCase(APITestCase): template = '{root}/{filename}_{{size}}.{extension}' if has_profile_image: url_root = 'http://example-storage.com/profile-images' - filename = hashlib.md5('secret' + self.user.username).hexdigest() + filename = hashlib.md5('secret' + self.user.username.encode('utf-8')).hexdigest() file_extension = 'jpg' template += '?v={}'.format(TEST_PROFILE_IMAGE_UPLOADED_AT.strftime("%s")) else: diff --git a/openedx/features/enterprise_support/utils.py b/openedx/features/enterprise_support/utils.py index 347029267d..a5a05e402c 100644 --- a/openedx/features/enterprise_support/utils.py +++ b/openedx/features/enterprise_support/utils.py @@ -39,7 +39,7 @@ def get_cache_key(**kwargs): """ key = '__'.join(['{}:{}'.format(item, value) for item, value in six.iteritems(kwargs)]) - return hashlib.md5(key).hexdigest() + return hashlib.md5(key.encode('utf-8')).hexdigest() def get_data_consent_share_cache_key(user_id, course_id): From 802929fde4e1490d709098b44eb360ed7cfb8b51 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 19 Sep 2019 16:18:23 -0400 Subject: [PATCH 15/18] BOM-622 Type errors in content matching. --- lms/djangoapps/commerce/tests/test_views.py | 8 ++-- lms/djangoapps/courseware/tests/test_i18n.py | 46 +++++++++---------- .../learner_dashboard/tests/test_programs.py | 2 +- lms/djangoapps/support/tests/test_views.py | 2 +- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lms/djangoapps/commerce/tests/test_views.py b/lms/djangoapps/commerce/tests/test_views.py index e1d6962402..32684badba 100644 --- a/lms/djangoapps/commerce/tests/test_views.py +++ b/lms/djangoapps/commerce/tests/test_views.py @@ -95,7 +95,7 @@ class ReceiptViewTests(UserMixin, ModuleStoreTestCase): else: expected_pattern = r"(\s+)Payment Failed" response = self.post_to_receipt_page(post_data) - self.assertRegexpMatches(response.content, expected_pattern) + self.assertRegexpMatches(response.content.decode('utf-8'), expected_pattern) @ddt.data('ACCEPT', 'REJECT', 'ERROR') def test_cybersource_decision(self, decision): @@ -106,7 +106,7 @@ class ReceiptViewTests(UserMixin, ModuleStoreTestCase): post_data = {'decision': decision, 'reason_code': '200', 'signed_field_names': 'dummy'} expected_pattern = r"<title>(\s+)Receipt" if decision == 'ACCEPT' else r"<title>(\s+)Payment Failed" response = self.post_to_receipt_page(post_data) - self.assertRegexpMatches(response.content, expected_pattern) + self.assertRegexpMatches(response.content.decode('utf-8'), expected_pattern) @ddt.data(True, False) @mock.patch('lms.djangoapps.commerce.views.is_user_payment_error') @@ -123,8 +123,8 @@ class ReceiptViewTests(UserMixin, ModuleStoreTestCase): user_message = "There was a problem with this transaction" system_message = "A system error occurred while processing your payment" - self.assertRegexpMatches(response.content, user_message if is_user_message_expected else system_message) - self.assertNotRegexpMatches(response.content, user_message if not is_user_message_expected else system_message) + self.assertRegexpMatches(response.content.decode('utf-8'), user_message if is_user_message_expected else system_message) + self.assertNotRegexpMatches(response.content.decode('utf-8'), user_message if not is_user_message_expected else system_message) @with_comprehensive_theme("edx.org") def test_hide_nav_header(self): diff --git a/lms/djangoapps/courseware/tests/test_i18n.py b/lms/djangoapps/courseware/tests/test_i18n.py index 2c658685b0..8133af78c5 100644 --- a/lms/djangoapps/courseware/tests/test_i18n.py +++ b/lms/djangoapps/courseware/tests/test_i18n.py @@ -80,30 +80,30 @@ class I18nTestCase(BaseI18nTestCase): def test_default_is_en(self): self.release_languages('fr') response = self.client.get('/') - self.assert_tag_has_attr(response.content, "html", "lang", "en") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "en") self.assertEqual(response['Content-Language'], 'en') - self.assert_tag_has_attr(response.content, "body", "class", "lang_en") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "lang_en") def test_esperanto(self): self.release_languages('fr, eo') response = self.client.get('/', HTTP_ACCEPT_LANGUAGE='eo') - self.assert_tag_has_attr(response.content, "html", "lang", "eo") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "eo") self.assertEqual(response['Content-Language'], 'eo') - self.assert_tag_has_attr(response.content, "body", "class", "lang_eo") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "lang_eo") def test_switching_languages_bidi(self): self.release_languages('ar, eo') response = self.client.get('/') - self.assert_tag_has_attr(response.content, "html", "lang", "en") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "en") self.assertEqual(response['Content-Language'], 'en') - self.assert_tag_has_attr(response.content, "body", "class", "lang_en") - self.assert_tag_has_attr(response.content, "body", "class", "ltr") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "lang_en") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "ltr") response = self.client.get('/', HTTP_ACCEPT_LANGUAGE='ar') - self.assert_tag_has_attr(response.content, "html", "lang", "ar") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "ar") self.assertEqual(response['Content-Language'], 'ar') - self.assert_tag_has_attr(response.content, "body", "class", "lang_ar") - self.assert_tag_has_attr(response.content, "body", "class", "rtl") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "lang_ar") + self.assert_tag_has_attr(response.content.decode('utf-8'), "body", "class", "rtl") class I18nRegressionTests(BaseI18nTestCase): @@ -114,7 +114,7 @@ class I18nRegressionTests(BaseI18nTestCase): # Regression test; LOC-72, and an issue with Django self.release_languages('es-419') response = self.client.get('/', HTTP_ACCEPT_LANGUAGE='es-419') - self.assert_tag_has_attr(response.content, "html", "lang", "es-419") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "es-419") def test_unreleased_lang_resolution(self): # Regression test; LOC-85 @@ -126,12 +126,12 @@ class I18nRegressionTests(BaseI18nTestCase): # in the http request (NOT with the ?preview-lang query param) should # receive files for 'fa' response = self.client.get(self.url, HTTP_ACCEPT_LANGUAGE='fa-ir') - self.assert_tag_has_attr(response.content, "html", "lang", "fa") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "fa") # Now try to access with dark lang self.client.post(self.preview_language_url, {'preview_language': 'fa-ir', 'action': 'set_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", "fa-ir") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "fa-ir") def test_preview_lang(self): self.user_login() @@ -141,23 +141,23 @@ class I18nRegressionTests(BaseI18nTestCase): site_lang = settings.LANGUAGE_CODE # Visit the front page; verify we see site default lang response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", site_lang) + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", site_lang) # Verify we can switch language using the preview-lang query param # Set the language self.client.post(self.preview_language_url, {'preview_language': 'eo', 'action': 'set_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", "eo") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "eo") # We should be able to see released languages using preview-lang, too self.client.post(self.preview_language_url, {'preview_language': 'es-419', 'action': 'set_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", "es-419") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "es-419") # Clearing the language should go back to site default self.client.post(self.preview_language_url, {'action': 'reset_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", site_lang) + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", site_lang) class I18nLangPrefTests(BaseI18nTestCase): @@ -187,18 +187,18 @@ class I18nLangPrefTests(BaseI18nTestCase): # Visit the front page; verify we see site default lang response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", self.site_lang) + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", self.site_lang) # Set user language preference self.set_lang_preference('ar') # and verify we now get an ar response response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", 'ar') + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", 'ar') # Verify that switching language preference gives the right language self.set_lang_preference('es-419') response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", 'es-419') + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", 'es-419') def test_preview_precedence(self): # Regression test; LOC-87 @@ -210,13 +210,13 @@ class I18nLangPrefTests(BaseI18nTestCase): self.client.post(self.preview_language_url, {'preview_language': 'eo', 'action': 'set_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", 'eo') + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", 'eo') # Hitting another page should keep the dark language set. response = self.client.get(reverse('courses')) - self.assert_tag_has_attr(response.content, "html", "lang", "eo") + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", "eo") # Clearing language must set language back to preference language self.client.post(self.preview_language_url, {'action': 'reset_preview_language'}) response = self.client.get(self.url) - self.assert_tag_has_attr(response.content, "html", "lang", 'ar') + self.assert_tag_has_attr(response.content.decode('utf-8'), "html", "lang", 'ar') diff --git a/lms/djangoapps/learner_dashboard/tests/test_programs.py b/lms/djangoapps/learner_dashboard/tests/test_programs.py index 5a300de3e9..1a63165790 100644 --- a/lms/djangoapps/learner_dashboard/tests/test_programs.py +++ b/lms/djangoapps/learner_dashboard/tests/test_programs.py @@ -40,7 +40,7 @@ def load_serialized_data(response, key): Extract and deserialize serialized data from the response. """ pattern = re.compile(u'{key}: (?P<data>\\[.*\\])'.format(key=key)) - match = pattern.search(response.content) + match = pattern.search(response.content.decode('utf-8')) serialized = match.group('data') return json.loads(serialized) diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 8094e69838..f0194d25c7 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -331,7 +331,7 @@ class SupportViewEnrollmentsTests(SharedModuleStoreTestCase, SupportViewTestCase data['course_id'] = six.text_type(self.course.id) response = self.client.post(self.url, data) self.assertEqual(response.status_code, 400) - self.assertIsNotNone(re.match(error_message, response.content)) + self.assertIsNotNone(re.match(error_message, response.content.decode('utf-8'))) self.assert_enrollment(CourseMode.AUDIT) self.assertIsNone(ManualEnrollmentAudit.get_manual_enrollment_by_email(self.student.email)) From 0e1ad4cd262727c07b5e0f30b8ddec70ef9ee028 Mon Sep 17 00:00:00 2001 From: Feanil Patel <feanil@edx.org> Date: Thu, 19 Sep 2019 16:19:26 -0400 Subject: [PATCH 16/18] Fix pylint error in email_opt_in_list. --- .../djangoapps/user_api/management/commands/email_opt_in_list.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py b/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py index 41f73cb2a4..ffe837e1e0 100644 --- a/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py +++ b/openedx/core/djangoapps/user_api/management/commands/email_opt_in_list.py @@ -252,7 +252,6 @@ class Command(BaseCommand): else: pref_set_datetime = self.DEFAULT_DATETIME_STR - if not full_name: full_name = "" From 0c1e18ff537fb71b3b7c3daaedec0394965ff33a Mon Sep 17 00:00:00 2001 From: Feanil Patel <feanil@edx.org> Date: Thu, 19 Sep 2019 16:57:54 -0400 Subject: [PATCH 17/18] BOM-622 Fix more type errors. --- lms/djangoapps/ccx/tests/test_views.py | 11 +++++------ lms/djangoapps/ccx/views.py | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index fb59095873..347fd89342 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -346,7 +346,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): self.assertEqual(response.status_code, 200) self.assertTrue(re.search( '<form action=".+create_ccx"', - response.content)) + response.content.decode('utf-8'))) def test_create_ccx_with_ccx_connector_set(self): """ @@ -365,7 +365,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): "A CCX can only be created on this course through an external service." " Contact a course admin to give you access." ) - self.assertTrue(re.search(error_message, response.content)) + self.assertTrue(re.search(error_message, response.content.decode('utf-8'))) def test_create_ccx(self, ccx_name='New CCX'): """ @@ -392,7 +392,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase): course_key = CourseKey.from_string(ccx_key) self.assertTrue(CourseEnrollment.is_enrolled(self.coach, course_key)) - self.assertTrue(re.search('id="ccx-schedule"', response.content)) + self.assertTrue(re.search('id="ccx-schedule"', response.content.decode('utf-8'))) # check if the max amount of student that can be enrolled has been overridden ccx = CustomCourseForEdX.objects.get() @@ -1104,9 +1104,8 @@ class TestCCXGrades(FieldOverrideTestMixin, SharedModuleStoreTestCase, LoginEnro response['content-disposition'], 'attachment' ) - rows = response.content.strip().split('\r') + rows = response.content.decode('utf-8').strip().split('\r') headers = rows[0] - # picking first student records data = dict(list(zip(headers.strip().split(','), rows[1].strip().split(',')))) self.assertNotIn('HW 04', data) @@ -1276,7 +1275,7 @@ class TestStudentViewsWithCCX(ModuleStoreTestCase): self.client.login(username=self.student.username, password=self.student_password) response = self.client.get(reverse('dashboard')) self.assertEqual(response.status_code, 200) - self.assertTrue(re.search('Test CCX', response.content)) + self.assertTrue(re.search('Test CCX', response.content.decode('utf-8'))) def test_load_courseware(self): self.client.login(username=self.student.username, password=self.student_password) diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index c7b3e17885..fdd77e9708 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -539,7 +539,7 @@ def ccx_grades_csv(request, course, ccx=None): if not header: # Encode the header row in utf-8 encoding in case there are # unicode characters - header = [section['label'].encode('utf-8') + header = [section['label'].encode('utf-8') if six.PY2 else section['label'] for section in course_grade.summary[u'section_breakdown']] rows.append(["id", "email", "username", "grade"] + header) From 844acea50ed277da2c0209fc62d1b2bcbb9c3d7e Mon Sep 17 00:00:00 2001 From: Feanil Patel <feanil@edx.org> Date: Thu, 19 Sep 2019 17:05:02 -0400 Subject: [PATCH 18/18] Fix linting errors. --- .../third_party_auth/tests/specs/test_google.py | 6 +++++- lms/djangoapps/commerce/tests/test_views.py | 10 ++++++++-- lms/djangoapps/courseware/tests/test_video_mongo.py | 4 ++-- .../user_api/accounts/tests/test_image_helpers.py | 4 +++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_google.py b/common/djangoapps/third_party_auth/tests/specs/test_google.py index 9562c0a9ac..fa6ab9beb1 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_google.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_google.py @@ -96,7 +96,11 @@ class GoogleOauth2IntegrationTest(base.Oauth2IntegrationTest): }) # Check the hash that is used to confirm the user's data in the GET parameter is correct secret_key = settings.THIRD_PARTY_AUTH_CUSTOM_AUTH_FORMS['custom1']['secret_key'] - hmac_expected = hmac.new(secret_key.encode('utf-8'), msg=data_decoded.encode('utf-8'), digestmod=hashlib.sha256).digest() + hmac_expected = hmac.new( + secret_key.encode('utf-8'), + msg=data_decoded.encode('utf-8'), + digestmod=hashlib.sha256 + ).digest() self.assertEqual(base64.b64decode(response.context['hmac']), hmac_expected) # Now our custom registration form creates or logs in the user: diff --git a/lms/djangoapps/commerce/tests/test_views.py b/lms/djangoapps/commerce/tests/test_views.py index 32684badba..53afd0eca5 100644 --- a/lms/djangoapps/commerce/tests/test_views.py +++ b/lms/djangoapps/commerce/tests/test_views.py @@ -123,8 +123,14 @@ class ReceiptViewTests(UserMixin, ModuleStoreTestCase): user_message = "There was a problem with this transaction" system_message = "A system error occurred while processing your payment" - self.assertRegexpMatches(response.content.decode('utf-8'), user_message if is_user_message_expected else system_message) - self.assertNotRegexpMatches(response.content.decode('utf-8'), user_message if not is_user_message_expected else system_message) + self.assertRegexpMatches( + response.content.decode('utf-8'), + user_message if is_user_message_expected else system_message + ) + self.assertNotRegexpMatches( + response.content.decode('utf-8'), + user_message if not is_user_message_expected else system_message + ) @with_comprehensive_theme("edx.org") def test_hide_nav_header(self): diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index b6639beaa7..34bb6f6d0a 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -1310,7 +1310,7 @@ class TestEditorSavedMethod(BaseTestVideoXBlock): self.MODULESTORE = MODULESTORES[default_store] # pylint: disable=invalid-name self.initialize_block(metadata=self.metadata) item = self.store.get_item(self.item_descriptor.location) - with open(self.file_path, "rb") as myfile: + with open(self.file_path, "rb") as myfile: # pylint: disable=open-builtin save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location) item.sub = "3_yD_cEKoCk" # subs_video.srt.sjson does not exist before calling editor_saved function @@ -1330,7 +1330,7 @@ class TestEditorSavedMethod(BaseTestVideoXBlock): self.MODULESTORE = MODULESTORES[default_store] self.initialize_block(metadata=self.metadata) item = self.store.get_item(self.item_descriptor.location) - with open(self.file_path, "rb") as myfile: + with open(self.file_path, "rb") as myfile: # pylint: disable=open-builtin save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location) save_to_store(myfile.read(), 'subs_video.srt.sjson', 'text/sjson', item.location) item.sub = "3_yD_cEKoCk" diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py b/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py index bb867c952b..97d95576f1 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py @@ -12,6 +12,7 @@ from pytz import UTC from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory +from six import text_type from ..image_helpers import get_profile_image_urls_for_user @@ -73,7 +74,8 @@ class ProfileImageUrlTestCase(TestCase): """ self.user.profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOAD_DT self.user.profile.save() - expected_name = hashlib.md5('secret' + self.user.username.encode('utf-8')).hexdigest() + expected_name = hashlib.md5( + 'secret' + text_type(self.user.username).encode('utf-8')).hexdigest() actual_urls = get_profile_image_urls_for_user(self.user) self.verify_urls(actual_urls, expected_name, is_default=False)