diff --git a/cms/djangoapps/contentstore/features/transcripts.py b/cms/djangoapps/contentstore/features/transcripts.py index 5f94ec90e5414d3248c771f1eaa72275403653a1..e9e25958aa31f4489b17e26e1623c01ed7380788 100644 --- a/cms/djangoapps/contentstore/features/transcripts.py +++ b/cms/djangoapps/contentstore/features/transcripts.py @@ -173,9 +173,9 @@ def remove_transcripts_from_store(_step, subs_id): try: content = contentstore().find(content_location) contentstore().delete(content.location) - print('Transcript file was removed from store.') + print 'Transcript file was removed from store.' except NotFoundError: - print('Transcript file was NOT found and not removed.') + print 'Transcript file was NOT found and not removed.' @step('I enter a "([^"]+)" source to field number (\d+)$') diff --git a/cms/djangoapps/contentstore/management/commands/clone_course.py b/cms/djangoapps/contentstore/management/commands/clone_course.py index f6df8bae4217d865acf90ef2febf35cd4efd7794..bbd94e52ebc9355cc189e98899fa006601bbda8d 100644 --- a/cms/djangoapps/contentstore/management/commands/clone_course.py +++ b/cms/djangoapps/contentstore/management/commands/clone_course.py @@ -36,11 +36,11 @@ class Command(BaseCommand): mstore = modulestore() - print("Cloning course {0} to {1}".format(source_course_id, dest_course_id)) + print "Cloning course {0} to {1}".format(source_course_id, dest_course_id) with mstore.bulk_operations(dest_course_id): if mstore.clone_course(source_course_id, dest_course_id, ModuleStoreEnum.UserID.mgmt_command): - print("copying User permissions...") + print "copying User permissions..." # purposely avoids auth.add_user b/c it doesn't have a caller to authorize CourseInstructorRole(dest_course_id).add_users( *CourseInstructorRole(source_course_id).users_with_role() diff --git a/cms/djangoapps/contentstore/management/commands/export_all_courses.py b/cms/djangoapps/contentstore/management/commands/export_all_courses.py index 205a9b4233d6e55ee275d7cbeb705e5e126a633e..c5ab34c421f3796a6c5511df0ff61a6c145168a3 100644 --- a/cms/djangoapps/contentstore/management/commands/export_all_courses.py +++ b/cms/djangoapps/contentstore/management/commands/export_all_courses.py @@ -23,13 +23,13 @@ class Command(BaseCommand): output_path = args[0] courses, failed_export_courses = export_courses_to_output_path(output_path) - print("=" * 80) - print(u"=" * 30 + u"> Export summary") - print(u"Total number of courses to export: {0}".format(len(courses))) - print(u"Total number of courses which failed to export: {0}".format(len(failed_export_courses))) - print(u"List of export failed courses ids:") - print(u"\n".join(failed_export_courses)) - print("=" * 80) + print "=" * 80 + print u"=" * 30 + u"> Export summary" + print u"Total number of courses to export: {0}".format(len(courses)) + print u"Total number of courses which failed to export: {0}".format(len(failed_export_courses)) + print u"List of export failed courses ids:" + print u"\n".join(failed_export_courses) + print "=" * 80 def export_courses_to_output_path(output_path): @@ -45,15 +45,15 @@ def export_courses_to_output_path(output_path): failed_export_courses = [] for course_id in course_ids: - print(u"-" * 80) - print(u"Exporting course id = {0} to {1}".format(course_id, output_path)) + print u"-" * 80 + print u"Exporting course id = {0} to {1}".format(course_id, output_path) try: course_dir = course_id.to_deprecated_string().replace('/', '...') export_course_to_xml(module_store, content_store, course_id, root_dir, course_dir) except Exception as err: # pylint: disable=broad-except failed_export_courses.append(unicode(course_id)) - print(u"=" * 30 + u"> Oops, failed to export {0}".format(course_id)) - print(u"Error:") - print(err) + print u"=" * 30 + u"> Oops, failed to export {0}".format(course_id) + print u"Error:" + print err return courses, failed_export_courses diff --git a/cms/djangoapps/contentstore/management/commands/export_convert_format.py b/cms/djangoapps/contentstore/management/commands/export_convert_format.py index 926ab69d0ace17b68077a977bbca108d7f7ee5fd..eb0cc575d9ec6a5b03d5df9d1921a52fdc253b4a 100644 --- a/cms/djangoapps/contentstore/management/commands/export_convert_format.py +++ b/cms/djangoapps/contentstore/management/commands/export_convert_format.py @@ -54,7 +54,7 @@ class Command(BaseCommand): finally: tar_file.close() - print("Created archive {0}".format(archive_name)) + print "Created archive {0}".format(archive_name) except ValueError as err: raise CommandError(err) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 710351ae4c1da8c201270e60341b795c6071347b..2a2f267b3ab3f00a15fdb37594bb420b3fee4e11 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -111,7 +111,7 @@ class AuthTestCase(ContentStoreTestCase): reverse('signup'), ) for page in pages: - print("Checking '{0}'".format(page)) + print "Checking '{0}'".format(page) self.check_page_get(page, 200) def test_create_account_errors(self): @@ -254,17 +254,17 @@ class AuthTestCase(ContentStoreTestCase): self.client = AjaxEnabledTestClient() # Not logged in. Should redirect to login. - print('Not logged in') + print 'Not logged in' for page in auth_pages: - print("Checking '{0}'".format(page)) + print "Checking '{0}'".format(page) self.check_page_get(page, expected=302) # Logged in should work. self.login(self.email, self.pw) - print('Logged in') + print 'Logged in' for page in simple_auth_pages: - print("Checking '{0}'".format(page)) + print "Checking '{0}'".format(page) self.check_page_get(page, expected=200) def test_index_auth(self): diff --git a/common/djangoapps/student/management/commands/transfer_students.py b/common/djangoapps/student/management/commands/transfer_students.py index 8e8557ff8806313818af1d036373e14f903123d8..835d954071bb745bbe4f6ba2269966cd5a682ea6 100644 --- a/common/djangoapps/student/management/commands/transfer_students.py +++ b/common/djangoapps/student/management/commands/transfer_students.py @@ -73,7 +73,7 @@ class Command(TrackedCommand): for user in source_students: with transaction.commit_on_success(): - print("Moving {}.".format(user.username)) + print "Moving {}.".format(user.username) # Find the old enrollment. enrollment = CourseEnrollment.objects.get( user=user, @@ -84,14 +84,14 @@ class Command(TrackedCommand): mode = enrollment.mode old_is_active = enrollment.is_active CourseEnrollment.unenroll(user, source_key, skip_refund=True) - print(u"Unenrolled {} from {}".format(user.username, unicode(source_key))) + print u"Unenrolled {} from {}".format(user.username, unicode(source_key)) for dest_key in dest_keys: if CourseEnrollment.is_enrolled(user, dest_key): # Un Enroll from source course but don't mess # with the enrollment in the destination course. msg = u"Skipping {}, already enrolled in destination course {}" - print(msg.format(user.username, unicode(dest_key))) + print msg.format(user.username, unicode(dest_key)) else: new_enrollment = CourseEnrollment.enroll(user, dest_key, mode=mode) @@ -128,7 +128,7 @@ class Command(TrackedCommand): course_enrollment=enrollment ) except CertificateItem.DoesNotExist: - print(u"No certificate for {}".format(user)) + print u"No certificate for {}".format(user) return certificate_item.course_id = dest_keys[0] diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index d18c76b00172efc76ef3457bb9d3bd3280c2eb04..6791d10af1f4e1ebabec1190d2e2df866e89cc33 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -1090,7 +1090,7 @@ def perform_xlint( for err_log in module_store.errored_courses.itervalues(): for err_log_entry in err_log.errors: msg = err_log_entry[0] - print(msg) + print msg if msg.startswith('ERROR:'): err_cnt += 1 else: @@ -1133,10 +1133,11 @@ def perform_xlint( ) warn_cnt += 1 - print("\n") - print("------------------------------------------") - print("VALIDATION SUMMARY: {err} Errors {warn} Warnings".format( - err=err_cnt, warn=warn_cnt) + print "\n" + print "------------------------------------------" + print "VALIDATION SUMMARY: {err} Errors {warn} Warnings".format( + err=err_cnt, + warn=warn_cnt ) if err_cnt > 0: @@ -1151,7 +1152,7 @@ def perform_xlint( "your courseware before importing" ) else: - print("This course can be imported successfully.") + print "This course can be imported successfully." return err_cnt diff --git a/common/lib/xmodule/xmodule/tests/rendering/__init__.py b/common/lib/xmodule/xmodule/tests/rendering/__init__.py index d93c168c8c58bf5c8c73baa8af1e3c8b5aadc8f1..fc6a0c4d9bf0af7946a7ce8fb0231b9eac561c8d 100644 --- a/common/lib/xmodule/xmodule/tests/rendering/__init__.py +++ b/common/lib/xmodule/xmodule/tests/rendering/__init__.py @@ -1 +1,4 @@ +""" +Package declaration for content assertions test helper module +""" import core diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index 13585b5415d0dda3549395e046782cc8a600880c..b9f361d4b0ea88bb87f8f28d09469092cf422f7f 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -37,7 +37,7 @@ def strip_filenames(descriptor): """ Recursively strips 'filename' from all children's definitions. """ - print("strip filename from {desc}".format(desc=descriptor.location.to_deprecated_string())) + print "strip filename from {desc}".format(desc=descriptor.location.to_deprecated_string()) if descriptor._field_data.has(descriptor, 'filename'): descriptor._field_data.delete(descriptor, 'filename') @@ -98,12 +98,12 @@ class RoundTripTestCase(unittest.TestCase): """).strip() root_dir = path(self.temp_dir) - print("Copying test course to temp dir {0}".format(root_dir)) + print "Copying test course to temp dir {0}".format(root_dir) data_dir = path(DATA_DIR) shutil.copytree(data_dir / course_dir, root_dir / course_dir) - print("Starting import") + print "Starting import" initial_import = XMLModuleStore(root_dir, source_dirs=[course_dir], xblock_mixins=(XModuleMixin,)) courses = initial_import.get_courses() @@ -112,7 +112,7 @@ class RoundTripTestCase(unittest.TestCase): # export to the same directory--that way things like the custom_tags/ folder # will still be there. - print("Starting export") + print "Starting export" file_system = OSFS(root_dir) initial_course.runtime.export_fs = file_system.makeopendir(course_dir) root = lxml.etree.Element('root') @@ -121,14 +121,14 @@ class RoundTripTestCase(unittest.TestCase): with initial_course.runtime.export_fs.open('course.xml', 'w') as course_xml: lxml.etree.ElementTree(root).write(course_xml) - print("Starting second import") + print "Starting second import" second_import = XMLModuleStore(root_dir, source_dirs=[course_dir], xblock_mixins=(XModuleMixin,)) courses2 = second_import.get_courses() self.assertEquals(len(courses2), 1) exported_course = courses2[0] - print("Checking course equality") + print "Checking course equality" # HACK: filenames change when changing file formats # during imports from old-style courses. Ignore them. @@ -139,13 +139,13 @@ class RoundTripTestCase(unittest.TestCase): self.assertEquals(initial_course.id, exported_course.id) course_id = initial_course.id - print("Checking key equality") + print "Checking key equality" self.assertItemsEqual( initial_import.modules[course_id].keys(), second_import.modules[course_id].keys() ) - print("Checking module equality") + print "Checking module equality" for location in initial_import.modules[course_id].keys(): print("Checking", location) self.assertTrue(blocks_are_equivalent( diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index 95a6fb1b5d37a39278709eaa447d4695a864f149..4464470eab9f2f9f9ec2a1a7d3326f1822d5a751 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -218,7 +218,8 @@ class RelativeTimeTest(unittest.TestCase): RelativeTimeTest.delta.to_json(100.0) ) - with self.assertRaisesRegexp(ValueError, "RelativeTime max value is 23:59:59=86400.0 seconds, but 90000.0 seconds is passed"): + error_msg = "RelativeTime max value is 23:59:59=86400.0 seconds, but 90000.0 seconds is passed" + with self.assertRaisesRegexp(ValueError, error_msg): RelativeTimeTest.delta.to_json(datetime.timedelta(seconds=90000)) with self.assertRaises(TypeError): diff --git a/common/lib/xmodule/xmodule/tests/test_imageannotation.py b/common/lib/xmodule/xmodule/tests/test_imageannotation.py index 68a30d21a5548046a11394e756b9e041669433d4..3daaeb3e2ca89d45b8db0a7d9f0d8ec5a15f0f9d 100644 --- a/common/lib/xmodule/xmodule/tests/test_imageannotation.py +++ b/common/lib/xmodule/xmodule/tests/test_imageannotation.py @@ -87,8 +87,19 @@ class ImageAnnotationModuleTestCase(unittest.TestCase): def test_student_view(self): """ - Tests the function that passes in all the information in the context that will be used in templates/textannotation.html + Tests the function that passes in all the information in the context + that will be used in templates/imageannotation.html """ context = self.mod.student_view({}).content - for key in ['display_name', 'instructions_html', 'annotation_storage', 'token', 'tag', 'openseadragonjson', 'default_tab', 'instructor_email', 'annotation_mode', 'is_course_staff']: + + for key in ['display_name', + 'instructions_html', + 'annotation_storage', + 'token', + 'tag', + 'openseadragonjson', + 'default_tab', + 'instructor_email', + 'annotation_mode', + 'is_course_staff']: self.assertIn(key, context) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 0eee605fd7bddafb92540da6098346f6148a668b..1cb42b8d43e64cc0ff29f46dbbfb541fed66d70c 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -66,7 +66,7 @@ class BaseCourseTestCase(unittest.TestCase): def get_course(self, name): """Get a test course by directory name. If there's more than one, error.""" - print("Importing {0}".format(name)) + print "Importing {0}".format(name) modulestore = XMLModuleStore( DATA_DIR, @@ -431,22 +431,22 @@ class ImportTestCase(BaseCourseTestCase): """] for xml_str in yes: - print("should be True for {0}".format(xml_str)) + print "should be True for {0}".format(xml_str) self.assertTrue(is_pointer_tag(etree.fromstring(xml_str))) for xml_str in no: - print("should be False for {0}".format(xml_str)) + print "should be False for {0}".format(xml_str) self.assertFalse(is_pointer_tag(etree.fromstring(xml_str))) def test_metadata_inherit(self): """Make sure that metadata is inherited properly""" - print("Starting import") + print "Starting import" course = self.get_course('toy') def check_for_key(key, node, value): "recursive check for presence of key" - print("Checking {0}".format(node.location.to_deprecated_string())) + print "Checking {0}".format(node.location.to_deprecated_string()) self.assertEqual(getattr(node, key), value) for c in node.get_children(): check_for_key(key, c, value) @@ -496,17 +496,17 @@ class ImportTestCase(BaseCourseTestCase): def test_colon_in_url_name(self): """Ensure that colons in url_names convert to file paths properly""" - print("Starting import") + print "Starting import" # Not using get_courses because we need the modulestore object too afterward modulestore = XMLModuleStore(DATA_DIR, source_dirs=['toy']) courses = modulestore.get_courses() self.assertEquals(len(courses), 1) course = courses[0] - print("course errors:") + print "course errors:" for (msg, err) in modulestore.get_course_errors(course.id): - print(msg) - print(err) + print msg + print err chapters = course.get_children() self.assertEquals(len(chapters), 5) @@ -514,12 +514,12 @@ class ImportTestCase(BaseCourseTestCase): ch2 = chapters[1] self.assertEquals(ch2.url_name, "secret:magic") - print("Ch2 location: ", ch2.location) + print "Ch2 location: ", ch2.location also_ch2 = modulestore.get_item(ch2.location) self.assertEquals(ch2, also_ch2) - print("making sure html loaded") + print "making sure html loaded" loc = course.id.make_usage_key('html', 'secret:toylab') html = modulestore.get_item(loc) self.assertEquals(html.display_name, "Toy lab") @@ -531,13 +531,13 @@ class ImportTestCase(BaseCourseTestCase): loaded because of unicode filenames, there are appropriate exceptions/errors to that effect.""" - print("Starting import") + print "Starting import" modulestore = XMLModuleStore(DATA_DIR, source_dirs=['test_unicode']) courses = modulestore.get_courses() self.assertEquals(len(courses), 1) course = courses[0] - print("course errors:") + print "course errors:" # Expect to find an error/exception about characters in "®esources" expect = "InvalidKeyError" @@ -573,7 +573,7 @@ class ImportTestCase(BaseCourseTestCase): for i in (2, 3): video = sections[i] # Name should be 'video_{hash}' - print("video {0} url_name: {1}".format(i, video.url_name)) + print "video {0} url_name: {1}".format(i, video.url_name) self.assertEqual(len(video.url_name), len('video_') + 12) diff --git a/common/lib/xmodule/xmodule/tests/test_lti_unit.py b/common/lib/xmodule/xmodule/tests/test_lti_unit.py index 052ae31821c944e645b02e8ac8d38f9cae0d99d8..fc77ca7c1e202280aeb65fbec47cd3b34cfbc326 100644 --- a/common/lib/xmodule/xmodule/tests/test_lti_unit.py +++ b/common/lib/xmodule/xmodule/tests/test_lti_unit.py @@ -56,7 +56,9 @@ class LTIModuleTest(LogicTest): self.user_id = self.xmodule.runtime.anonymous_student_id self.lti_id = self.xmodule.lti_id - self.unquoted_resource_link_id = u'{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format(self.xmodule.runtime.hostname) + self.unquoted_resource_link_id = u'{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format( + self.xmodule.runtime.hostname + ) sourced_id = u':'.join(urllib.quote(i) for i in (self.lti_id, self.unquoted_resource_link_id, self.user_id)) @@ -104,7 +106,10 @@ class LTIModuleTest(LogicTest): 'action': action } - @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) + @patch( + 'xmodule.lti_module.LTIModule.get_client_key_secret', + return_value=('test_client_key', u'test_client_secret') + ) def test_authorization_header_not_present(self, _get_key_secret): """ Request has no Authorization header. @@ -125,7 +130,10 @@ class LTIModuleTest(LogicTest): self.assertEqual(response.status_code, 200) self.assertDictEqual(expected_response, real_response) - @patch('xmodule.lti_module.LTIModule.get_client_key_secret', return_value=('test_client_key', u'test_client_secret')) + @patch( + 'xmodule.lti_module.LTIModule.get_client_key_secret', + return_value=('test_client_key', u'test_client_secret') + ) def test_authorization_header_empty(self, _get_key_secret): """ Request Authorization header has no value. @@ -350,7 +358,10 @@ class LTIModuleTest(LogicTest): self.xmodule.get_client_key_secret() @patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=True)) - @patch('xmodule.lti_module.LTIModule.get_client_key_secret', Mock(return_value=('test_client_key', u'test_client_secret'))) + @patch( + 'xmodule.lti_module.LTIModule.get_client_key_secret', + Mock(return_value=('test_client_key', u'test_client_secret')) + ) def test_successful_verify_oauth_body_sign(self): """ Test if OAuth signing was successful. @@ -397,7 +408,7 @@ class LTIModuleTest(LogicTest): '<imsx_POXHeader><imsx_POXRequestHeaderInfo><imsx_version>V1.0</imsx_version>' '<imsx_messageIdentifier>edX_fix</imsx_messageIdentifier></imsx_POXRequestHeaderInfo>' '</imsx_POXHeader><imsx_POXBody><replaceResultRequest><resultRecord><sourcedGUID>' - '<sourcedId>MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131</sourcedId>' + '<sourcedId>MITxLTI/MITxLTI/201x:localhost%3A8000-i4x-MITxLTI-MITxLTI-lti-3751833a214a4f66a0d18f63234207f2:363979ef768ca171b50f9d1bfb322131</sourcedId>' # pylint: disable=line-too-long '</sourcedGUID><result><resultScore><language>en</language><textString>0.32</textString></resultScore>' '</result></resultRecord></replaceResultRequest></imsx_POXBody></imsx_POXEnvelopeRequest>' ) @@ -428,7 +439,10 @@ class LTIModuleTest(LogicTest): self.assertEqual(self.defaults['action'], action) @patch('xmodule.lti_module.signature.verify_hmac_sha1', Mock(return_value=False)) - @patch('xmodule.lti_module.LTIModule.get_client_key_secret', Mock(return_value=('test_client_key', u'test_client_secret'))) + @patch( + 'xmodule.lti_module.LTIModule.get_client_key_secret', + Mock(return_value=('test_client_key', u'test_client_secret')) + ) def test_failed_verify_oauth_body_sign(self): """ Oauth signing verify fail. diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py index 0e895100e823f335abb6c1082521904b91cddddb..39274d193b78fbd7adb894fc3ac6d101f5e984d6 100644 --- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py +++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py @@ -1,3 +1,6 @@ +""" +Test cases covering behaviors and workflows of the Peer Grading XBlock +""" import unittest import json import logging @@ -219,11 +222,15 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): self.assertEqual(score_dict["score"], 0.0) def get_score(self, success, count_graded, count_required): + """ + Returns the peer-graded score based on the provided graded/required values + """ self.peer_grading.use_for_single_location_local = True self.peer_grading.graded = True # Patch for external grading service. - with patch('xmodule.peer_grading_module.PeerGradingModule.query_data_for_location') as mock_query_data_for_location: + module_name = 'xmodule.peer_grading_module.PeerGradingModule.query_data_for_location' + with patch(module_name) as mock_query_data_for_location: mock_query_data_for_location.return_value = ( success, {"count_graded": count_graded, "count_required": count_required} @@ -249,6 +256,9 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore): class MockPeerGradingServiceProblemList(MockPeerGradingService): + """ + Mock object representing a set of peer-grading problems + """ def get_problem_list(self, course_id, grader_id): return {'success': True, 'problem_list': [ @@ -348,6 +358,9 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore): return ScopeIds(None, None, self.problem_location, self.problem_location) def _create_peer_grading_descriptor_with_linked_problem(self): + """ + Internal helper method to construct a peer grading XBlock + """ # Initialize the peer grading module. system = get_test_descriptor_system() diff --git a/common/lib/xmodule/xmodule/tests/test_randomize_module.py b/common/lib/xmodule/xmodule/tests/test_randomize_module.py index 485d3efb153143fb4e90dea728de9f120556775d..798d3028b1651769e3de5029627a20a997a99a45 100644 --- a/common/lib/xmodule/xmodule/tests/test_randomize_module.py +++ b/common/lib/xmodule/xmodule/tests/test_randomize_module.py @@ -1,3 +1,6 @@ +""" +Test cases covering workflows and behaviors for the Randomize XModule +""" import unittest from datetime import datetime, timedelta diff --git a/common/lib/xmodule/xmodule/tests/test_self_assessment.py b/common/lib/xmodule/xmodule/tests/test_self_assessment.py index 3c577473cb5e707274dce6d26fe6a88113344a12..0d64009911cb598767f01c490c591478dfd630e4 100644 --- a/common/lib/xmodule/xmodule/tests/test_self_assessment.py +++ b/common/lib/xmodule/xmodule/tests/test_self_assessment.py @@ -1,3 +1,6 @@ +""" +Test cases covering workflows and behaviors of the Self Assessment feature +""" from datetime import datetime import json import unittest @@ -15,6 +18,9 @@ import test_util_open_ended class SelfAssessmentTest(unittest.TestCase): + """ + Test cases covering workflows and behaviors of the Self Assessment feature + """ rubric = '''<rubric><rubric> <category> <description>Response Quality</description> @@ -78,9 +84,15 @@ class SelfAssessmentTest(unittest.TestCase): responses = {'assessment': '0', 'score_list[]': ['0', '0']} def get_fake_item(name): + """ + Returns the specified key from the parent workflow container + """ return responses[name] def get_data_for_location(self, location, student): + """ + Returns a dictionary of keys having zero values + """ return { 'count_graded': 0, 'count_required': 0, diff --git a/common/lib/xmodule/xmodule/tests/test_split_test_module.py b/common/lib/xmodule/xmodule/tests/test_split_test_module.py index b9adbb2ea57f108bdb5bd23b490cf6346e1cee00..e4b35ec5919dbc51e2fdd3841023b5241a917512 100644 --- a/common/lib/xmodule/xmodule/tests/test_split_test_module.py +++ b/common/lib/xmodule/xmodule/tests/test_split_test_module.py @@ -68,7 +68,7 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): parent=sequence, attribs={ 'user_partition_id': '0', - 'group_id_to_child': '{"0": "i4x://edX/xml_test_course/html/split_test_cond0", "1": "i4x://edX/xml_test_course/html/split_test_cond1"}' + 'group_id_to_child': '{"0": "i4x://edX/xml_test_course/html/split_test_cond0", "1": "i4x://edX/xml_test_course/html/split_test_cond1"}' # pylint: disable=line-too-long } ) xml.HtmlFactory(parent=split_test, url_name='split_test_cond0', text='HTML FOR GROUP 0') @@ -135,7 +135,10 @@ class SplitTestModuleLMSTest(SplitTestModuleTest): # If a user_tag has a missing value, a group should be saved/persisted for that user. # So, we check that we get the same url_name when we call on the url_name twice. # We run the test ten times so that, if our storage is failing, we'll be most likely to notice it. - self.assertEquals(self.split_test_module.child_descriptor.url_name, self.split_test_module.child_descriptor.url_name) + self.assertEquals( + self.split_test_module.child_descriptor.url_name, + self.split_test_module.child_descriptor.url_name + ) # Patch the definition_to_xml for the html children. @patch('xmodule.html_module.HtmlDescriptor.definition_to_xml') diff --git a/common/lib/xmodule/xmodule/tests/test_textannotation.py b/common/lib/xmodule/xmodule/tests/test_textannotation.py index 1aab9dcec7aaf89d1b4129a2c43a396d03d407b5..a2b1f241592dca3424308dad4cf5ff08acf1160d 100644 --- a/common/lib/xmodule/xmodule/tests/test_textannotation.py +++ b/common/lib/xmodule/xmodule/tests/test_textannotation.py @@ -72,8 +72,19 @@ class TextAnnotationModuleTestCase(unittest.TestCase): def test_student_view(self): """ - Tests the function that passes in all the information in the context that will be used in templates/textannotation.html + Tests the function that passes in all the information in the context + that will be used in templates/textannotation.html """ context = self.mod.student_view({}).content - for key in ['display_name', 'tag', 'source', 'instructions_html', 'content_html', 'annotation_storage', 'token', 'diacritic_marks', 'default_tab', 'annotation_mode', 'is_course_staff']: + for key in ['display_name', + 'tag', + 'source', + 'instructions_html', + 'content_html', + 'annotation_storage', + 'token', + 'diacritic_marks', + 'default_tab', + 'annotation_mode', + 'is_course_staff']: self.assertIn(key, context) diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index a2710ba52c245a7881b7cfb7c782f8bdefb79f69..b863d5c22c77ba8ecb8b515c18a77b0bd2eff1d2 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,3 +1,6 @@ +""" +Utility classes and data structures supporting the Open-Ended Grading feature of Open edX +""" import json from StringIO import StringIO @@ -29,15 +32,27 @@ class MockS3Key(object): pass def set_metadata(self, key, value): + """ + Appends an attribute the current instance using the provided key/value pair + """ setattr(self, key, value) def set_contents_from_file(self, fileobject): + """ + Sets the 'data' parameter to the contents of the provided file object + """ self.data = fileobject.read() def set_acl(self, acl): + """ + Sets the 'acl' metadata parameter to the provided value + """ self.set_metadata("acl", acl) def generate_url(self, timeout): + """ + Returns a sample URL for use in tests + """ return "http://www.edx.org/sample_url" @@ -52,9 +67,15 @@ class MockS3Connection(object): pass def create_bucket(self, bucket_name, **kwargs): + """ + Mock boto operation: create_bucket -- returns a simple string value + """ return "edX Bucket" def lookup(self, bucket_name): + """ + Mock boto operation: lookup -- returns None + """ return None @@ -70,9 +91,15 @@ class MockUploadedFile(object): self.name = name def seek(self, index): + """ + Returns the file contents at the provided index + """ return self.mock_file.seek(index) def read(self): + """ + Returns the contents of the mock file + """ return self.mock_file.read() @@ -82,9 +109,15 @@ class DummyModulestore(object): """ def get_module_system(self, descriptor): + """ + Pseudo-abstract method that forces derivatives to devise a module system implementation + """ raise NotImplementedError("Sub-tests must specify how to generate a module-system") def setup_modulestore(self, name): + """ + Sets the modulestore to an XMLModuleStore instance + """ # pylint: disable=attribute-defined-outside-init self.modulestore = XMLModuleStore(DATA_DIR, source_dirs=[name]) @@ -94,6 +127,9 @@ class DummyModulestore(object): return courses[0] def get_module_from_location(self, usage_key): + """ + Returns the content descriptor for the given usage key + """ descriptor = self.modulestore.get_item(usage_key, depth=None) descriptor.xmodule_runtime = self.get_module_system(descriptor) return descriptor @@ -921,6 +957,8 @@ STATE_POST_ASSESSMENT = serialize_open_ended_instance_state(""" "max_attempts": 1 }""") + +# pylint: disable=line-too-long # Task state with self assessment only. TEST_STATE_SA = ["{\"child_created\": false, \"child_attempts\": 1, \"version\": 1, \"child_history\": [{\"answer\": \"Censorship in the Libraries\\r<br>'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author\\r<br><br>Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.\", \"post_assessment\": \"[3, 3, 2, 2, 2]\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"done\"}", "{\"child_created\": false, \"child_attempts\": 0, \"version\": 1, \"child_history\": [{\"answer\": \"Censorship in the Libraries\\r<br>'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author\\r<br><br>Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.\", \"post_assessment\": \"{\\\"submission_id\\\": 1461, \\\"score\\\": 12, \\\"feedback\\\": \\\"{\\\\\\\"feedback\\\\\\\": \\\\\\\"\\\\\\\"}\\\", \\\"success\\\": true, \\\"grader_id\\\": 5414, \\\"grader_type\\\": \\\"IN\\\", \\\"rubric_scores_complete\\\": true, \\\"rubric_xml\\\": \\\"<rubric><category><description>\\\\nIdeas\\\\n</description><score>3</score><option points='0'>\\\\nDifficult for the reader to discern the main idea. Too brief or too repetitive to establish or maintain a focus.\\\\n</option><option points='1'>\\\\nAttempts a main idea. Sometimes loses focus or ineffectively displays focus.\\\\n</option><option points='2'>\\\\nPresents a unifying theme or main idea, but may include minor tangents. Stays somewhat focused on topic and task.\\\\n</option><option points='3'>\\\\nPresents a unifying theme or main idea without going off on tangents. Stays completely focused on topic and task.\\\\n</option></category><category><description>\\\\nContent\\\\n</description><score>3</score><option points='0'>\\\\nIncludes little information with few or no details or unrelated details. Unsuccessful in attempts to explore any facets of the topic.\\\\n</option><option points='1'>\\\\nIncludes little information and few or no details. Explores only one or two facets of the topic.\\\\n</option><option points='2'>\\\\nIncludes sufficient information and supporting details. (Details may not be fully developed; ideas may be listed.) Explores some facets of the topic.\\\\n</option><option points='3'>\\\\nIncludes in-depth information and exceptional supporting details that are fully developed. Explores all facets of the topic.\\\\n</option></category><category><description>\\\\nOrganization\\\\n</description><score>2</score><option points='0'>\\\\nIdeas organized illogically, transitions weak, and response difficult to follow.\\\\n</option><option points='1'>\\\\nAttempts to logically organize ideas. Attempts to progress in an order that enhances meaning, and demonstrates use of transitions.\\\\n</option><option points='2'>\\\\nIdeas organized logically. Progresses in an order that enhances meaning. Includes smooth transitions.\\\\n</option></category><category><description>\\\\nStyle\\\\n</description><score>2</score><option points='0'>\\\\nContains limited vocabulary, with many words used incorrectly. Demonstrates problems with sentence patterns.\\\\n</option><option points='1'>\\\\nContains basic vocabulary, with words that are predictable and common. Contains mostly simple sentences (although there may be an attempt at more varied sentence patterns).\\\\n</option><option points='2'>\\\\nIncludes vocabulary to make explanations detailed and precise. Includes varied sentence patterns, including complex sentences.\\\\n</option></category><category><description>\\\\nVoice\\\\n</description><score>2</score><option points='0'>\\\\nDemonstrates language and tone that may be inappropriate to task and reader.\\\\n</option><option points='1'>\\\\nDemonstrates an attempt to adjust language and tone to task and reader.\\\\n</option><option points='2'>\\\\nDemonstrates effective adjustment of language and tone to task and reader.\\\\n</option></category></rubric>\\\"}\", \"score\": 12}], \"max_score\": 12, \"child_state\": \"post_assessment\"}"] diff --git a/common/lib/xmodule/xmodule/tests/test_validation.py b/common/lib/xmodule/xmodule/tests/test_validation.py index f3e6b95b500b25962ce3a369dc9f7a4724df8aa9..308abffb9130ca52e86fd0deb798f1e7a498ee7b 100644 --- a/common/lib/xmodule/xmodule/tests/test_validation.py +++ b/common/lib/xmodule/xmodule/tests/test_validation.py @@ -209,10 +209,25 @@ class StudioValidationTest(unittest.TestCase): expected = { "xblock_id": "id", "messages": [ - {"type": "error", "text": u"Error message", "action_label": u"Action label", "action_class": "edit-button"}, - {"type": "not-configured", "text": u"Not configured message", "action_label": u"Action label", "action_runtime_event": "make groups"} + { + "type": "error", + "text": u"Error message", + "action_label": u"Action label", + "action_class": "edit-button" + }, + { + "type": "not-configured", + "text": u"Not configured message", + "action_label": u"Action label", + "action_runtime_event": "make groups" + } ], - "summary": {"type": "warning", "text": u"Summary message", "action_label": u"Summary label", "action_runtime_event": "fix everything"}, + "summary": { + "type": "warning", + "text": u"Summary message", + "action_label": u"Summary label", + "action_runtime_event": "fix everything" + }, "empty": False } self.assertEqual(expected, validation.to_json()) diff --git a/common/lib/xmodule/xmodule/tests/test_videoannotation.py b/common/lib/xmodule/xmodule/tests/test_videoannotation.py index fa6689fc9003becef2fa035b45ba206a10a5bd80..f9cf5f1402eaa8ec85d77619b5aa60f2d13293c5 100644 --- a/common/lib/xmodule/xmodule/tests/test_videoannotation.py +++ b/common/lib/xmodule/xmodule/tests/test_videoannotation.py @@ -83,5 +83,14 @@ class VideoAnnotationModuleTestCase(unittest.TestCase): Tests to make sure variables passed in truly exist within the html once it is all rendered. """ context = self.mod.student_view({}).content - for key in ['display_name', 'instructions_html', 'sourceUrl', 'typeSource', 'poster', 'annotation_storage', 'default_tab', 'instructor_email', 'annotation_mode', "is_course_staff"]: + for key in ['display_name', + 'instructions_html', + 'sourceUrl', + 'typeSource', + 'poster', + 'annotation_storage', + 'default_tab', + 'instructor_email', + 'annotation_mode', + 'is_course_staff']: self.assertIn(key, context) diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index cee5d1c7e9afd5a2c42219c7cfb7c4a8c5d5796a..edf3e5b72e11ec9af3d0ea84e41180292f1a8c7d 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -383,7 +383,8 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat lang, ) except TranscriptException as ex: - item.transcripts.pop(lang) # remove key from transcripts because proper srt file does not exist in assets. + # remove key from transcripts because proper srt file does not exist in assets. + item.transcripts.pop(lang) reraised_message += ' ' + ex.message if reraised_message: item.save_with_metadata(user) diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 6e93accb98967884bed3f828af7bd2a966f18d46..4e7ad7108d2ad603baecb1c5083ecd22a67b00b9 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -185,7 +185,8 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, return track_url, transcript_language, sorted_languages def get_html(self): - transcript_download_format = self.transcript_download_format if not (self.download_track and self.track) else None + track_status = (self.download_track and self.track) + transcript_download_format = self.transcript_download_format if not track_status else None sources = filter(None, self.html5_sources) download_video_link = None @@ -458,7 +459,11 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler languages.sort(key=lambda l: l['label']) editable_fields['transcripts']['languages'] = languages editable_fields['transcripts']['type'] = 'VideoTranslations' - editable_fields['transcripts']['urlRoot'] = self.runtime.handler_url(self, 'studio_transcript', 'translation').rstrip('/?') + editable_fields['transcripts']['urlRoot'] = self.runtime.handler_url( + self, + 'studio_transcript', + 'translation' + ).rstrip('/?') editable_fields['handout']['type'] = 'FileUploader' return editable_fields @@ -568,6 +573,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler youtube_id_1_0 = metadata_fields['youtube_id_1_0'] def get_youtube_link(video_id): + """ + Returns the fully-qualified YouTube URL for the given video identifier + """ # First try a lookup in VAL. If we have a YouTube entry there, it overrides the # one passed in. if self.edx_video_id and edxval_api: @@ -582,7 +590,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler _ = self.runtime.service(self, "i18n").ugettext video_url.update({ - 'help': _('The URL for your video. This can be a YouTube URL or a link to an .mp4, .ogg, or .webm video file hosted elsewhere on the Internet.'), + 'help': _('The URL for your video. This can be a YouTube URL or a link to an .mp4, .ogg, or .webm video file hosted elsewhere on the Internet.'), # pylint: disable=line-too-long 'display_name': _('Default Video URL'), 'field_name': 'video_url', 'type': 'VideoList', diff --git a/common/lib/xmodule/xmodule/video_module/video_xfields.py b/common/lib/xmodule/xmodule/video_module/video_xfields.py index b40b0fd2cf74145adcfeb1df02d3743fa5c89c3b..f494b02e558126e32bf48326d0a5f53374aaf21a 100644 --- a/common/lib/xmodule/xmodule/video_module/video_xfields.py +++ b/common/lib/xmodule/xmodule/video_module/video_xfields.py @@ -82,30 +82,30 @@ class VideoFields(object): default="" ) download_video = Boolean( - help=_("Allow students to download versions of this video in different formats if they cannot use the edX video player or do not have access to YouTube. You must add at least one non-YouTube URL in the Video File URLs field."), + help=_("Allow students to download versions of this video in different formats if they cannot use the edX video player or do not have access to YouTube. You must add at least one non-YouTube URL in the Video File URLs field."), # pylint: disable=line-too-long display_name=_("Video Download Allowed"), scope=Scope.settings, default=False ) html5_sources = List( - help=_("The URL or URLs where you've posted non-YouTube versions of the video. Each URL must end in .mpeg, .mp4, .ogg, or .webm and cannot be a YouTube URL. (For browser compatibility, we strongly recommend .mp4 and .webm format.) Students will be able to view the first listed video that's compatible with the student's computer. To allow students to download these videos, set Video Download Allowed to True."), + help=_("The URL or URLs where you've posted non-YouTube versions of the video. Each URL must end in .mpeg, .mp4, .ogg, or .webm and cannot be a YouTube URL. (For browser compatibility, we strongly recommend .mp4 and .webm format.) Students will be able to view the first listed video that's compatible with the student's computer. To allow students to download these videos, set Video Download Allowed to True."), # pylint: disable=line-too-long display_name=_("Video File URLs"), scope=Scope.settings, ) track = String( - help=_("By default, students can download an .srt or .txt transcript when you set Download Transcript Allowed to True. If you want to provide a downloadable transcript in a different format, we recommend that you upload a handout by using the Upload a Handout field. If this isn't possible, you can post a transcript file on the Files & Uploads page or on the Internet, and then add the URL for the transcript here. Students see a link to download that transcript below the video."), + help=_("By default, students can download an .srt or .txt transcript when you set Download Transcript Allowed to True. If you want to provide a downloadable transcript in a different format, we recommend that you upload a handout by using the Upload a Handout field. If this isn't possible, you can post a transcript file on the Files & Uploads page or on the Internet, and then add the URL for the transcript here. Students see a link to download that transcript below the video."), # pylint: disable=line-too-long display_name=_("Downloadable Transcript URL"), scope=Scope.settings, default='' ) download_track = Boolean( - help=_("Allow students to download the timed transcript. A link to download the file appears below the video. By default, the transcript is an .srt or .txt file. If you want to provide the transcript for download in a different format, upload a file by using the Upload Handout field."), + help=_("Allow students to download the timed transcript. A link to download the file appears below the video. By default, the transcript is an .srt or .txt file. If you want to provide the transcript for download in a different format, upload a file by using the Upload Handout field."), # pylint: disable=line-too-long display_name=_("Download Transcript Allowed"), scope=Scope.settings, default=False ) sub = String( - help=_("The default transcript for the video, from the Default Timed Transcript field on the Basic tab. This transcript should be in English. You don't have to change this setting."), + help=_("The default transcript for the video, from the Default Timed Transcript field on the Basic tab. This transcript should be in English. You don't have to change this setting."), # pylint: disable=line-too-long display_name=_("Default Timed Transcript"), scope=Scope.settings, default="" @@ -118,7 +118,7 @@ class VideoFields(object): ) # Data format: {'de': 'german_translation', 'uk': 'ukrainian_translation'} transcripts = Dict( - help=_("Add transcripts in different languages. Click below to specify a language and upload an .srt transcript file for that language."), + help=_("Add transcripts in different languages. Click below to specify a language and upload an .srt transcript file for that language."), # pylint: disable=line-too-long display_name=_("Transcript Languages"), scope=Scope.settings, default={} @@ -154,7 +154,7 @@ class VideoFields(object): default=True ) handout = String( - help=_("Upload a handout to accompany this video. Students can download the handout by clicking Download Handout under the video."), + help=_("Upload a handout to accompany this video. Students can download the handout by clicking Download Handout under the video."), # pylint: disable=line-too-long display_name=_("Upload Handout"), scope=Scope.settings, ) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index f7921e0f9bbe5943bccb474bb20703e9a9789922..0807a66fa8154f1a3183e23af560db95e7bc2328 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -34,15 +34,27 @@ log = logging.getLogger(__name__) def extract(dic, keys): + """ + Returns a subset of keys from the provided dictionary + """ return {k: dic.get(k) for k in keys} def strip_none(dic): + """ + Returns a dictionary stripped of any keys having values of None + """ return dict([(k, v) for k, v in dic.iteritems() if v is not None]) def strip_blank(dic): + """ + Returns a dictionary stripped of any 'blank' (empty) keys + """ def _is_blank(v): + """ + Determines if the provided value contains no information + """ return isinstance(v, str) and len(v.strip()) == 0 return dict([(k, v) for k, v in dic.iteritems() if not _is_blank(v)]) @@ -50,16 +62,23 @@ def strip_blank(dic): def merge_dict(dic1, dic2): + """ + Combines the keys from the two provided dictionaries + """ return dict(dic1.items() + dic2.items()) def get_role_ids(course_id): + """ + Returns a dictionary having role names as keys and a list of users as values + """ roles = Role.objects.filter(course_id=course_id).exclude(name=FORUM_ROLE_STUDENT) return dict([(role.name, list(role.users.values_list('id', flat=True))) for role in roles]) def has_discussion_privileges(user, course_id): - """Returns True if the user is privileged in teams discussions for + """ + Returns True if the user is privileged in teams discussions for this course. The user must be one of Discussion Admin, Moderator, or Community TA. @@ -79,6 +98,9 @@ def has_discussion_privileges(user, course_id): def has_forum_access(uname, course_id, rolename): + """ + Boolean operation which tests a user's role-based permissions (not actually forums-specific) + """ try: role = Role.objects.get(name=rolename, course_id=course_id) except Role.DoesNotExist: @@ -87,10 +109,17 @@ def has_forum_access(uname, course_id, rolename): def has_required_keys(module): - """Returns True iff module has the proper attributes for generating metadata with get_discussion_id_map_entry()""" + """ + Returns True iff module has the proper attributes for generating metadata + with get_discussion_id_map_entry() + """ for key in ('discussion_id', 'discussion_category', 'discussion_target'): if getattr(module, key, None) is None: - log.debug("Required key '%s' not in discussion %s, leaving out of category map", key, module.location) + log.debug( + "Required key '%s' not in discussion %s, leaving out of category map", + key, + module.location + ) return False return True @@ -170,7 +199,10 @@ def get_discussion_id_map(course, user): def _filter_unstarted_categories(category_map): - + """ + Returns a subset of categories from the provided map which have not yet met the start date + Includes information about category children, subcategories (different), and entries + """ now = datetime.now(UTC()) result_map = {} @@ -208,6 +240,9 @@ def _filter_unstarted_categories(category_map): def _sort_map_entries(category_map, sort_alpha): + """ + Internal helper method to list category entries according to the provided sort order + """ things = [] for title, entry in category_map["entries"].items(): if entry["sort_key"] is None and sort_alpha: @@ -387,14 +422,26 @@ def get_discussion_categories_ids(course, user, include_all=False): class JsonResponse(HttpResponse): + """ + Django response object delivering JSON representations + """ def __init__(self, data=None): + """ + Object constructor, converts data (if provided) to JSON + """ content = json.dumps(data, cls=i4xEncoder) super(JsonResponse, self).__init__(content, mimetype='application/json; charset=utf-8') class JsonError(HttpResponse): + """ + Django response object delivering JSON exceptions + """ def __init__(self, error_messages=[], status=400): + """ + Object constructor, returns an error response containing the provided exception messages + """ if isinstance(error_messages, basestring): error_messages = [error_messages] content = json.dumps({'errors': error_messages}, indent=2, ensure_ascii=False) @@ -403,12 +450,24 @@ class JsonError(HttpResponse): class HtmlResponse(HttpResponse): + """ + Django response object delivering HTML representations + """ def __init__(self, html=''): + """ + Object constructor, brokers provided HTML to caller + """ super(HtmlResponse, self).__init__(html, content_type='text/plain') class ViewNameMiddleware(object): + """ + Django middleware object to inject view name into request context + """ def process_view(self, request, view_func, view_args, view_kwargs): + """ + Injects the view name value into the request context + """ request.view_name = view_func.__name__ @@ -420,6 +479,9 @@ class QueryCountDebugMiddleware(object): multi-db setups. """ def process_response(self, request, response): + """ + Log information for 200 OK responses as part of the outbound pipeline + """ if response.status_code == 200: total_time = 0 @@ -439,6 +501,9 @@ class QueryCountDebugMiddleware(object): def get_ability(course_id, content, user): + """ + Return a dictionary of forums-oriented actions and the user's permission to perform them + """ return { 'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"), 'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment"), @@ -487,6 +552,10 @@ def get_annotated_content_infos(course_id, thread, user, user_info): def get_metadata_for_threads(course_id, threads, user, user_info): + """ + Returns annotated content information for the specified course, threads, and user information + """ + def infogetter(thread): return get_annotated_content_infos(course_id, thread, user, user_info) diff --git a/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py b/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py index 502185d25d82a23de18444e8a243589035710cd6..6fdd1cc86f4dda54f40b4853c7602096a03a0e2a 100644 --- a/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py @@ -28,14 +28,14 @@ class Command(BaseCommand): attempts_to_retry = SoftwareSecurePhotoVerification.objects.filter(status='must_retry') force_must_retry = False - print("Attempting to retry {0} failed PhotoVerification submissions".format(len(attempts_to_retry))) + print "Attempting to retry {0} failed PhotoVerification submissions".format(len(attempts_to_retry)) for index, attempt in enumerate(attempts_to_retry): - print("Retrying submission #{0} (ID: {1}, User: {2})".format(index, attempt.id, attempt.user)) + print "Retrying submission #{0} (ID: {1}, User: {2})".format(index, attempt.id, attempt.user) # Set the attempts status to 'must_retry' so that we can re-submit it if force_must_retry: attempt.status = 'must_retry' attempt.submit(copy_id_photo_from=attempt.copy_id_photo_from) - print("Retry result: {0}".format(attempt.status)) - print("Done resubmitting failed photo verifications") + print "Retry result: {0}".format(attempt.status) + print "Done resubmitting failed photo verifications" diff --git a/openedx/core/djangoapps/content/course_structures/admin.py b/openedx/core/djangoapps/content/course_structures/admin.py index 64bbdad86cbfa74b3eafeb3b20786784984f112d..cdf31c58e8c5adf3182780da19d0972ec82bb6b9 100644 --- a/openedx/core/djangoapps/content/course_structures/admin.py +++ b/openedx/core/djangoapps/content/course_structures/admin.py @@ -1,9 +1,15 @@ +""" +Django Admin model registry for Course Structures sub-application +""" from ratelimitbackend import admin from .models import CourseStructure class CourseStructureAdmin(admin.ModelAdmin): + """ + Django Admin class for managing Course Structures model data + """ search_fields = ('course_id',) list_display = ('course_id', 'modified') ordering = ('course_id', '-modified') diff --git a/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py b/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py index 1ef34ae2a95ae59255dd76303da55ec84d713243..2bb12b1aab1d246f444fe9321353d3c5fbdf54c1 100644 --- a/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py +++ b/openedx/core/djangoapps/content/course_structures/management/commands/generate_course_structure.py @@ -1,3 +1,8 @@ +""" +Django Management Command: Generate Course Structure +Generates and stores course structure information for one or more courses. +""" + import logging from optparse import make_option @@ -12,6 +17,9 @@ log = logging.getLogger(__name__) class Command(BaseCommand): + """ + Generates and stores course structure information for one or more courses. + """ args = '<course_id course_id ...>' help = 'Generates and stores course structure for one or more courses.' @@ -23,7 +31,9 @@ class Command(BaseCommand): ) def handle(self, *args, **options): - + """ + Perform the course structure generation workflow + """ if options['all']: course_keys = [course.id for course in modulestore().get_courses()] else: diff --git a/openedx/core/djangoapps/content/course_structures/models.py b/openedx/core/djangoapps/content/course_structures/models.py index b28e416bb28c28f13f12701a1552f26034131db5..40a9745837159b789dbe8f4455d15bb2daa50b3d 100644 --- a/openedx/core/djangoapps/content/course_structures/models.py +++ b/openedx/core/djangoapps/content/course_structures/models.py @@ -1,3 +1,6 @@ +""" +Django ORM model specifications for the Course Structures sub-application +""" import json import logging @@ -12,6 +15,9 @@ logger = logging.getLogger(__name__) # pylint: disable=invalid-name class CourseStructure(TimeStampedModel): + """ + The CourseStructure model is an aggregated representation of the course content tree + """ course_id = CourseKeyField(max_length=255, db_index=True, unique=True, verbose_name='Course ID') # Right now the only thing we do with the structure doc is store it and @@ -26,6 +32,9 @@ class CourseStructure(TimeStampedModel): @property def structure(self): + """ + Deserializes a course structure JSON object + """ if self.structure_json: return json.loads(self.structure_json) return None diff --git a/openedx/core/djangoapps/content/course_structures/signals.py b/openedx/core/djangoapps/content/course_structures/signals.py index 18ebe3544c2d28071c998127415251baf01a7c4a..0770d573b45da2f6568fd6c279b78fe0e3acc183 100644 --- a/openedx/core/djangoapps/content/course_structures/signals.py +++ b/openedx/core/djangoapps/content/course_structures/signals.py @@ -1,3 +1,6 @@ +""" +Django Signals classes and functions for the Course Structure application +""" from django.dispatch.dispatcher import receiver from xmodule.modulestore.django import SignalHandler @@ -7,6 +10,9 @@ from .models import CourseStructure @receiver(SignalHandler.course_published) def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument + """ + Course Structure application receiver for the course_published signal + """ # Import tasks here to avoid a circular import. from .tasks import update_course_structure diff --git a/openedx/core/djangoapps/content/course_structures/tasks.py b/openedx/core/djangoapps/content/course_structures/tasks.py index d246adf17d1f1e0983785d1d560e9ddb0c9e6af0..aa56ff01cf1bcfbb34a18cf29a5b47907706721d 100644 --- a/openedx/core/djangoapps/content/course_structures/tasks.py +++ b/openedx/core/djangoapps/content/course_structures/tasks.py @@ -1,3 +1,6 @@ +""" +Asynchronous tasks related to the Course Structure sub-application +""" import json import logging diff --git a/openedx/core/djangoapps/content/course_structures/tests.py b/openedx/core/djangoapps/content/course_structures/tests.py index 38e0237270efe3898233e4021b25a11992831aaa..8db13b51050c6c79c96554f15c10bb68126c83de 100644 --- a/openedx/core/djangoapps/content/course_structures/tests.py +++ b/openedx/core/djangoapps/content/course_structures/tests.py @@ -1,3 +1,6 @@ +""" +Course Structure Content sub-application test cases +""" import json from xmodule_django.models import UsageKey @@ -20,6 +23,9 @@ class SignalDisconnectTestMixin(object): class CourseStructureTaskTests(ModuleStoreTestCase): + """ + Test cases covering Course Structure task-related workflows + """ def setUp(self, **kwargs): super(CourseStructureTaskTests, self).setUp() self.course = CourseFactory.create(org='TestX', course='TS101', run='T1') @@ -40,6 +46,9 @@ class CourseStructureTaskTests(ModuleStoreTestCase): blocks = {} def add_block(block): + """ + Inserts new child XBlocks into the existing course tree + """ children = block.get_children() if block.has_children else [] blocks[unicode(block.location)] = { diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index aec125daa309ca4421eabb89ce49cda0257d3fe4..5a0ee2b018d7ea200f64cd2c6bdb5affe38dceee 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -38,6 +38,9 @@ def _cohort_added(sender, **kwargs): def _cohort_membership_changed(sender, **kwargs): """Emits a tracking log event each time cohort membership is modified""" def get_event_iter(user_id_iter, cohort_iter): + """ + Returns a dictionary containing a mashup of cohort and user information for the given lists + """ return ( {"cohort_id": cohort.id, "cohort_name": cohort.name, "user_id": user_id} for user_id in user_id_iter diff --git a/openedx/core/djangoapps/course_groups/tests/helpers.py b/openedx/core/djangoapps/course_groups/tests/helpers.py index 636b8b05e8d6e9f030a8d613f59d00714b203f3b..a95b6b5e41f817d214dab30a51992645a356e7ee 100644 --- a/openedx/core/djangoapps/course_groups/tests/helpers.py +++ b/openedx/core/djangoapps/course_groups/tests/helpers.py @@ -101,6 +101,9 @@ def config_course_cohorts_legacy( Nothing -- modifies course in place. """ def to_id(name): + """ + Helper method to convert a discussion topic name to a database identifier + """ return topic_name_to_id(course, name) topics = dict((name, {"sort_key": "A", diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index e07721b58554727003395a0856891be5ab30de87..5ef2b0edbc565fab744cc60b77a93368853b60e4 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -26,6 +26,10 @@ from ..tests.helpers import ( @patch("openedx.core.djangoapps.course_groups.cohorts.tracker") class TestCohortSignals(TestCase): + """ + Test cases to validate event emissions for various cohort-related workflows + """ + def setUp(self): super(TestCohortSignals, self).setUp() self.course_key = SlashSeparatedCourseKey("dummy", "dummy", "dummy") @@ -67,6 +71,9 @@ class TestCohortSignals(TestCase): mock_tracker.reset_mock() def assert_events(event_name_suffix, user_list, cohort_list): + """ + Confirms the presence of the specifed event for each user in the specified list of cohorts + """ mock_tracker.emit.assert_has_calls([ call( "edx.cohort.user_" + event_name_suffix, diff --git a/openedx/core/djangoapps/course_groups/tests/test_views.py b/openedx/core/djangoapps/course_groups/tests/test_views.py index 1db50864767dbe2d9e150b3c2c6b817de588318c..ec93ea6b6f3c434af680965ebd9aa5d3aaed9d6a 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_views.py +++ b/openedx/core/djangoapps/course_groups/tests/test_views.py @@ -100,6 +100,9 @@ class CohortViewsTestCase(ModuleStoreTestCase): self.assertRaises(Http404, view, *view_args) def create_cohorted_discussions(self): + """ + Set up a cohorted discussion in the system, complete with all the fixings + """ cohorted_inline_discussions = ['Topic A'] cohorted_course_wide_discussions = ["Topic B"] cohorted_discussions = cohorted_inline_discussions + cohorted_course_wide_discussions diff --git a/openedx/core/djangoapps/profile_images/tests/test_views.py b/openedx/core/djangoapps/profile_images/tests/test_views.py index eecf56e9135d343b77b90ad546732dbebc89129f..026d2739d5298c5ee6ee73febd2401937d809537 100644 --- a/openedx/core/djangoapps/profile_images/tests/test_views.py +++ b/openedx/core/djangoapps/profile_images/tests/test_views.py @@ -196,7 +196,10 @@ class ProfileImageViewPostTestCase(ProfileImageEndpointMixin, APITestCase): self.check_anonymous_request_rejected('post') self.assertFalse(mock_log.info.called) - @patch('openedx.core.djangoapps.profile_images.views._make_upload_dt', side_effect=[TEST_UPLOAD_DT, TEST_UPLOAD_DT2]) + @patch( + 'openedx.core.djangoapps.profile_images.views._make_upload_dt', + side_effect=[TEST_UPLOAD_DT, TEST_UPLOAD_DT2] + ) def test_upload_self(self, mock_make_image_version, mock_log): # pylint: disable=unused-argument """ Test that an authenticated user can POST to their own upload endpoint. diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index e7e716a3b52ca3bc63c9c302c21f48e7f9631e35..461f5e92dcac9f484993b16d22ecc037f3c8a8c5 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -1,3 +1,6 @@ +""" +Programmatic integration point for User API Accounts sub-application +""" from django.utils.translation import ugettext as _ from django.db import transaction, IntegrityError import datetime diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index df4d37da2c8bd4c13006f1e9e17ad45dbd6906d9..fff44bbbf74fc2835497324e04990e9bbfad98e7 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -1,3 +1,6 @@ +""" +Django REST Framework serializers for the User API Accounts sub-application +""" from rest_framework import serializers from django.contrib.auth.models import User from django.conf import settings diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index d3201eaa39eed2293b879520d43a203802ebb882..fa0f62f7619c46204012e230d185389f56e55cda 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -212,6 +212,9 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase): Test that eventing of language proficiencies, which happens update_account_settings method, behaves correctly. """ def verify_event_emitted(new_value, old_value): + """ + Confirm that the user setting event was properly emitted + """ update_account_settings(self.user, {"language_proficiencies": new_value}) self.assert_user_setting_event_emitted(setting='language_proficiencies', old=old_value, new=new_value) self.reset_tracker() @@ -226,7 +229,9 @@ class TestAccountApi(UserSettingsEventTestMixin, TestCase): @patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @patch.dict( - 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True + 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', + {'full': 50, 'small': 10}, + clear=True ) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class AccountSettingsOnCreationTest(TestCase): @@ -275,7 +280,9 @@ class AccountSettingsOnCreationTest(TestCase): @ddt.ddt class AccountCreationActivationAndPasswordChangeTest(TestCase): - + """ + Test cases to cover the account initialization workflow + """ USERNAME = u'frank-underwood' PASSWORD = u'ṕáśśẃőŕd' EMAIL = u'frank+underwood@example.com' @@ -415,6 +422,9 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): self.assertEqual(len(mail.outbox), 1) def _assert_is_datetime(self, timestamp): + """ + Internal helper to validate the type of the provided timestamp + """ if not timestamp: return False try: 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 2467cc19e24b79d2b02daf107096028b88f7a197..d161be798919fba46f151845424773957abba6dd 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1,4 +1,7 @@ # -*- coding: utf-8 -*- +""" +Test cases to cover Accounts-related behaviors of the User API application +""" import datetime from copy import deepcopy import ddt @@ -109,7 +112,9 @@ class UserAPITestCase(APITestCase): @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') @patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @patch.dict( - 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True + 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', + {'full': 50, 'small': 10}, + clear=True ) class TestAccountAPI(UserAPITestCase): """ @@ -261,6 +266,9 @@ class TestAccountAPI(UserAPITestCase): Test the return from GET based on user visibility setting. """ def verify_fields_visible_to_all_users(response): + """ + Confirms that private fields are private, and public/shareable fields are public/shareable + """ if preference_visibility == PRIVATE_VISIBILITY: self._verify_private_account_response(response) else: @@ -288,6 +296,9 @@ class TestAccountAPI(UserAPITestCase): as created by the test UserFactory). """ def verify_get_own_information(): + """ + Internal helper to perform the actual assertions + """ response = self.send_get(self.client) data = response.data self.assertEqual(15, len(data)) @@ -417,6 +428,9 @@ class TestAccountAPI(UserAPITestCase): client = self.login_client("client", "user") def verify_error_response(field_name, data): + """ + Internal helper to check the error messages returned + """ self.assertEqual( "This field is not editable via this API", data["field_errors"][field_name]["developer_message"] ) @@ -467,12 +481,18 @@ class TestAccountAPI(UserAPITestCase): Test the metadata stored when changing the name field. """ def get_name_change_info(expected_entries): + """ + Internal method to encapsulate the retrieval of old names used + """ legacy_profile = UserProfile.objects.get(id=self.user.id) name_change_info = legacy_profile.get_meta()["old_names"] self.assertEqual(expected_entries, len(name_change_info)) return name_change_info def verify_change_info(change_info, old_name, requester, new_name): + """ + Internal method to validate name changes + """ self.assertEqual(3, len(change_info)) self.assertEqual(old_name, change_info[0]) self.assertEqual("Name change requested through account API by {}".format(requester), change_info[1]) @@ -566,7 +586,10 @@ class TestAccountAPI(UserAPITestCase): ([u"not_a_JSON_object"], [{u'non_field_errors': [u'Invalid data. Expected a dictionary, but got unicode.']}]), ([{}], [{"code": [u"This field is required."]}]), ([{u"code": u"invalid_language_code"}], [{'code': [u'"invalid_language_code" is not a valid choice.']}]), - ([{u"code": u"kw"}, {u"code": u"el"}, {u"code": u"kw"}], [u'The language_proficiencies field must consist of unique languages']), + ( + [{u"code": u"kw"}, {u"code": u"el"}, {u"code": u"kw"}], + [u'The language_proficiencies field must consist of unique languages'] + ), ) @ddt.unpack def test_patch_invalid_language_proficiencies(self, patch_value, expected_error_message): @@ -578,7 +601,10 @@ class TestAccountAPI(UserAPITestCase): response = self.send_patch(client, {"language_proficiencies": patch_value}, expected_status=400) self.assertEqual( response.data["field_errors"]["language_proficiencies"]["developer_message"], - u"Value '{patch_value}' is not valid for field 'language_proficiencies': {error_message}".format(patch_value=patch_value, error_message=expected_error_message) + u"Value '{patch_value}' is not valid for field 'language_proficiencies': {error_message}".format( + patch_value=patch_value, + error_message=expected_error_message + ) ) @patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save') diff --git a/openedx/core/djangoapps/user_api/forms.py b/openedx/core/djangoapps/user_api/forms.py index 0dd0456e834142d9f4e75257b5b5113d34945731..df296db2a2dca25db3bc2281341a5dc69c2a0422 100644 --- a/openedx/core/djangoapps/user_api/forms.py +++ b/openedx/core/djangoapps/user_api/forms.py @@ -1,3 +1,6 @@ -# pylint: disable=unused-import, missing-docstring +# pylint: disable=unused-import # TODO: eventually move this implementation into the user_api +""" +Django Administration forms module +""" from student.forms import PasswordResetFormNoActive diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 0fd22ac83d94878aec3db83c3f6260f769946dd7..3f934c80336536725a3d5d94d62a5fd19b169aad 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -1,3 +1,6 @@ +""" +Django ORM model specifications for the User API application +""" from django.contrib.auth.models import User from django.core.validators import RegexValidator from django.db import models diff --git a/openedx/core/djangoapps/user_api/preferences/tests/test_api.py b/openedx/core/djangoapps/user_api/preferences/tests/test_api.py index ef95c7b2db68ead6dee3586a6bf7d1bd17917bc7..a3b3fe8b66209a522e1c7a83a49c90ebf9f46633 100644 --- a/openedx/core/djangoapps/user_api/preferences/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/preferences/tests/test_api.py @@ -293,7 +293,9 @@ class TestPreferenceAPI(TestCase): @ddt.ddt class UpdateEmailOptInTests(ModuleStoreTestCase): - + """ + Test cases to cover API-driven email list opt-in update workflows + """ USERNAME = u'frank-underwood' PASSWORD = u'ṕáśśẃőŕd' EMAIL = u'frank+underwood@example.com' @@ -385,6 +387,9 @@ class UpdateEmailOptInTests(ModuleStoreTestCase): self.assertEqual(result_obj.value, expected_result) def _assert_is_datetime(self, timestamp): + """ + Internal helper to assert the type of the provided timestamp value + """ if not timestamp: return False try: diff --git a/openedx/core/djangoapps/user_api/preferences/tests/test_views.py b/openedx/core/djangoapps/user_api/preferences/tests/test_views.py index d7486e66d67cbcc762560ea0599f2c9586a32fbc..fab722df9b445d0f06d63bbc876e89d437255f13 100644 --- a/openedx/core/djangoapps/user_api/preferences/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/preferences/tests/test_views.py @@ -133,6 +133,9 @@ class TestPreferencesAPI(UserAPITestCase): self._do_create_preferences_test(False) def _do_create_preferences_test(self, is_active): + """ + Internal helper to generalize the creation of a set of preferences + """ self.client.login(username=self.user.username, password=self.test_password) if not is_active: self.user.is_active = False @@ -361,6 +364,9 @@ class TestPreferencesDetailAPI(UserAPITestCase): self._set_url(self.test_pref_key) def _set_url(self, preference_key): + """ + Sets the url attribute including the username and provided preference key + """ self.url = reverse( self.url_endpoint_name, kwargs={'username': self.user.username, 'preference_key': preference_key} @@ -448,6 +454,9 @@ class TestPreferencesDetailAPI(UserAPITestCase): self._do_create_preference_test(False) def _do_create_preference_test(self, is_active): + """ + Generalization of the actual test workflow + """ self.client.login(username=self.user.username, password=self.test_password) if not is_active: self.user.is_active = False diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index 2bf15b5ad8555d633622680b57093630557d285c..bb2bac9592f8c25cbdead88a4a4b571f5251818f 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/openedx/core/djangoapps/user_api/serializers.py @@ -1,3 +1,6 @@ +""" +Django REST Framework serializers for the User API application +""" from django.contrib.auth.models import User from rest_framework import serializers from student.models import UserProfile @@ -6,14 +9,23 @@ from .models import UserPreference class UserSerializer(serializers.HyperlinkedModelSerializer): + """ + Serializer that generates a representation of a User entity containing a subset of fields + """ name = serializers.SerializerMethodField() preferences = serializers.SerializerMethodField() def get_name(self, user): + """ + Return the name attribute from the user profile object + """ profile = UserProfile.objects.get(user=user) return profile.name def get_preferences(self, user): + """ + Returns the set of preferences as a dict for the specified user + """ return dict([(pref.key, pref.value) for pref in user.preferences.all()]) class Meta(object): # pylint: disable=missing-docstring @@ -24,6 +36,9 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): class UserPreferenceSerializer(serializers.HyperlinkedModelSerializer): + """ + Serializer that generates a represenation of a UserPreference entity + """ user = UserSerializer() class Meta(object): # pylint: disable=missing-docstring diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index cd18934b19d415ee5f0753fe07271be5e2d57442..0d36d995e107f78252e729609c1e0435c6cfdd0e 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -51,11 +51,10 @@ class InterceptErrorsTest(TestCase): @mock.patch('openedx.core.djangoapps.user_api.helpers.LOGGER') def test_logs_errors(self, mock_logger): + exception = 'openedx.core.djangoapps.user_api.tests.test_helpers.FakeInputException' expected_log_msg = ( - u"An unexpected error occurred when calling 'intercepted_function' " - u"with arguments '()' and " - u"keyword arguments '{'raise_error': <class 'openedx.core.djangoapps.user_api.tests.test_helpers.FakeInputException'>}': " - u"FakeInputException()" + u"An unexpected error occurred when calling 'intercepted_function' with arguments '()' and " + u"keyword arguments '{'raise_error': <class '" + exception + u"'>}': FakeInputException()" ) # Verify that the raised exception has the error message diff --git a/openedx/core/djangoapps/user_api/tests/test_models.py b/openedx/core/djangoapps/user_api/tests/test_models.py index 7b4cec224a15be8fb2ccd45dba31941e4fe0cfba..d3977e1c1bc156e285ef59d7adb19400a9ed8d2f 100644 --- a/openedx/core/djangoapps/user_api/tests/test_models.py +++ b/openedx/core/djangoapps/user_api/tests/test_models.py @@ -15,6 +15,9 @@ from ..preferences.api import set_user_preference class UserPreferenceModelTest(ModuleStoreTestCase): + """ + Test case covering User Preference ORM model attributes and custom operations + """ def test_duplicate_user_key(self): user = UserFactory.create() UserPreferenceFactory.create(user=user, key="testkey", value="first") diff --git a/openedx/core/djangoapps/user_api/tests/test_partition_schemes.py b/openedx/core/djangoapps/user_api/tests/test_partition_schemes.py index 794f837a31c14a5349b3db3db951963275a29ea3..ba80846ceaf2445d1ff8e8c4096cc6a6c15f8e29 100644 --- a/openedx/core/djangoapps/user_api/tests/test_partition_schemes.py +++ b/openedx/core/djangoapps/user_api/tests/test_partition_schemes.py @@ -53,7 +53,11 @@ class TestRandomUserPartitionScheme(PartitionTestCase): # make sure we get the same group back out every time for __ in range(10): - group2_id = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, self.user_partition) + group2_id = RandomUserPartitionScheme.get_group_for_user( + self.MOCK_COURSE_ID, + self.user, + self.user_partition + ) self.assertEqual(group1_id, group2_id) def test_get_group_for_user_with_assign(self): diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 2481da0b03eae34c82690f96cf9e96c0a2aa63ac..a480130905d3d6990c02f8badfb1fb0f992e9028 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -50,10 +50,16 @@ ROLE_LIST_URI = "/user_api/v1/forum_roles/Moderator/users/" @override_settings(EDX_API_KEY=TEST_API_KEY) class ApiTestCase(TestCase): + """ + Parent test case for API workflow coverage + """ LIST_URI = USER_LIST_URI def basic_auth(self, username, password): + """ + Returns a dictionary containing the http auth header with encoded username+password + """ return {'HTTP_AUTHORIZATION': 'Basic ' + base64.b64encode('%s:%s' % (username, password))} def request_with_auth(self, method, *args, **kwargs): @@ -107,6 +113,9 @@ class ApiTestCase(TestCase): self.assertSelfReferential(user) def assertPrefIsValid(self, pref): + """ + Assert that the given preference is acknowledged by the system + """ self.assertItemsEqual(pref.keys(), ["user", "key", "value", "url"]) self.assertSelfReferential(pref) self.assertUserIsValid(pref["user"]) @@ -142,6 +151,9 @@ class ApiTestCase(TestCase): class EmptyUserTestCase(ApiTestCase): + """ + Test that the endpoint supports empty user result sets + """ def test_get_list_empty(self): result = self.get_json(self.LIST_URI) self.assertEqual(result["count"], 0) @@ -165,6 +177,9 @@ class EmptyRoleTestCase(ApiTestCase): class UserApiTestCase(ApiTestCase): + """ + Generalized test case class for specific implementations below + """ def setUp(self): super(UserApiTestCase, self).setUp() self.users = [ @@ -182,6 +197,9 @@ class UserApiTestCase(ApiTestCase): class RoleTestCase(UserApiTestCase): + """ + Test cases covering Role-related views and their behaviors + """ course_id = SlashSeparatedCourseKey.from_deprecated_string("org/course/run") LIST_URI = ROLE_LIST_URI + "?course_id=" + course_id.to_deprecated_string() @@ -268,6 +286,9 @@ class RoleTestCase(UserApiTestCase): class UserViewSetTest(UserApiTestCase): + """ + Test cases covering the User DRF view set class and its various behaviors + """ LIST_URI = USER_LIST_URI def setUp(self): @@ -382,6 +403,9 @@ class UserViewSetTest(UserApiTestCase): class UserPreferenceViewSetTest(UserApiTestCase): + """ + Test cases covering the User Preference DRF view class and its various behaviors + """ LIST_URI = USER_PREFERENCE_LIST_URI def setUp(self): @@ -522,6 +546,9 @@ class UserPreferenceViewSetTest(UserApiTestCase): class PreferenceUsersListViewTest(UserApiTestCase): + """ + Test cases covering the list viewing behavior for user preferences + """ LIST_URI = "/user_api/v1/preferences/key0/users/" def test_options(self): @@ -880,7 +907,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase): u"required": True, u"label": u"Public username", u"placeholder": u"JaneDoe", - u"instructions": u"The name that will identify you in your courses - <strong>(cannot be changed later)</strong>", + u"instructions": u"The name that will identify you in your courses - <strong>(cannot be changed later)</strong>", # pylint: disable=line-too-long u"restrictions": { "min_length": USERNAME_MIN_LENGTH, "max_length": USERNAME_MAX_LENGTH @@ -968,7 +995,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase): u"required": True, u"label": u"Public username", u"placeholder": u"JaneDoe", - u"instructions": u"The name that will identify you in your courses - <strong>(cannot be changed later)</strong>", + u"instructions": u"The name that will identify you in your courses - <strong>(cannot be changed later)</strong>", # pylint: disable=line-too-long u"restrictions": { "min_length": USERNAME_MIN_LENGTH, "max_length": USERNAME_MAX_LENGTH @@ -1097,19 +1124,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase): ) @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) def test_registration_honor_code_mktg_site_enabled(self): + link_html = '<a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>' self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the {platform_name} <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "label": "I agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/honor\">Terms of Service and Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "required": "You must agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ) } } @@ -1118,19 +1148,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase): @override_settings(MKTG_URLS_LINK_MAP={"HONOR": "honor"}) @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) def test_registration_honor_code_mktg_site_disabled(self): + link_html = '<a href=\"/honor\">Terms of Service and Honor Code</a>' self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the {platform_name} <a href=\"/honor\">Terms of Service and Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "label": "I agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} <a href=\"/honor\">Terms of Service and Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "required": "You must agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ) } } @@ -1145,38 +1178,44 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, ApiTestCase): def test_registration_separate_terms_of_service_mktg_site_enabled(self): # Honor code field should say ONLY honor code, # not "terms of service and honor code" + link_html = '<a href=\"https://www.test.com/honor\">Honor Code</a>' self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} <a href=\"https://www.test.com/honor\">Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "label": "I agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/honor\">Honor Code</a>.".format( - platform_name=settings.PLATFORM_NAME + "required": "You must agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ) } } ) # Terms of service field should also be present + link_html = '<a href=\"https://www.test.com/tos\">Terms of Service</a>' self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} <a href=\"https://www.test.com/tos\">Terms of Service</a>.".format( - platform_name=settings.PLATFORM_NAME + "label": "I agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ), "name": "terms_of_service", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} <a href=\"https://www.test.com/tos\">Terms of Service</a>.".format( - platform_name=settings.PLATFORM_NAME + "required": "You must agree to the {platform_name} {link_html}.".format( + platform_name=settings.PLATFORM_NAME, + link_html=link_html ) } } diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index f0fdf371efd959989d6d5ca4f7d171a967a7d406..67031ac21aeaa53d6b081928ba0ca8e4a0ba1a74 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -270,7 +270,7 @@ class RegistrationView(APIView): # Translators: This message is shown to users who attempt to create a new # account using an email address associated with an existing account. "email": _( - u"It looks like {email_address} belongs to an existing account. Try again with a different email address." + u"It looks like {email_address} belongs to an existing account. Try again with a different email address." # pylint: disable=line-too-long ).format(email_address=email), # Translators: This message is shown to users who attempt to create a new # account using a username associated with an existing account. @@ -800,6 +800,9 @@ class PasswordResetView(APIView): class UserViewSet(viewsets.ReadOnlyModelViewSet): + """ + DRF class for interacting with the User ORM object + """ authentication_classes = (authentication.SessionAuthentication,) permission_classes = (ApiKeyHeaderPermission,) queryset = User.objects.all().prefetch_related("preferences") @@ -833,6 +836,9 @@ class ForumRoleUsersListView(generics.ListAPIView): class UserPreferenceViewSet(viewsets.ReadOnlyModelViewSet): + """ + DRF class for interacting with the UserPreference ORM + """ authentication_classes = (authentication.SessionAuthentication,) permission_classes = (ApiKeyHeaderPermission,) queryset = UserPreference.objects.all() @@ -844,6 +850,9 @@ class UserPreferenceViewSet(viewsets.ReadOnlyModelViewSet): class PreferenceUsersListView(generics.ListAPIView): + """ + DRF class for listing a user's preferences + """ authentication_classes = (authentication.SessionAuthentication,) permission_classes = (ApiKeyHeaderPermission,) serializer_class = UserSerializer diff --git a/openedx/core/lib/api/parsers.py b/openedx/core/lib/api/parsers.py index bec80da9a05dac438f487e7b998cd1ad56d58ff6..fb5470f496678c7d46bdf5410179b68a782521b8 100644 --- a/openedx/core/lib/api/parsers.py +++ b/openedx/core/lib/api/parsers.py @@ -1,3 +1,6 @@ +""" +Custom Django REST Framework request/response pipeline parsers +""" from rest_framework import parsers diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index 5b2701787c04ee17863017649f8d2859ecfc47a7..dc371431860c8e8f03bbff6395828fef3dff890c 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -1,3 +1,7 @@ +""" +API library for Django REST Framework permissions-oriented workflows +""" + from django.conf import settings from rest_framework import permissions from django.http import Http404 @@ -6,6 +10,9 @@ from student.roles import CourseStaffRole class ApiKeyHeaderPermission(permissions.BasePermission): + """ + Django REST Framework permissions class used to manage API Key integrations + """ def has_permission(self, request, view): """ Check for permissions by matching the configured API key and header diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py index 0f5a7756158dd309fbc856fbff1c3f871387de3d..d736b86bc0d374ac15c4dc127596724a12da6319 100644 --- a/openedx/core/lib/api/view_utils.py +++ b/openedx/core/lib/api/view_utils.py @@ -24,7 +24,6 @@ from openedx.core.lib.api.authentication import ( OAuth2AuthenticationAllowInactiveUser, ) from openedx.core.lib.api.permissions import IsUserInUrl -from util.milestones_helpers import any_unfulfilled_milestones class DeveloperErrorViewMixin(object): @@ -63,6 +62,10 @@ class DeveloperErrorViewMixin(object): return self.make_error_response(400, validation_error.messages[0]) def handle_exception(self, exc): + """ + Generalized helper method for managing specific API exception workflows + """ + if isinstance(exc, APIException): return self.make_error_response(exc.status_code, exc.detail) elif isinstance(exc, Http404): diff --git a/openedx/core/operations.py b/openedx/core/operations.py index 8b3b1cf24c9e32e72217fee38d0667825d702f08..ec0d607b69ed37350f2d96d3e333eabad02c3fb6 100644 --- a/openedx/core/operations.py +++ b/openedx/core/operations.py @@ -1,3 +1,6 @@ +""" +Workflows useful for reporting on runtime characteristics of the system +""" import os import signal import tempfile diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py index 4da81515b1dfd62c5701e5e619d60817587994a7..dfcfdeb96ad2bdb02e61f74c15e0797abf5b5e2e 100644 --- a/pavelib/acceptance_test.py +++ b/pavelib/acceptance_test.py @@ -46,13 +46,13 @@ def test_acceptance(options): 'red', 'No system specified, running tests for both cms and lms.' ) - print(msg) + print msg if opts['default_store'] not in ['draft', 'split']: msg = colorize( 'red', 'No modulestore specified, running tests for both draft and split.' ) - print(msg) + print msg suite = AcceptanceTestSuite('{} acceptance'.format(opts['system']), **opts) suite.run() diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index 808fc410a1a9939965c7f920cc3045e54ca0083a..b76936fa4014ff7d1957bbbac324b12753b42345 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -107,7 +107,7 @@ def run_bokchoy(**opts): default_store=test_suite.default_store, ) ) - print(msg) + print msg test_suite.run() @@ -120,12 +120,12 @@ def bokchoy_coverage(): coveragerc = Env.BOK_CHOY_COVERAGERC msg = colorize('green', "Combining coverage reports") - print(msg) + print msg sh("coverage combine --rcfile={}".format(coveragerc)) msg = colorize('green', "Generating coverage reports") - print(msg) + print msg sh("coverage html --rcfile={}".format(coveragerc)) sh("coverage xml --rcfile={}".format(coveragerc)) diff --git a/pavelib/docs.py b/pavelib/docs.py index a6901fefe922220b9b9d1cd96b0c248133d497bf..1e8957ad7be91469d8708b7a575634a89a30519a 100644 --- a/pavelib/docs.py +++ b/pavelib/docs.py @@ -1,3 +1,7 @@ +""" +Open edX Documentation Builder +Ties into Sphinx to generate files at the specified location(s) +""" from __future__ import print_function import sys from paver.easy import * diff --git a/pavelib/i18n.py b/pavelib/i18n.py index 43f4b3390eae95b8d27876880b00d13ce7ce08aa..dd737adebfa76e7f40071a5b715aac0ae8629206 100644 --- a/pavelib/i18n.py +++ b/pavelib/i18n.py @@ -150,13 +150,13 @@ def i18n_rtl(): cmd = "i18n_tool transifex" sh(cmd + " rtl") - print("Now generating langugage files...") + print "Now generating langugage files..." cmd = "i18n_tool generate" sh(cmd + " --rtl") - print("Committing translations...") + print "Committing translations..." sh('git clean -fdX conf/locale') sh('git add conf/locale') sh('git commit --amend') @@ -170,13 +170,13 @@ def i18n_ltr(): cmd = "i18n_tool transifex" sh(cmd + " ltr") - print("Now generating langugage files...") + print "Now generating langugage files..." cmd = "i18n_tool generate" sh(cmd + " --ltr") - print("Committing translations...") + print "Committing translations..." sh('git clean -fdX conf/locale') sh('git add conf/locale') sh('git commit --amend') @@ -194,12 +194,13 @@ def i18n_robot_pull(): """ Pull source strings, generate po and mo files, and validate """ - # sh('paver test_i18n') # TODO tests were removed from repo, but there should still be tests that cover the translations... - # Validate the recently pulled translations, and give a bail option + # sh('paver test_i18n') + # Tests were removed from repo, but there should still be tests covering the translations + # TODO: Validate the recently pulled translations, and give a bail option sh('git clean -fdX conf/locale/rtl') sh('git clean -fdX conf/locale/eo') cmd = "i18n_tool validate" - print("\n\nValidating translations with `i18n_tool validate`...") + print "\n\nValidating translations with `i18n_tool validate`..." sh("{cmd}".format(cmd=cmd)) con = raw_input("Continue with committing these translations (y/n)? ") diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py index 502c875774381581cc556f6126ca6d2bc04873cc..1bfd71fba7dd4b1d4120a3aebb9bb7c2974f9d58 100644 --- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py +++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py @@ -10,6 +10,9 @@ REPO_DIR = os.getcwd() class TestPaverBokChoyCmd(unittest.TestCase): + """ + Paver Bok Choy Command test cases + """ def _expected_command(self, name, store=None): """ diff --git a/pavelib/paver_tests/test_prereqs.py b/pavelib/paver_tests/test_prereqs.py index 47437ee9076829e9149b51663b67afaeecb7ba62..4c7ed9ba1045ec2d464146e0ec3ade9a105614af 100644 --- a/pavelib/paver_tests/test_prereqs.py +++ b/pavelib/paver_tests/test_prereqs.py @@ -1,3 +1,7 @@ +""" +Tests covering the Open edX Paver prequisites installation workflow +""" + import os import unittest from pavelib.prereqs import no_prereq_install @@ -29,25 +33,25 @@ class TestPaverPrereqInstall(unittest.TestCase): os.environ.clear() os.environ.update(_orig_environ) - def test_no_prereq_install_true(self): + def test_no_prereq_install_true_lowercase(self): """ Ensure that 'true' will be True. """ self.check_val('true', True) - def test_no_prereq_install_false(self): + def test_no_prereq_install_false_lowercase(self): """ Ensure that 'false' will be False. """ self.check_val('false', False) - def test_no_prereq_install_True(self): + def test_no_prereq_install_true(self): """ Ensure that 'True' will be True. """ self.check_val('True', True) - def test_no_prereq_install_False(self): + def test_no_prereq_install_false(self): """ Ensure that 'False' will be False. """ diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index d302924ba60fbbc097339c505ef376fabbba7c7a..029b0e7693eba1fd8c45bff9b14d61ed099f8ca1 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -57,19 +57,19 @@ def compute_fingerprint(path_list): hasher = hashlib.sha1() - for path in path_list: + for path_item in path_list: # For directories, create a hash based on the modification times # of first-level subdirectories - if os.path.isdir(path): - for dirname in sorted(os.listdir(path)): - p = os.path.join(path, dirname) - if os.path.isdir(p): - hasher.update(str(os.stat(p).st_mtime)) + if os.path.isdir(path_item): + for dirname in sorted(os.listdir(path_item)): + path_name = os.path.join(path_item, dirname) + if os.path.isdir(path_name): + hasher.update(str(os.stat(path_name).st_mtime)) # For files, hash the contents of the file - if os.path.isfile(path): - with open(path, "rb") as file_handle: + if os.path.isfile(path_item): + with open(path_item, "rb") as file_handle: hasher.update(file_handle.read()) return hasher.hexdigest() @@ -113,7 +113,7 @@ def prereq_cache(cache_name, paths, install_func): post_install_hash = compute_fingerprint(paths) cache_file.write(post_install_hash) else: - print('{cache} unchanged, skipping...'.format(cache=cache_name)) + print '{cache} unchanged, skipping...'.format(cache=cache_name) def ruby_prereqs_installation(): diff --git a/pavelib/servers.py b/pavelib/servers.py index 7c0d778bbf1cc04358bf617c302cb0924effe20e..3fd3c335b09579b687157390213ddb321b106c94 100644 --- a/pavelib/servers.py +++ b/pavelib/servers.py @@ -263,5 +263,5 @@ def check_settings(args): django_shell_cmd = django_cmd(system, settings, 'shell', '--plain', '--pythonpath=.') sh("{import_cmd} | {shell_cmd}".format(import_cmd=import_cmd, shell_cmd=django_shell_cmd)) - except: + except: # pylint: disable=bare-except print("Failed to import settings", file=sys.stderr) diff --git a/pavelib/tests.py b/pavelib/tests.py index f8f2aeaf8116c495758e9a36cacf23cc7cda1c34..72e94823c3d4c0fbde3cc1b8f661ffb5c1946a37 100644 --- a/pavelib/tests.py +++ b/pavelib/tests.py @@ -194,8 +194,6 @@ def coverage(options): """ Build the html, xml, and diff coverage reports """ - compare_branch = getattr(options, 'compare_branch', 'origin/master') - report_dir = Env.REPORT_DIR rcfile = Env.PYTHON_COVERAGERC @@ -262,4 +260,4 @@ def diff_coverage(options): ) ) - print("\n") + print "\n" diff --git a/pavelib/utils/process.py b/pavelib/utils/process.py index 4d4cb9e3e917704eb8c67a0fd32096cbfc2318bc..d2e2b3bafdfe3a95faca69c7887c2306ef69a567 100644 --- a/pavelib/utils/process.py +++ b/pavelib/utils/process.py @@ -18,6 +18,7 @@ def kill_process(proc): """ p1_group = psutil.Process(proc.pid) + # pylint: disable=unexpected-keyword-arg child_pids = p1_group.get_children(recursive=True) for child_pid in child_pids: @@ -55,6 +56,7 @@ def run_multi_processes(cmd_list, out_log=None, err_log=None): for cmd in cmd_list: pids.extend([subprocess.Popen(cmd, **kwargs)]) + # pylint: disable=unused-argument def _signal_handler(*args): """ What to do when process is ended @@ -66,6 +68,7 @@ def run_multi_processes(cmd_list, out_log=None, err_log=None): signal.pause() print("Processes ending") + # pylint: disable=broad-except except Exception as err: print("Error running process {}".format(err), file=sys.stderr) @@ -109,6 +112,7 @@ def run_background_process(cmd, out_log=None, err_log=None, cwd=None): """ p1_group = psutil.Process(proc.pid) + # pylint: disable=unexpected-keyword-arg child_pids = p1_group.get_children(recursive=True) for child_pid in child_pids: diff --git a/pavelib/utils/test/bokchoy_utils.py b/pavelib/utils/test/bokchoy_utils.py index 237621a9f0d353517bfd3230cc9276def49ccfd8..23edbdd204f373ed05335155a1645c0de21cc472 100644 --- a/pavelib/utils/test/bokchoy_utils.py +++ b/pavelib/utils/test/bokchoy_utils.py @@ -101,7 +101,7 @@ def wait_for_test_servers(): "red", "Could not contact {} test server".format(service) ) - print(msg) + print msg sys.exit(1) @@ -112,7 +112,7 @@ def is_mongo_running(): # The mongo command will connect to the service, # failing with a non-zero exit code if it cannot connect. output = os.popen('mongo --eval "print(\'running\')"').read() - return (output and "running" in output) + return output and "running" in output def is_memcache_running(): @@ -130,9 +130,9 @@ def is_mysql_running(): """ # We need to check whether or not mysql is running as a process # even if it is not daemonized. - with open(os.devnull, 'w') as DEVNULL: + with open(os.devnull, 'w') as os_devnull: #pgrep returns the PID, which we send to /dev/null - returncode = subprocess.call("pgrep mysqld", stdout=DEVNULL, shell=True) + returncode = subprocess.call("pgrep mysqld", stdout=os_devnull, shell=True) return returncode == 0 @@ -153,7 +153,7 @@ def check_mongo(): """ if not is_mongo_running(): msg = colorize('red', "Mongo is not running locally.") - print(msg) + print msg sys.exit(1) @@ -163,7 +163,7 @@ def check_memcache(): """ if not is_memcache_running(): msg = colorize('red', "Memcache is not running locally.") - print(msg) + print msg sys.exit(1) @@ -173,7 +173,7 @@ def check_mysql(): """ if not is_mysql_running(): msg = colorize('red', "MySQL is not running locally.") - print(msg) + print msg sys.exit(1) diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py index 52bf492909254461fbb1a01d6917210713a25bc1..b9b6447444cbcd18a5face4c22ffeaa2dae15d8d 100644 --- a/pavelib/utils/test/suites/acceptance_suite.py +++ b/pavelib/utils/test/suites/acceptance_suite.py @@ -51,6 +51,9 @@ class AcceptanceTest(TestSuite): return cmd def _update_assets(self): + """ + Internal helper method to manage asset compilation + """ args = [self.system, '--settings=acceptance'] if self.fasttest: diff --git a/pavelib/utils/test/suites/suite.py b/pavelib/utils/test/suites/suite.py index 58b1f841e943ee5256938a1549f1467354b660e4..9f8fbfcff0168a4241f367528fd8da4cdbdc649b 100644 --- a/pavelib/utils/test/suites/suite.py +++ b/pavelib/utils/test/suites/suite.py @@ -37,7 +37,7 @@ class TestSuite(object): i.e. Checking for and defining required directories. """ - print("\nSetting up for {suite_name}".format(suite_name=self.root)) + print "\nSetting up for {suite_name}".format(suite_name=self.root) self.failed_suites = [] def __exit__(self, exc_type, exc_value, traceback): @@ -50,7 +50,7 @@ class TestSuite(object): i.e. Cleaning mongo after the lms tests run. """ - print("\nCleaning up after {suite_name}".format(suite_name=self.root)) + print "\nCleaning up after {suite_name}".format(suite_name=self.root) @property def cmd(self): @@ -94,7 +94,7 @@ class TestSuite(object): kill_process(process) sys.exit(1) else: - return (process.returncode == 0) + return process.returncode == 0 def run_suite_tests(self): """ @@ -123,7 +123,7 @@ class TestSuite(object): else: msg = colorize('green', "\n\n{bar}\nNo test failures ".format(bar="=" * 48)) - print(msg) + print msg def run(self): """ diff --git a/pavelib/utils/test/utils.py b/pavelib/utils/test/utils.py index 75e10b74082a1468afb4dba4f5c44bd886a4d6da..6a573981131bcf2c9499f04efa1c9c7e882f3a0d 100644 --- a/pavelib/utils/test/utils.py +++ b/pavelib/utils/test/utils.py @@ -44,7 +44,7 @@ def clean_reports_dir(options): Clean coverage files, to ensure that we don't use stale data to generate reports. """ if getattr(options, 'skip_clean', False): - print('--skip_clean is set, skipping...') + print '--skip_clean is set, skipping...' return # We delete the files but preserve the directory structure @@ -71,16 +71,23 @@ def check_firefox_version(): """ expected_firefox_ver = "Mozilla Firefox 28.0" firefox_ver = subprocess.check_output("firefox --version", shell=True).strip() + debian_location = 'https://s3.amazonaws.com/vagrant.testeng.edx.org/' + debian_package = 'firefox_28.0%2Bbuild2-0ubuntu0.12.04.1_amd64.deb' + debian_path = '{location}{package}'.format(location=debian_location, package=debian_package) if firefox_ver != expected_firefox_ver: raise Exception( 'Required firefox version not found.\n' 'Expected: {expected_version}; Actual: {actual_version}.\n\n' 'As the vagrant user in devstack, run the following:\n\n' - '\t$ sudo wget -O /tmp/firefox_28.deb https://s3.amazonaws.com/vagrant.testeng.edx.org/firefox_28.0%2Bbuild2-0ubuntu0.12.04.1_amd64.deb\n' + '\t$ sudo wget -O /tmp/firefox_28.deb {debian_path}\n' '\t$ sudo apt-get remove firefox\n\n' '\t$ sudo gdebi -nq /tmp/firefox_28.deb\n\n' 'Confirm the new version:\n' '\t$ firefox --version\n' - '\t{expected_version}'.format(actual_version=firefox_ver, expected_version=expected_firefox_ver) + '\t{expected_version}'.format( + actual_version=firefox_ver, + expected_version=expected_firefox_ver, + debian_path=debian_path + ) ) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index a6f7a02117e5e6d46be238279ac2bd58d1e8e712..35192a381dcc7d5a596ef60442149cfee0267f00 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -50,7 +50,7 @@ git+https://github.com/edx/edx-oauth2-provider.git@0.5.7#egg=oauth2-provider==0. -e git+https://github.com/pmitros/RecommenderXBlock.git@518234bc354edbfc2651b9e534ddb54f96080779#egg=recommender-xblock -e git+https://github.com/edx/edx-search.git@release-2015-09-11a#egg=edx-search -e git+https://github.com/edx/edx-milestones.git@9b44a37edc3d63a23823c21a63cdd53ef47a7aa4#egg=edx-milestones -git+https://github.com/edx/edx-lint.git@c5745631d2eee4e2efe8c31fa7b42fe2c12a0755#egg=edx_lint==0.2.7 +git+https://github.com/edx/edx-lint.git@v0.2.9#egg=edx_lint==0.2.9 -e git+https://github.com/edx/xblock-utils.git@213a97a50276d6a2504d8133650b2930ead357a0#egg=xblock-utils -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive -e git+https://github.com/edx/edx-reverification-block.git@5162ca169ca1486479e02f6ad2f538e6b8ae6f00#egg=edx-reverification-block