In the middle of addressing pull request comments. This is a safety commit in…

In the middle of addressing pull request comments. This is a safety commit in case I have to revert some changes I'm about to make.

common/lib/xmodule/xmodule/modulestore/tests/django_utils.py

@@ -14,10 +14,16 @@ class ModuleStoreTestCase(TestCase):
     collection with templates before running the TestCase
     and drops it they are finished. """
 
-    def update_course(self, course, data):
+    @staticmethod
+    def update_course(course, data):
         """
         Updates the version of course in the mongo modulestore
         with the metadata in data and returns the updated version.
+
+        'course' is an instance of CourseDescriptor for which we want
+        to update metadata.
+
+        'data' is a dictionary with an entry for each CourseField we want to update.
         """
 
         store = xmodule.modulestore.django.modulestore()

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -60,8 +60,8 @@ class XModuleCourseFactory(Factory):
         if data is not None:
             store.update_item(new_course.location, data)
 
-        '''update_item updates the the course as it exists in the modulestore, but doesn't
-        update the instance we are working with, so have to refetch the course after updating it.'''
+        #update_item updates the the course as it exists in the modulestore, but doesn't
+        #update the instance we are working with, so have to refetch the course after updating it.
         new_course = store.get_instance(new_course.id, new_course.location)
 
         return new_course
@@ -152,8 +152,8 @@ class XModuleItemFactory(Factory):
         if new_item.location.category not in DETACHED_CATEGORIES:
             store.update_children(parent_location, parent.children + [new_item.location.url()])
 
-        '''update_children updates the the item as it exists in the modulestore, but doesn't
-        update the instance we are working with, so have to refetch the item after updating it.'''
+        #update_children updates the the item as it exists in the modulestore, but doesn't
+        #update the instance we are working with, so have to refetch the item after updating it.
         new_item = store.get_item(new_item.location)
 
         return new_item

lms/djangoapps/courseware/tests/check_request_code.py

-
-
-def check_for_get_code(code, url):
-        """
-        Check that we got the expected code when accessing url via GET.
-        Returns 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(code, url, data={}):
-        """
-        Check that we got the expected code when accessing url via POST.
-        Returns 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

lms/djangoapps/courseware/tests/helpers.py

@@ -12,7 +12,12 @@ 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.
+
+        `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,
@@ -25,7 +30,11 @@ 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.
+        `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,
@@ -62,8 +71,8 @@ class LoginEnrollmentTestCase(TestCase):
 
     def logout(self):
         """
-        Logout, check that it worked.
-        Returns an HTTP response which e
+        Logout; check that the HTTP response code indicates redirection
+        as expected.
         """
         resp = self.client.get(reverse('logout'), {})
         # should redirect
@@ -106,8 +115,8 @@ class LoginEnrollmentTestCase(TestCase):
     def enroll(self, course, verify=False):
         """
         Try to enroll and return boolean indicating result.
-        'course' is an instance of CourseDescriptor.
-        'verify' is an optional parameter specifying whether we
+        `course` is an instance of CourseDescriptor.
+        `verify` is an optional parameter specifying whether we
         want to verify that the student was successfully enrolled
         in the course.
         """
@@ -122,20 +131,12 @@ class LoginEnrollmentTestCase(TestCase):
             self.assertTrue(result)
         return result
 
-    # def enroll(self, course):
-    #     """
-    #     Enroll the currently logged-in user, and check that it worked.
-    #     """
-
-    #     result = self.try_enroll(course)
-    #     self.assertTrue(result)
-
     def unenroll(self, course):
         """
         Unenroll the currently logged-in user, and check that it worked.
-        'course' is an instance of CourseDescriptor.
+        `course` is an instance of CourseDescriptor.
         """
