From d6d9a17e675abd926daa434ac95d8318723644c6 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sat, 28 Sep 2019 16:00:53 -0400 Subject: [PATCH] student app: python-3 upgrade --- .../management/commands/bulk_unenroll.py | 2 +- .../tests/test_bulk_change_enrollment_csv.py | 20 ++++--- .../management/tests/test_bulk_unenroll.py | 26 +++++---- .../student/tests/test_bulk_email_settings.py | 8 +-- .../student/tests/test_verification_status.py | 4 +- common/djangoapps/student/tests/test_views.py | 58 ++++++++++--------- common/djangoapps/student/tests/tests.py | 13 +++-- 7 files changed, 73 insertions(+), 58 deletions(-) diff --git a/common/djangoapps/student/management/commands/bulk_unenroll.py b/common/djangoapps/student/management/commands/bulk_unenroll.py index 2d73b9aeda..6ac0a4f0a1 100644 --- a/common/djangoapps/student/management/commands/bulk_unenroll.py +++ b/common/djangoapps/student/management/commands/bulk_unenroll.py @@ -32,7 +32,7 @@ class Command(BaseCommand): def handle(self, *args, **options): csv_path = options['csv_path'] - with open(csv_path) as csvfile: + with open(csv_path, 'rb') as csvfile: reader = unicodecsv.DictReader(csvfile) for row in reader: username = row['username'] diff --git a/common/djangoapps/student/management/tests/test_bulk_change_enrollment_csv.py b/common/djangoapps/student/management/tests/test_bulk_change_enrollment_csv.py index fe83d3d396..bff7a102e0 100644 --- a/common/djangoapps/student/management/tests/test_bulk_change_enrollment_csv.py +++ b/common/djangoapps/student/management/tests/test_bulk_change_enrollment_csv.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +import six import unittest from tempfile import NamedTemporaryFile @@ -43,10 +44,11 @@ class BulkChangeEnrollmentCSVTests(SharedModuleStoreTestCase): self.courses.append(course) self.enrollments.append(CourseEnrollment.enroll(user, course.id, mode=CourseMode.AUDIT)) - def _write_test_csv(self, csv, lines=None): + def _write_test_csv(self, csv, lines): """Write a test csv file with the lines provided""" - csv.write("course_id,user,mode,\n") - csv.writelines(lines) + csv.write(b"course_id,user,mode,\n") + for line in lines: + csv.write(six.b(line)) csv.seek(0) return csv @@ -54,7 +56,7 @@ class BulkChangeEnrollmentCSVTests(SharedModuleStoreTestCase): def test_user_not_exist(self): """Verify that warning is logged for non existing user.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines="course-v1:edX+DemoX+Demo_Course,user,audit\n") + csv = self._write_test_csv(csv, lines=["course-v1:edX+DemoX+Demo_Course,user,audit\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_change_enrollment_csv", "--csv_file_path={}".format(csv.name)) @@ -70,7 +72,7 @@ class BulkChangeEnrollmentCSVTests(SharedModuleStoreTestCase): def test_invalid_course_key(self): """Verify in case of invalid course key warning is logged.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines="Demo_Course,river,audit\n") + csv = self._write_test_csv(csv, lines=["Demo_Course,river,audit\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_change_enrollment_csv", "--csv_file_path={}".format(csv.name)) @@ -86,7 +88,7 @@ class BulkChangeEnrollmentCSVTests(SharedModuleStoreTestCase): def test_already_enrolled_student(self): """ Verify in case if a user is already enrolled warning is logged.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines=str(self.courses[0].id) + ",amy,audit\n") + csv = self._write_test_csv(csv, lines=[str(self.courses[0].id) + ",amy,audit\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_change_enrollment_csv", "--csv_file_path={}".format(csv.name)) @@ -105,8 +107,10 @@ class BulkChangeEnrollmentCSVTests(SharedModuleStoreTestCase): @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_bulk_enrollment(self): """ Test all users are enrolled using the command.""" - lines = (str(enrollment.course.id) + "," + str(enrollment.user.username) + ",verified\n" - for enrollment in self.enrollments) + lines = [ + str(enrollment.course.id) + "," + str(enrollment.user.username) + ",verified\n" + for enrollment in self.enrollments + ] with NamedTemporaryFile() as csv: csv = self._write_test_csv(csv, lines=lines) diff --git a/common/djangoapps/student/management/tests/test_bulk_unenroll.py b/common/djangoapps/student/management/tests/test_bulk_unenroll.py index 55906eb1e4..5a402e1968 100644 --- a/common/djangoapps/student/management/tests/test_bulk_unenroll.py +++ b/common/djangoapps/student/management/tests/test_bulk_unenroll.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +import six from tempfile import NamedTemporaryFile from django.core.management import call_command @@ -38,17 +39,18 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): self.users.append(user) self.enrollments.append(CourseEnrollment.enroll(user, self.course.id, mode='audit')) - def _write_test_csv(self, csv, lines=None): - """Write a test csv file with the lines procided""" - csv.write("user_id,username,email,course_id\n") - csv.writelines(lines) + def _write_test_csv(self, csv, lines): + """Write a test csv file with the lines provided""" + csv.write(b"user_id,username,email,course_id\n") + for line in lines: + csv.write(six.b(line)) csv.seek(0) return csv def test_user_not_exist(self): """Verify that warning user not exist is logged for non existing user.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines="111,test,test@example.com,course-v1:edX+DemoX+Demo_Course\n") + csv = self._write_test_csv(csv, lines=["111,test,test@example.com,course-v1:edX+DemoX+Demo_Course\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) @@ -63,7 +65,7 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): def test_invalid_course_key(self): """Verify in case of invalid course key warning is logged.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines="111,amy,amy@pond.com,test_course\n") + csv = self._write_test_csv(csv, lines=["111,amy,amy@pond.com,test_course\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) @@ -79,7 +81,7 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): def test_user_not_enrolled(self): """Verify in case of user not enrolled in course warning is logged.""" with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines="111,amy,amy@pond.com,course-v1:edX+DemoX+Demo_Course\n") + csv = self._write_test_csv(csv, lines=["111,amy,amy@pond.com,course-v1:edX+DemoX+Demo_Course\n"]) with LogCapture(LOGGER_NAME) as log: call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) @@ -94,11 +96,13 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): def test_bulk_un_enroll(self): """Verify users are unenrolled using the command.""" - lines = (str(enrollment.user.id) + "," + enrollment.user.username + "," + - enrollment.user.email + "," + str(enrollment.course.id) + "\n" - for enrollment in self.enrollments) + lines = [ + str(enrollment.user.id) + "," + enrollment.user.username + "," + + enrollment.user.email + "," + str(enrollment.course.id) + "\n" + for enrollment in self.enrollments + ] with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines=lines)\ + csv = self._write_test_csv(csv, lines=lines) call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) for enrollment in CourseEnrollment.objects.all(): diff --git a/common/djangoapps/student/tests/test_bulk_email_settings.py b/common/djangoapps/student/tests/test_bulk_email_settings.py index 50bd90dc07..f6a008a423 100644 --- a/common/djangoapps/student/tests/test_bulk_email_settings.py +++ b/common/djangoapps/student/tests/test_bulk_email_settings.py @@ -58,13 +58,13 @@ class TestStudentDashboardEmailView(SharedModuleStoreTestCase): BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False) # Assert that the URL for the email view is in the response response = self.client.get(self.url) - self.assertIn(self.email_modal_link, response.content) + self.assertContains(response, self.email_modal_link) def test_email_flag_false(self): BulkEmailFlag.objects.create(enabled=False) # Assert that the URL for the email view is not in the response response = self.client.get(self.url) - self.assertNotIn(self.email_modal_link, response.content) + self.assertNotContains(response, self.email_modal_link) def test_email_unauthorized(self): BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=True) @@ -73,7 +73,7 @@ class TestStudentDashboardEmailView(SharedModuleStoreTestCase): # Assert that the URL for the email view is not in the response # if this course isn't authorized response = self.client.get(self.url) - self.assertNotIn(self.email_modal_link, response.content) + self.assertNotContains(response, self.email_modal_link) def test_email_authorized(self): BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=True) @@ -85,4 +85,4 @@ class TestStudentDashboardEmailView(SharedModuleStoreTestCase): # Assert that the URL for the email view is not in the response # if this course isn't authorized response = self.client.get(self.url) - self.assertIn(self.email_modal_link, response.content) + self.assertContains(response, self.email_modal_link) diff --git a/common/djangoapps/student/tests/test_verification_status.py b/common/djangoapps/student/tests/test_verification_status.py index fb180fd79a..426b9ffa01 100644 --- a/common/djangoapps/student/tests/test_verification_status.py +++ b/common/djangoapps/student/tests/test_verification_status.py @@ -299,7 +299,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): self._assert_course_verification_status(VERIFY_STATUS_APPROVED) response2 = self.client.get(self.dashboard_url) self.assertContains(response2, attempt2.expiration_datetime.strftime("%m/%d/%Y")) - self.assertEqual(response2.content.count(attempt2.expiration_datetime.strftime("%m/%d/%Y")), 2) + self.assertContains(response2, attempt2.expiration_datetime.strftime("%m/%d/%Y"), count=2) def _setup_mode_and_enrollment(self, deadline, enrollment_mode): """Create a course mode and enrollment. @@ -383,7 +383,7 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase): # and fail if none of these are found. found_msg = False for message in self.NOTIFICATION_MESSAGES[status]: - if message in response.content: + if six.b(message) in response.content: found_msg = True break diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index a88c82642d..fe18588917 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -263,11 +263,11 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, set_prerequisite_courses(self.course.id, [six.text_type(self.pre_requisite_course.id)]) response = self.client.get(reverse('dashboard')) - self.assertIn('
', response.content) + self.assertContains(response, '
') remove_prerequisite_course(self.course.id, get_course_milestones(self.course.id)[0]) response = self.client.get(reverse('dashboard')) - self.assertNotIn('
', response.content) + self.assertNotContains(response, '
') @patch('openedx.core.djangoapps.programs.utils.get_programs') @patch('student.views.dashboard.get_visible_sessions_for_entitlement') @@ -301,10 +301,10 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, 'type': 'verified' } response = self.client.get(self.path) - self.assertIn('class="course-target-link enter-course hidden"', response.content) - self.assertIn('You must select a session to access the course.', response.content) - self.assertIn('
', response.content) - self.assertIn('Related Programs:', response.content) + self.assertContains(response, 'class="course-target-link enter-course hidden"') + self.assertContains(response, 'You must select a session to access the course.') + self.assertContains(response, '
') + self.assertContains(response, 'Related Programs:') # If an entitlement has already been redeemed by the user for a course run, do not let the run be selectable enrollment = CourseEnrollmentFactory( @@ -326,9 +326,9 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, response = self.client.get(self.path) # There should be two entitlements on the course page, one prompting for a mandatory session, but no # select option for the courses as there is only the single course run which has already been redeemed - self.assertEqual(response.content.count('
  • '), 2) - self.assertIn('You must select a session to access the course.', response.content) - self.assertNotIn('To access the course, select a session.', response.content) + self.assertContains(response, '
  • ', count=2) + self.assertContains(response, 'You must select a session to access the course.') + self.assertNotContains(response, 'To access the course, select a session.') @patch('student.views.dashboard.get_visible_sessions_for_entitlement') @patch.object(CourseOverview, 'get_from_id') @@ -354,7 +354,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, } ] response = self.client.get(self.path) - self.assertEqual(response.content.count('
  • '), 0) + self.assertNotContains(response, '
  • ') @patch('entitlements.api.v1.views.get_course_runs_for_course') @patch.object(CourseOverview, 'get_from_id') @@ -454,9 +454,9 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, program['courses'][0]['uuid'] = entitlement.course_uuid mock_get_programs.return_value = [program] response = self.client.get(self.path) - self.assertEqual(response.content.count('
  • '), 1) - self.assertIn('