From 1eab25f29248d12b10d78ecf44edc1494abcd56e Mon Sep 17 00:00:00 2001 From: Matt Drayer <mattdrayer@edx.org> Date: Wed, 14 Oct 2015 10:42:36 -0400 Subject: [PATCH] mattdrayer/increment-edx-lint: Bump to v0.2.9 and address pylint/pep8 violations * Fix paver violations to stablize edx-lint update * Parens, Line2Long * Fix missing docstrings * Fix PEP8 issues * Address PR feedback (thanks @nedbat!) --- .../contentstore/features/transcripts.py | 4 +- .../management/commands/clone_course.py | 4 +- .../management/commands/export_all_courses.py | 24 +++--- .../commands/export_convert_format.py | 2 +- cms/djangoapps/contentstore/tests/tests.py | 10 +-- .../management/commands/transfer_students.py | 8 +- .../xmodule/modulestore/xml_importer.py | 13 ++-- .../xmodule/tests/rendering/__init__.py | 3 + .../lib/xmodule/xmodule/tests/test_export.py | 16 ++-- .../lib/xmodule/xmodule/tests/test_fields.py | 3 +- .../xmodule/tests/test_imageannotation.py | 15 +++- .../lib/xmodule/xmodule/tests/test_import.py | 28 +++---- .../xmodule/xmodule/tests/test_lti_unit.py | 26 +++++-- .../xmodule/tests/test_peer_grading.py | 15 +++- .../xmodule/tests/test_randomize_module.py | 3 + .../xmodule/tests/test_self_assessment.py | 12 +++ .../xmodule/tests/test_split_test_module.py | 7 +- .../xmodule/tests/test_textannotation.py | 15 +++- .../xmodule/tests/test_util_open_ended.py | 38 +++++++++ .../xmodule/xmodule/tests/test_validation.py | 21 ++++- .../xmodule/tests/test_videoannotation.py | 11 ++- .../xmodule/video_module/transcripts_utils.py | 3 +- .../xmodule/video_module/video_module.py | 14 +++- .../xmodule/video_module/video_xfields.py | 14 ++-- lms/djangoapps/django_comment_client/utils.py | 77 ++++++++++++++++++- .../retry_failed_photo_verifications.py | 8 +- .../content/course_structures/admin.py | 6 ++ .../commands/generate_course_structure.py | 12 ++- .../content/course_structures/models.py | 9 +++ .../content/course_structures/signals.py | 6 ++ .../content/course_structures/tasks.py | 3 + .../content/course_structures/tests.py | 9 +++ .../core/djangoapps/course_groups/cohorts.py | 3 + .../djangoapps/course_groups/tests/helpers.py | 3 + .../course_groups/tests/test_cohorts.py | 7 ++ .../course_groups/tests/test_views.py | 3 + .../profile_images/tests/test_views.py | 5 +- .../core/djangoapps/user_api/accounts/api.py | 3 + .../user_api/accounts/serializers.py | 3 + .../user_api/accounts/tests/test_api.py | 14 +++- .../user_api/accounts/tests/test_views.py | 32 +++++++- openedx/core/djangoapps/user_api/forms.py | 5 +- openedx/core/djangoapps/user_api/models.py | 3 + .../user_api/preferences/tests/test_api.py | 7 +- .../user_api/preferences/tests/test_views.py | 9 +++ .../core/djangoapps/user_api/serializers.py | 15 ++++ .../djangoapps/user_api/tests/test_helpers.py | 7 +- .../djangoapps/user_api/tests/test_models.py | 3 + .../user_api/tests/test_partition_schemes.py | 6 +- .../djangoapps/user_api/tests/test_views.py | 75 +++++++++++++----- openedx/core/djangoapps/user_api/views.py | 11 ++- openedx/core/lib/api/parsers.py | 3 + openedx/core/lib/api/permissions.py | 7 ++ openedx/core/lib/api/view_utils.py | 5 +- openedx/core/operations.py | 3 + pavelib/acceptance_test.py | 4 +- pavelib/bok_choy.py | 6 +- pavelib/docs.py | 4 + pavelib/i18n.py | 15 ++-- .../paver_tests/test_paver_bok_choy_cmds.py | 3 + pavelib/paver_tests/test_prereqs.py | 12 ++- pavelib/prereqs.py | 18 ++--- pavelib/servers.py | 2 +- pavelib/tests.py | 4 +- pavelib/utils/process.py | 4 + pavelib/utils/test/bokchoy_utils.py | 14 ++-- pavelib/utils/test/suites/acceptance_suite.py | 3 + pavelib/utils/test/suites/suite.py | 8 +- pavelib/utils/test/utils.py | 13 +++- requirements/edx/github.txt | 2 +- 70 files changed, 614 insertions(+), 169 deletions(-) diff --git a/cms/djangoapps/contentstore/features/transcripts.py b/cms/djangoapps/contentstore/features/transcripts.py index 5f94ec90e54..e9e25958aa3 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 f6df8bae421..bbd94e52ebc 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 205a9b4233d..c5ab34c421f 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 926ab69d0ac..eb0cc575d9e 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 710351ae4c1..2a2f267b3ab 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 8e8557ff880..835d954071b 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 d18c76b0017..6791d10af1f 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 d93c168c8c5..fc6a0c4d9bf 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 13585b5415d..b9f361d4b0e 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 95a6fb1b5d3..4464470eab9 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 68a30d21a55..3daaeb3e2ca 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 0eee605fd7b..1cb42b8d43e 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 052ae31821c..fc77ca7c1e2 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 0e895100e82..39274d193b7 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 485d3efb153..798d3028b16 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 3c577473cb5..0d64009911c 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 b9adbb2ea57..e4b35ec5919 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 1aab9dcec7a..a2b1f241592 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 a2710ba52c2..b863d5c22c7 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 f3e6b95b500..308abffb913 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 fa6689fc900..f9cf5f1402e 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 cee5d1c7e9a..edf3e5b72e1 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 6e93accb989..4e7ad7108d2 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 b40b0fd2cf7..f494b02e558 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 f7921e0f9bb..0807a66fa81 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 502185d25d8..6fdd1cc86f4 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 64bbdad86cb..cdf31c58e8c 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 1ef34ae2a95..2bb12b1aab1 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 b28e416bb28..40a97458371 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 18ebe3544c2..0770d573b45 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 d246adf17d1..aa56ff01cf1 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 38e0237270e..8db13b51050 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 aec125daa30..5a0ee2b018d 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 636b8b05e8d..a95b6b5e41f 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 e07721b5855..5ef2b0edbc5 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 1db50864767..ec93ea6b6f3 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 eecf56e9135..026d2739d52 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 e7e716a3b52..461f5e92dca 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 df4d37da2c8..fff44bbbf74 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 d3201eaa39e..fa0f62f7619 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 2467cc19e24..d161be79891 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 0dd0456e834..df296db2a2d 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 0fd22ac83d9..3f934c80336 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 ef95c7b2db6..a3b3fe8b662 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 d7486e66d67..fab722df9b4 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 2bf15b5ad85..bb2bac9592f 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 cd18934b19d..0d36d995e10 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 7b4cec224a1..d3977e1c1bc 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 794f837a31c..ba80846ceaf 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 2481da0b03e..a480130905d 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 f0fdf371efd..67031ac21ae 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 bec80da9a05..fb5470f4966 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 5b2701787c0..dc371431860 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 0f5a7756158..d736b86bc0d 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 8b3b1cf24c9..ec0d607b69e 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 4da81515b1d..dfcfdeb96ad 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 808fc410a1a..b76936fa401 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 a6901fefe92..1e8957ad7be 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 43f4b3390ea..dd737adebfa 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 502c8757743..1bfd71fba7d 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 47437ee9076..4c7ed9ba104 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 d302924ba60..029b0e7693e 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 7c0d778bbf1..3fd3c335b09 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 f8f2aeaf811..72e94823c3d 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 4d4cb9e3e91..d2e2b3bafdf 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 237621a9f0d..23edbdd204f 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 52bf4929092..b9b6447444c 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 58b1f841e94..9f8fbfcff01 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 75e10b74082..6a573981131 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 a6f7a02117e..35192a381dc 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 -- GitLab