-        resp = self.client.post('/change_enrollment', {
+        resp = self.client.post(reverse('change_enrollment'), {
             'enrollment_action': 'unenroll',
             'course_id': course.id,
         })

lms/djangoapps/courseware/tests/modulestore_config.py

@@ -4,11 +4,11 @@ from django.conf import settings
 
 
 def mongo_store_config(data_dir):
-    '''
-    Defines default module store using MongoModuleStore
+    """
+    Defines default module store using MongoModuleStore.
 
-    Use of this config requires mongo to be running
-    '''
+    Use of this config requires mongo to be running.
+    """
     store = {
         'default': {
             'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
@@ -27,7 +27,9 @@ def mongo_store_config(data_dir):
 
 
 def draft_mongo_store_config(data_dir):
-    '''Defines default module store using DraftMongoModuleStore'''
+    """
+    Defines default module store using DraftMongoModuleStore.
+    """
     return {
         'default': {
             'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
@@ -55,7 +57,9 @@ def draft_mongo_store_config(data_dir):
 
 
 def xml_store_config(data_dir):
-    '''Defines default module store using XMLModuleStore'''
+    """
+    Defines default module store using XMLModuleStore.
+    """
     return {
         'default': {
             'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',

lms/djangoapps/courseware/tests/test_navigation.py

@@ -36,15 +36,18 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         # Create student accounts and activate them.
         for i in range(len(self.STUDENT_INFO)):
-            self.create_account('u{0}'.format(i), self.STUDENT_INFO[i][0], self.STUDENT_INFO[i][1])
-            self.activate_user(self.STUDENT_INFO[i][0])
+            email, password = self.STUDENT_INFO[i]
+            username = 'u{0}'.format(i)
+            self.create_account(username, email, password)
+            self.activate_user(email)
 
     def test_redirects_first_time(self):
         """
         Verify that the first time we click on the courseware tab we are
         redirected to the 'Welcome' section.
         """
-        self.login(self.STUDENT_INFO[0][0], self.STUDENT_INFO[0][1])
+        email, password = self.STUDENT_INFO[0]
+        self.login(email, password)
         self.enroll(self.course, True)
         self.enroll(self.full, True)
 
@@ -61,7 +64,8 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
         Verify the accordion remembers we've already visited the Welcome section
         and redirects correpondingly.
         """
-        self.login(self.STUDENT_INFO[0][0], self.STUDENT_INFO[0][1])
+        email, password = self.STUDENT_INFO[0]
+        self.login(email, password)
         self.enroll(self.course, True)
         self.enroll(self.full, True)
 
@@ -80,8 +84,8 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """
         Verify the accordion remembers which chapter you were last viewing.
         """
-
-        self.login(self.STUDENT_INFO[0][0], self.STUDENT_INFO[0][1])
+        email, password = self.STUDENT_INFO[0]
+        self.login(email, password)
         self.enroll(self.course, True)
         self.enroll(self.full, True)
 

lms/djangoapps/courseware/tests/test_view_authentication.py

@@ -32,28 +32,40 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
     @classmethod
     def _instructor_urls(self, course):
         """
-        List of urls that only instructors/staff should be able to see.
+        `course` is an instance of CourseDescriptor whose section URLs are to be returned.
+
+        Returns a list of URLs corresponding to sections in the passed in course.
         """
+
         urls = [reverse(name, kwargs={'course_id': course.id}) for name in (
             'instructor_dashboard',
             'gradebook',
             'grade_summary',)]
 
+        email, _ = self.ACCOUNT_INFO[0]
+        student_id = User.objects.get(email=email).id
+
         urls.append(reverse('student_progress',
                             kwargs={'course_id': course.id,
-                                    'student_id': User.objects.get(email=self.ACCOUNT_INFO[0][0]).id}))
+                                    'student_id': student_id}))
         return urls
 
     @staticmethod
     def _reverse_urls(names, course):
         """
         Reverse a list of course urls.
+
+        `names` is a list of URL names that correspond to sections in a course.
+
+        `course` is the instance of CourseDescriptor whose section URLs are to be returned.
+
+        Returns a list URLs corresponding to section in the passed in course.
+
         """
         return [reverse(name, kwargs={'course_id': course.id})
                 for name in names]
 
     def setUp(self):
-        xmodule.modulestore.django._MODULESTORES = {}
 
         self.full = CourseFactory.create(number='666', display_name='Robot_Sub_Course')
         self.course = CourseFactory.create()
@@ -68,8 +80,9 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         # Create two accounts and activate them.
         for i in range(len(self.ACCOUNT_INFO)):
-            self.create_account('u{0}'.format(i), self.ACCOUNT_INFO[i][0], self.ACCOUNT_INFO[i][1])
-            self.activate_user(self.ACCOUNT_INFO[i][0])
+            username, email, password = 'u{0}'.format(i), self.ACCOUNT_INFO[i][0], self.ACCOUNT_INFO[i][1]
+            self.create_account(username, email, password)
+            self.activate_user(email)
 
     def test_redirection_unenrolled(self):
         """
@@ -77,7 +90,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         instead of the 'Welcome' section after clicking on the courseware tab.
         """
 
