From 5457775cfe426b6d6a39706930281ea2cdad20bf Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 1 Aug 2014 14:17:09 -0400 Subject: [PATCH] Replace `check_for_get_code` with `assert_request_status_code` More flexible, uses the word "assert", method instead of function --- lms/djangoapps/courseware/tests/helpers.py | 67 ++++++---------- .../courseware/tests/test_navigation.py | 78 ++++++++++--------- .../tests/test_view_authentication.py | 46 ++++++----- lms/djangoapps/open_ended_grading/tests.py | 14 ++-- 4 files changed, 97 insertions(+), 108 deletions(-) diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 8c76b6844f..a36a76cf30 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -9,41 +9,6 @@ from student.models import Registration from django.test import TestCase -def check_for_get_code(self, code, url): - """ - Check that we got the expected code when accessing url via GET. - Returns the HTTP response. - - `self` is a class that subclasses TestCase. - - `code` is a status code for HTTP responses. - - `url` is a url pattern for which we have to test the response. - """ - resp = self.client.get(url) - self.assertEqual(resp.status_code, code, - "got code %d for url '%s'. Expected code %d" - % (resp.status_code, url, code)) - return resp - - -def check_for_post_code(self, code, url, data={}): - """ - Check that we got the expected code when accessing url via POST. - Returns the HTTP response. - `self` is a class that subclasses TestCase. - - `code` is a status code for HTTP responses. - - `url` is a url pattern for which we want to test the response. - """ - resp = self.client.post(url, data) - self.assertEqual(resp.status_code, code, - "got code %d for url '%s'. Expected code %d" - % (resp.status_code, url, code)) - return resp - - def get_request_for_user(user): """Create a request object for user.""" @@ -72,6 +37,19 @@ class LoginEnrollmentTestCase(TestCase): self.activate_user(self.email) self.login(self.email, self.password) + def assert_request_status_code(self, status_code, url, method="GET", **kwargs): + make_request = getattr(self.client, method.lower()) + response = make_request(url, **kwargs) + self.assertEqual( + response.status_code, status_code, + "{method} request to {url} returned status code {actual}, " + "expected status code {expected}".format( + method=method, url=url, + actual=response.status_code, expected=status_code + ) + ) + return response + # ============ User creation and login ============== def login(self, email, password): @@ -90,20 +68,22 @@ class LoginEnrollmentTestCase(TestCase): as expected. """ # should redirect - check_for_get_code(self, 302, reverse('logout')) + self.assert_request_status_code(302, reverse('logout')) def create_account(self, username, email, password): """ Create the account and check that it worked. """ - resp = check_for_post_code(self, 200, reverse('create_account'), { + url = reverse('create_account') + request_data = { 'username': username, 'email': email, 'password': password, 'name': 'username', 'terms_of_service': 'true', 'honor_code': 'true', - }) + } + resp = self.assert_request_status_code(200, url, method="POST", data=request_data) data = json.loads(resp.content) self.assertEqual(data['success'], True) # Check both that the user is created, and inactive @@ -118,7 +98,8 @@ class LoginEnrollmentTestCase(TestCase): """ activation_key = Registration.objects.get(user__email=email).activation_key # and now we try to activate - check_for_get_code(self, 200, reverse('activate', kwargs={'key': activation_key})) + url = reverse('activate', kwargs={'key': activation_key}) + self.assert_request_status_code(200, url) # Now make sure that the user is now actually activated self.assertTrue(User.objects.get(email=email).is_active) @@ -144,7 +125,9 @@ class LoginEnrollmentTestCase(TestCase): Unenroll the currently logged-in user, and check that it worked. `course` is an instance of CourseDescriptor. """ - check_for_post_code(self, 200, reverse('change_enrollment'), { + url = reverse('change_enrollment') + request_data = { 'enrollment_action': 'unenroll', - 'course_id': course.id.to_deprecated_string() - }) + 'course_id': course.id.to_deprecated_string(), + } + self.assert_request_status_code(200, url, method="POST", data=request_data) diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index aac89f5324..82078e0f86 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -11,7 +11,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code +from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.factories import GlobalStaffFactory @@ -70,14 +70,14 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): self.staff_user = GlobalStaffFactory() def assertTabActive(self, tabname, response): - ''' Check if the progress tab is active in the tab set ''' + ''' Check if the progress tab is active in the tab set ''' for line in response.content.split('\n'): if tabname in line and 'active' in line: return raise AssertionError("assertTabActive failed: "+tabname+" not active") def assertTabInactive(self, tabname, response): - ''' Check if the progress tab is active in the tab set ''' + ''' Check if the progress tab is active in the tab set ''' for line in response.content.split('\n'): if tabname in line and 'active' in line: raise AssertionError("assertTabInactive failed: "+tabname+" active") @@ -85,7 +85,7 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): def test_chrome_settings(self): ''' - Test settings for disabling and modifying navigation chrome in the courseware: + Test settings for disabling and modifying navigation chrome in the courseware: - Accordion enabled, or disabled - Navigation tabs enabled, disabled, or redirected ''' @@ -108,7 +108,7 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): })) self.assertEquals('open_close_accordion' in response.content, accordion) self.assertEquals('course-tabs' in response.content, tabs) - + self.assertTabInactive('progress', response) self.assertTabActive('courseware', response) @@ -179,13 +179,14 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): resp = self.client.get(reverse('courseware', kwargs={'course_id': self.course.id.to_deprecated_string()})) - self.assertRedirects(resp, reverse( + redirect_url = reverse( 'courseware_chapter', kwargs={ 'course_id': self.course.id.to_deprecated_string(), 'chapter': 'Overview' } - )) + ) + self.assertRedirects(resp, redirect_url) def test_accordion_state(self): """ @@ -197,22 +198,31 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): self.enroll(self.test_course, True) # Now we directly navigate to a section in a chapter other than 'Overview'. - check_for_get_code(self, 200, reverse( + url = reverse( 'courseware_section', kwargs={ 'course_id': self.course.id.to_deprecated_string(), 'chapter': 'factory_chapter', 'section': 'factory_section' } - )) + ) + self.assert_request_status_code(200, url) # And now hitting the courseware tab should redirect to 'factory_chapter' - resp = self.client.get(reverse('courseware', - kwargs={'course_id': self.course.id.to_deprecated_string()})) + url = reverse( + 'courseware', + kwargs={'course_id': self.course.id.to_deprecated_string()} + ) + resp = self.client.get(url) - self.assertRedirects(resp, reverse('courseware_chapter', - kwargs={'course_id': self.course.id.to_deprecated_string(), - 'chapter': 'factory_chapter'})) + redirect_url = reverse( + 'courseware_chapter', + kwargs={ + 'course_id': self.course.id.to_deprecated_string(), + 'chapter': 'factory_chapter', + } + ) + self.assertRedirects(resp, redirect_url) def test_incomplete_course(self): email = self.staff_user.email @@ -222,46 +232,38 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase): test_course_id = self.test_course.id.to_deprecated_string() - check_for_get_code( - self, 200, - reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} ) + self.assert_request_status_code(200, url) section = ItemFactory.create( parent_location=self.test_course.location, display_name='New Section' ) - check_for_get_code( - self, 200, - reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} ) + self.assert_request_status_code(200, url) subsection = ItemFactory.create( parent_location=section.location, display_name='New Subsection' ) - check_for_get_code( - self, 200, - reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} ) + self.assert_request_status_code(200, url) ItemFactory.create( parent_location=subsection.location, display_name='New Unit' ) - check_for_get_code( - self, 302, - reverse( - 'courseware', - kwargs={'course_id': test_course_id} - ) + url = reverse( + 'courseware', + kwargs={'course_id': test_course_id} ) + self.assert_request_status_code(302, url) diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index 92ebaf6867..de871a9fb2 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/lms/djangoapps/courseware/tests/test_view_authentication.py @@ -15,7 +15,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory -from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code +from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.tests.factories import ( BetaTesterFactory, @@ -60,7 +60,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): urls = [reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()}), reverse('courses')] for url in urls: - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) def _check_non_staff_dark(self, course): """ @@ -75,7 +75,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): for index, book in enumerate(course.textbooks) ]) for url in urls: - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def _check_staff(self, course): """ @@ -89,7 +89,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): for index in xrange(len(course.textbooks)) ]) for url in urls: - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) # The student progress tab is not accessible to a student # before launch, so the instructor view-as-student feature @@ -97,14 +97,18 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): # TODO (vshnayder): If this is not the behavior we want, will need # to make access checking smarter and understand both the effective # user (the student), and the requesting user (the prof) - url = reverse('student_progress', - kwargs={'course_id': course.id.to_deprecated_string(), - 'student_id': self.enrolled_user.id}) - check_for_get_code(self, 404, url) + url = reverse( + 'student_progress', + kwargs={ + 'course_id': course.id.to_deprecated_string(), + 'student_id': self.enrolled_user.id, + } + ) + self.assert_request_status_code(404, url) # The courseware url should redirect, not 200 url = self._reverse_urls(['courseware'], course)[0] - check_for_get_code(self, 302, url) + self.assert_request_status_code(302, url) def login(self, user): return super(TestViewAuth, self).login(user.email, 'test') @@ -212,7 +216,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): # Shouldn't be able to get to the instructor pages for url in urls: - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def test_staff_course_access(self): """ @@ -223,10 +227,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): # Now should be able to get to self.course, but not self.test_course url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.test_course.id.to_deprecated_string()}) - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def test_instructor_course_access(self): """ @@ -237,10 +241,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): # Now should be able to get to self.course, but not self.test_course url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.test_course.id.to_deprecated_string()}) - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def test_org_staff_access(self): """ @@ -249,13 +253,13 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): """ self.login(self.org_staff_user) url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.test_course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.other_org_course.id.to_deprecated_string()}) - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def test_org_instructor_access(self): """ @@ -264,13 +268,13 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): """ self.login(self.org_instructor_user) url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.test_course.id.to_deprecated_string()}) - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) url = reverse('instructor_dashboard', kwargs={'course_id': self.other_org_course.id.to_deprecated_string()}) - check_for_get_code(self, 404, url) + self.assert_request_status_code(404, url) def test_global_staff_access(self): """ @@ -283,7 +287,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): reverse('instructor_dashboard', kwargs={'course_id': self.test_course.id.to_deprecated_string()})] for url in urls: - check_for_get_code(self, 200, url) + self.assert_request_status_code(200, url) @patch.dict('courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_dark_launch_enrolled_student(self): diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 9403ca93e1..84e5f88693 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -24,7 +24,7 @@ from xmodule.open_ended_grading_classes import peer_grading_service, controller_ from xmodule.tests import test_util_open_ended from courseware.tests import factories -from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code, check_for_post_code +from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from lms.lib.xblock.runtime import LmsModuleSystem from student.roles import CourseStaffRole @@ -133,8 +133,8 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): # both get and post should return 404 for view_name in ('staff_grading_get_next', 'staff_grading_save_grade'): url = reverse(view_name, kwargs={'course_id': self.course_id.to_deprecated_string()}) - check_for_get_code(self, 404, url) - check_for_post_code(self, 404, url) + self.assert_request_status_code(404, url, method="GET") + self.assert_request_status_code(404, url, method="POST") def test_get_next(self): self.login(self.instructor, self.password) @@ -142,7 +142,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse('staff_grading_get_next', kwargs={'course_id': self.course_id.to_deprecated_string()}) data = {'location': self.location_string} - response = check_for_post_code(self, 200, url, data) + response = self.assert_request_status_code(200, url, method="POST", data=data) content = json.loads(response.content) @@ -171,7 +171,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): if skip: data.update({'skipped': True}) - response = check_for_post_code(self, 200, url, data) + response = self.assert_request_status_code(200, url, method="POST", data=data) content = json.loads(response.content) self.assertTrue(content['success'], str(content)) self.assertEquals(content['submission_id'], self.mock_service.cnt) @@ -188,7 +188,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse('staff_grading_get_problem_list', kwargs={'course_id': self.course_id.to_deprecated_string()}) data = {} - response = check_for_post_code(self, 200, url, data) + response = self.assert_request_status_code(200, url, method="POST", data=data) content = json.loads(response.content) self.assertTrue(content['success']) @@ -237,7 +237,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase): (staff_grading_service.MAX_ALLOWED_FEEDBACK_LENGTH / len(feedback_fragment) + 1) ) - response = check_for_post_code(self, 200, url, data) + response = self.assert_request_status_code(200, url, method="POST", data=data) content = json.loads(response.content) # Should not succeed.