-        self.login(self.ACCOUNT_INFO[0][0], self.ACCOUNT_INFO[0][1])
+        email, password = self.ACCOUNT_INFO[0]
+        self.login(email, password)
         response = self.client.get(reverse('courseware',
                                            kwargs={'course_id': self.course.id}))
         self.assertRedirects(response,
@@ -90,7 +104,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         the chapter after clicking on the courseware tab.
         """
 
-        self.login(self.ACCOUNT_INFO[0][0], self.ACCOUNT_INFO[0][1])
+        email, password = self.ACCOUNT_INFO[0]
+        self.login(email, password)
         self.enroll(self.course)
 
         response = self.client.get(reverse('courseware',
@@ -108,7 +123,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         dashboard, the grade views, and student profile pages.
         """
 
-        self.login(self.ACCOUNT_INFO[0][0], self.ACCOUNT_INFO[0][1])
+        email, password = self.ACCOUNT_INFO[0]
+        self.login(email, password)
 
         self.enroll(self.course)
         self.enroll(self.full)
@@ -127,12 +143,14 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         and student profile pages for their course.
         """
 
+        email, password = self.ACCOUNT_INFO[1]
+
         # Make the instructor staff in self.course
         group_name = _course_staff_group_name(self.course.location)
         group = Group.objects.create(name=group_name)
-        group.user_set.add(User.objects.get(email=self.ACCOUNT_INFO[1][0]))
+        group.user_set.add(User.objects.get(email=email))
 
-        self.login(self.ACCOUNT_INFO[1][0], self.ACCOUNT_INFO[1][1])
+        self.login(email, password)
 
         # Now should be able to get to self.course, but not  self.full
         url = random.choice(self._instructor_urls(self.course))
@@ -149,10 +167,11 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         staff permissions.
         """
 
-        self.login(self.ACCOUNT_INFO[1][0], self.ACCOUNT_INFO[1][1])
+        email, password = self.ACCOUNT_INFO[1]
+        self.login(email, password)
 
         # now make the instructor also staff
-        instructor = User.objects.get(email=self.ACCOUNT_INFO[1][0])
+        instructor = User.objects.get(email=email)
         instructor.is_staff = True
         instructor.save()
 
@@ -202,6 +221,9 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         Actually do the test, relying on settings to be right.
         """
 
+        student_email, student_password = self.ACCOUNT_INFO[0]
+        instructor_email, instructor_password = self.ACCOUNT_INFO[1]
+
         # Make courses start in the future
         now = datetime.datetime.now(pytz.UTC)
         tomorrow = now + datetime.timedelta(days=1)
@@ -300,7 +322,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         # First, try with an enrolled student
         print '=== Testing student access....'
-        self.login(self.ACCOUNT_INFO[0][0], self.ACCOUNT_INFO[0][1])
+        self.login(student_email, student_password)
         self.enroll(self.course, True)
         self.enroll(self.full, True)
 
@@ -314,10 +336,10 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         # Make the instructor staff in  self.course
         group_name = _course_staff_group_name(self.course.location)
         group = Group.objects.create(name=group_name)
-        group.user_set.add(User.objects.get(email=self.ACCOUNT_INFO[1][0]))
+        group.user_set.add(User.objects.get(email=instructor_email))
 
         self.logout()
-        self.login(self.ACCOUNT_INFO[1][0], self.ACCOUNT_INFO[1][1])
+        self.login(instructor_email, instructor_password)
         # Enroll in the classes---can't see courseware otherwise.
         self.enroll(self.course, True)
         self.enroll(self.full, True)
@@ -329,7 +351,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         print '=== Testing staff access....'
         # now also make the instructor staff
-        instructor = User.objects.get(email=self.ACCOUNT_INFO[1][0])
+        instructor = User.objects.get(email=instructor_email)
         instructor.is_staff = True
         instructor.save()
 
@@ -342,6 +364,9 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         Actually do the test, relying on settings to be right.
         """
 
+        student_email, student_password = self.ACCOUNT_INFO[0]
+        instructor_email, instructor_password = self.ACCOUNT_INFO[1]
+
         # Make courses start in the future
         now = datetime.datetime.now(pytz.UTC)
         tomorrow = now + datetime.timedelta(days=1)
@@ -360,7 +385,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         print "login"
         # First, try with an enrolled student
         print '=== Testing student access....'
-        self.login(self.ACCOUNT_INFO[0][0], self.ACCOUNT_INFO[0][1])
+        self.login(student_email, student_password)
         self.assertFalse(self.enroll(self.course))
         self.assertTrue(self.enroll(self.full))
 
@@ -368,18 +393,18 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         # Make the instructor staff in the self.course
         group_name = _course_staff_group_name(self.course.location)
         group = Group.objects.create(name=group_name)
-        group.user_set.add(User.objects.get(email=self.ACCOUNT_INFO[1][0]))
+        group.user_set.add(User.objects.get(email=instructor_email))
 
         print "logout/login"
         self.logout()
-        self.login(self.ACCOUNT_INFO[1][0], self.ACCOUNT_INFO[1][1])
+        self.login(instructor_email, instructor_password)
         print "Instructor should be able to enroll in self.course"
         self.assertTrue(self.enroll(self.course))
 
         print '=== Testing staff access....'
         # now make the instructor global staff, but not in the instructor group
-        group.user_set.remove(User.objects.get(email=self.ACCOUNT_INFO[1][0]))
-        instructor = User.objects.get(email=self.ACCOUNT_INFO[1][0])
+        group.user_set.remove(User.objects.get(email=instructor_email))
+        instructor = User.objects.get(email=instructor_email)
         instructor.is_staff = True
         instructor.save()
 
@@ -392,6 +417,9 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         Actually test beta periods, relying on settings to be right.
         """
 
+        student_email, student_password = self.ACCOUNT_INFO[0]
+        instructor_email, instructor_password = self.ACCOUNT_INFO[1]
+
         # trust, but verify :)
         self.assertFalse(settings.MITX_FEATURES['DISABLE_START_DATES'])
 
@@ -408,7 +436,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.course.lms.days_early_for_beta = 2
 
         # student user shouldn't see it
-        student_user = User.objects.get(email=self.ACCOUNT_INFO[0][0])
+        student_user = User.objects.get(email=student_email)
         self.assertFalse(has_access(student_user, self.course, 'load'))
 
         # now add the student to the beta test group