From a557c5d368b543d3257106c2222bdac49824b865 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:18:50 -0800 Subject: [PATCH 01/15] Parameterize system list for quality checks --- pavelib/quality.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index a5c7eb4634..547bff0fc2 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -7,6 +7,8 @@ import re from .utils.envs import Env +ALL_SYSTEMS = 'lms,cms,common' + @task @needs('pavelib.prereqs.install_python_prereqs') @@ -18,7 +20,7 @@ def find_fixme(options): Run pylint on system code, only looking for fixme items. """ num_fixme = 0 - systems = getattr(options, 'system', 'lms,cms,common').split(',') + systems = getattr(options, 'system', ALL_SYSTEMS).split(',') for system in systems: # Directory to put the pylint report in. @@ -72,7 +74,7 @@ def run_pylint(options): num_violations = 0 violations_limit = int(getattr(options, 'limit', -1)) errors = getattr(options, 'errors', False) - systems = getattr(options, 'system', 'lms,cms,common').split(',') + systems = getattr(options, 'system', ALL_SYSTEMS).split(',') for system in systems: # Directory to put the pylint report in. @@ -149,7 +151,7 @@ def run_pep8(options): fail the task if too many violations are found. """ num_violations = 0 - systems = getattr(options, 'system', 'lms,cms,common').split(',') + systems = getattr(options, 'system', ALL_SYSTEMS).split(',') violations_limit = int(getattr(options, 'limit', -1)) for system in systems: From 19c767eb52e89bfada6c944c20060587a26310b8 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:23:55 -0800 Subject: [PATCH 02/15] Extend quality checks to cover more directories --- pavelib/quality.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index 547bff0fc2..8722b848b2 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -7,7 +7,7 @@ import re from .utils.envs import Env -ALL_SYSTEMS = 'lms,cms,common' +ALL_SYSTEMS = 'lms,cms,common,openedx,pavelib,scripts,docs' @task From c4a63a5eff5a467a522199c649d57dc519647bcd Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:24:38 -0800 Subject: [PATCH 03/15] Fix PEP8: E302 expected 2 blank lines --- openedx/core/djangoapps/user_api/accounts/tests/test_views.py | 1 + openedx/core/operations.py | 1 + 2 files changed, 2 insertions(+) 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 0d91b86da3..57ea85d0aa 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -13,6 +13,7 @@ from student.models import UserProfile TEST_PASSWORD = "test" + @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class TestAccountAPI(APITestCase): diff --git a/openedx/core/operations.py b/openedx/core/operations.py index 363d50c508..8f25868a13 100644 --- a/openedx/core/operations.py +++ b/openedx/core/operations.py @@ -10,6 +10,7 @@ def dump_memory(signum, frame): """Dump memory stats for the current process to a temp directory. Uses the meliae output format.""" scanner.dump_all_objects('{}/meliae.{}.{}.dump'.format(tempfile.gettempdir(), datetime.now().isoformat(), os.getpid())) + def install_memory_dumper(dump_signal=signal.SIGPROF): """ Install a signal handler on `signal` to dump memory stats for the current process. From c4cdd65744eaebff9cb00fbcda8951f98a735b41 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:26:37 -0800 Subject: [PATCH 04/15] Fix PEP8: E303 too many blank lines --- docs/en_us/enrollment_api/source/conf.py | 2 -- openedx/core/djangoapps/course_groups/tests/test_cohorts.py | 1 - pavelib/tests.py | 1 - 3 files changed, 4 deletions(-) diff --git a/docs/en_us/enrollment_api/source/conf.py b/docs/en_us/enrollment_api/source/conf.py index c76367e309..2410c2f8ab 100644 --- a/docs/en_us/enrollment_api/source/conf.py +++ b/docs/en_us/enrollment_api/source/conf.py @@ -54,8 +54,6 @@ if on_rtd: else: os.environ['DJANGO_SETTINGS_MODULE'] = 'lms' - - # -- General configuration ----------------------------------------------------- # Add any Sphinx extension module names here, as strings. They can be extensions diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 4f396d32d2..66a2b6322d 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -225,7 +225,6 @@ class TestCohorts(ModuleStoreTestCase): # get_cohort should return a group for user self.assertEquals(cohorts.get_cohort(user, course.id).name, "AutoGroup") - def test_auto_cohorting(self): """ Make sure cohorts.get_cohort() does the right thing with auto_cohort_groups diff --git a/pavelib/tests.py b/pavelib/tests.py index 11e82d9e5c..b1ad113742 100644 --- a/pavelib/tests.py +++ b/pavelib/tests.py @@ -204,7 +204,6 @@ def coverage(options): dir=directory )) - call_task('diff_coverage', options=dict(options)) From 5785fda7b13c499a0fb4480667528bd3904c8b41 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:27:41 -0800 Subject: [PATCH 05/15] Fix PEP8: W391 blank line at end of file --- docs/en_us/enrollment_api/source/conf.py | 2 -- docs/en_us/platform_api/source/conf.py | 2 -- openedx/core/djangoapps/user_api/tests/test_views.py | 1 - 3 files changed, 5 deletions(-) diff --git a/docs/en_us/enrollment_api/source/conf.py b/docs/en_us/enrollment_api/source/conf.py index 2410c2f8ab..ba1b50ba5f 100644 --- a/docs/en_us/enrollment_api/source/conf.py +++ b/docs/en_us/enrollment_api/source/conf.py @@ -70,5 +70,3 @@ exclude_patterns = ['build', 'links.rst'] project = u'edX Enrollment API Version 1' copyright = u'2015, edX' - - diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index 72fedfd9ed..fbd9cb05e2 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -83,5 +83,3 @@ project = u'edX Platform API Version 0.5 Alpha' copyright = u'2015, edX' exclude_patterns = ['build', 'links.rst'] - - diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 02eb0f3e90..0120750543 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -1599,4 +1599,3 @@ class UpdateEmailOptInTestCase(ApiTestCase, ModuleStoreTestCase): self.assertHttpBadRequest(response) with self.assertRaises(UserOrgTag.DoesNotExist): UserOrgTag.objects.get(user=self.user, org=self.course.id.org, key="email-optin") - From ed36b2db55718c4f9e53bc1c8fa894512666ab5e Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:28:43 -0800 Subject: [PATCH 06/15] Fix PEP8: E123 closing bracket does not match indentation of opening bracket's line --- pavelib/bok_choy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index cfc38e298a..ad97ff1323 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -95,8 +95,9 @@ def run_bokchoy(**opts): msg = colorize( 'green', 'Running tests using {default_store} modulestore.'.format( - default_store=test_suite.default_store) + default_store=test_suite.default_store, ) + ) print(msg) test_suite.run() From ef420ffd7c5339ca3e9a8dd5a042b7978817b669 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:31:35 -0800 Subject: [PATCH 07/15] Fix PEP8: E127 continuation line over-indented for visual indent --- .../paver_tests/test_paver_bok_choy_cmds.py | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py index 92bbf94ba7..964117cbb6 100644 --- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py +++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py @@ -14,15 +14,20 @@ class TestPaverBokChoyCmd(unittest.TestCase): def _expected_command(self, expected_text_append, expected_default_store=None): if expected_text_append: expected_text_append = "/" + expected_text_append - expected_statement = ("DEFAULT_STORE={default_store} SCREENSHOT_DIR='{repo_dir}/test_root/log' " - "BOK_CHOY_HAR_DIR='{repo_dir}/test_root/log/hars' " - "SELENIUM_DRIVER_LOG_DIR='{repo_dir}/test_root/log' " - "nosetests {repo_dir}/common/test/acceptance/tests{exp_text} " - "--with-xunit " - "--xunit-file={repo_dir}/reports/bok_choy/xunit.xml " - "--verbosity=2 ".format(default_store=expected_default_store, - repo_dir=REPO_DIR, - exp_text=expected_text_append)) + expected_statement = ( + "DEFAULT_STORE={default_store} " + "SCREENSHOT_DIR='{repo_dir}/test_root/log' " + "BOK_CHOY_HAR_DIR='{repo_dir}/test_root/log/hars' " + "SELENIUM_DRIVER_LOG_DIR='{repo_dir}/test_root/log' " + "nosetests {repo_dir}/common/test/acceptance/tests{exp_text} " + "--with-xunit " + "--xunit-file={repo_dir}/reports/bok_choy/xunit.xml " + "--verbosity=2 " + ).format( + default_store=expected_default_store, + repo_dir=REPO_DIR, + exp_text=expected_text_append, + ) return expected_statement def test_default_bokchoy(self): From 1a80097aeb00d13c878bc9563e880287602c5833 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:33:26 -0800 Subject: [PATCH 08/15] Fix PEP8: E112 expected an indented block --- pavelib/paver_tests/test_paver_quality.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 4c9e0d2ec4..fb8d3c7bed 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -31,9 +31,11 @@ class TestPaverQualityViolations(unittest.TestCase): @file_data('pylint_test_list.json') def test_pylint_parser_count_violations(self, value): - # Tests: - # * Different types of violations - # * One violation covering multiple lines + """ + Tests: + - Different types of violations + - One violation covering multiple lines + """ with open(self.f.name, 'w') as f: f.write(value) num = pavelib.quality._count_pylint_violations(f.name) From cdf7876a0e6715caea037e9b3c5da1c792913060 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:34:18 -0800 Subject: [PATCH 09/15] Fix PEP8: W293 blank line contains whitespace --- docs/en_us/platform_api/source/conf.py | 2 +- .../core/djangoapps/user_api/accounts/tests/test_views.py | 5 ----- pavelib/utils/test/suites/nose_suite.py | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index fbd9cb05e2..f4401836ae 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -8,7 +8,7 @@ import os from path import path import sys import mock - + MOCK_MODULES = ['lxml', 'requests', 'xblock', 'fields', 'xblock.fields', 'frament', 'xblock.fragment', 'webob', 'multidict', 'webob.multidict', 'core', 'xblock.core', 'runtime', 'xblock.runtime', 'sortedcontainers', 'contracts', 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 57ea85d0aa..c3335c183f 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -20,17 +20,12 @@ class TestAccountAPI(APITestCase): def setUp(self): super(TestAccountAPI, self).setUp() - self.anonymous_client = APIClient() - self.different_user = UserFactory.create(password=TEST_PASSWORD) self.different_client = APIClient() - self.staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD) self.staff_client = APIClient() - self.user = UserFactory.create(password=TEST_PASSWORD) - self.url = reverse("accounts_api", kwargs={'username': self.user.username}) def test_get_account_anonymous_user(self): diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py index 4d5ff04559..27b2d38cdc 100644 --- a/pavelib/utils/test/suites/nose_suite.py +++ b/pavelib/utils/test/suites/nose_suite.py @@ -141,7 +141,7 @@ class SystemTestSuite(NoseTestSuite): if self.root == 'lms': default_test_id += " {system}/tests.py".format(system=self.root) - + if self.root == 'cms': default_test_id += " {system}/tests/*".format(system=self.root) From 0b8fcc551beec16de3944c85948891bef94e690c Mon Sep 17 00:00:00 2001 From: stv Date: Mon, 16 Feb 2015 21:55:04 -0800 Subject: [PATCH 10/15] Fix PEP8: E713 test for membership should be 'not in' --- scripts/release.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/release.py b/scripts/release.py index 888c905c8e..8405e12efb 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -98,7 +98,7 @@ def ensure_pr_fetch(): """ modified = False remotes = git.remote().splitlines() - if not "edx" in remotes: + if 'edx' not in remotes: git.remote("add", "edx", "https://github.com/edx/edx-platform.git") modified = True # it would be nice to use the git-python API to do this, but it doesn't seem @@ -251,9 +251,9 @@ def ensure_github_creds(attempts=3): else: config = {} # update config - if not "credentials" in config: + if 'credentials' not in config: config["credentials"] = {} - if not "api.github.com" in config["credentials"]: + if 'api.github.com' not in config['credentials']: config["credentials"]["api.github.com"] = {} config["credentials"]["api.github.com"]["username"] = username config["credentials"]["api.github.com"]["token"] = token From 40cb7372982ff018a3b223c9aa1271cfa09cfd84 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:37:54 -0800 Subject: [PATCH 11/15] Fix PEP8: E128 continuation line under-indented for visual indent --- docs/en_us/platform_api/source/conf.py | 38 ++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index f4401836ae..36532a8f1b 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -9,12 +9,38 @@ from path import path import sys import mock -MOCK_MODULES = ['lxml', 'requests', 'xblock', 'fields', 'xblock.fields', -'frament', 'xblock.fragment', 'webob', 'multidict', 'webob.multidict', 'core', -'xblock.core', 'runtime', 'xblock.runtime', 'sortedcontainers', 'contracts', -'plugin', 'xblock.plugin', 'opaque_keys.edx.asides', 'asides', -'dogstats_wrapper', 'fs', 'fs.errors', 'edxmako', 'edxmako.shortcuts', -'shortcuts', 'crum', 'opaque_keys.edx.locator', 'LibraryLocator', 'Location'] +MOCK_MODULES = [ + 'lxml', + 'requests', + 'xblock', + 'fields', + 'xblock.fields', + 'frament', + 'xblock.fragment', + 'webob', + 'multidict', + 'webob.multidict', + 'core', + 'xblock.core', + 'runtime', + 'xblock.runtime', + 'sortedcontainers', + 'contracts', + 'plugin', + 'xblock.plugin', + 'opaque_keys.edx.asides', + 'asides', + 'dogstats_wrapper', + 'fs', + 'fs.errors', + 'edxmako', + 'edxmako.shortcuts', + 'shortcuts', + 'crum', + 'opaque_keys.edx.locator', + 'LibraryLocator', + 'Location', +] for mod_name in MOCK_MODULES: sys.modules[mod_name] = mock.Mock() From f994912cd86e4c13f110d7199bd2fdb3f386e335 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 15 Feb 2015 16:38:25 -0800 Subject: [PATCH 12/15] Fix PEP8: W291 trailing whitespace --- docs/en_us/platform_api/source/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en_us/platform_api/source/conf.py b/docs/en_us/platform_api/source/conf.py index 36532a8f1b..41df4a3c25 100644 --- a/docs/en_us/platform_api/source/conf.py +++ b/docs/en_us/platform_api/source/conf.py @@ -42,7 +42,7 @@ MOCK_MODULES = [ 'Location', ] -for mod_name in MOCK_MODULES: +for mod_name in MOCK_MODULES: sys.modules[mod_name] = mock.Mock() on_rtd = os.environ.get('READTHEDOCS', None) == 'True' From 49925873f5bd34174680a3dc637162b6d547d90f Mon Sep 17 00:00:00 2001 From: stv Date: Fri, 27 Feb 2015 10:00:39 -0800 Subject: [PATCH 13/15] Fix PEP8: W292 no newline at end of file --- openedx/core/djangoapps/user_api/accounts/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 58759e062b..ff29ec51bf 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -157,4 +157,4 @@ class AccountView(APIView): } validation_errors['field_errors'] = field_errors - return validation_errors \ No newline at end of file + return validation_errors From 747756a9116ada2d20b3edaafdac36e4d9d4ae08 Mon Sep 17 00:00:00 2001 From: stv Date: Sun, 22 Feb 2015 12:00:00 -0800 Subject: [PATCH 14/15] Refactor quality checks --- pavelib/quality.py | 203 ++++++++++++++++++++++-------------- pavelib/utils/test/utils.py | 1 + 2 files changed, 124 insertions(+), 80 deletions(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index 8722b848b2..26862d8674 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -1,13 +1,98 @@ """ Check code quality using pep8, pylint, and diff_quality. """ +from __future__ import print_function from paver.easy import sh, task, cmdopts, needs, BuildFailure import os import re from .utils.envs import Env -ALL_SYSTEMS = 'lms,cms,common,openedx,pavelib,scripts,docs' +DIRECTORIES_TOP_LEVEL_COMMON = { + 'common', + 'openedx', +} + +DIRECTORIES_TOP_LEVEL_SYSTEMS = { + 'cms', + 'docs', + 'lms', + 'pavelib', + 'scripts', +} + +DIRECTORIES_TOP_LEVEL_ALL = set() +DIRECTORIES_TOP_LEVEL_ALL.update(DIRECTORIES_TOP_LEVEL_COMMON) +DIRECTORIES_TOP_LEVEL_ALL.update(DIRECTORIES_TOP_LEVEL_SYSTEMS) + +DIRECTORIES_INNER = { + 'djangoapps', + 'lib', +} + + +def _get_path_list(system): + """ + Gather a list of subdirectories within the system + + :param system: the system directory to search; e.g. 'lms', 'cms' + :returns: a list of system subdirectories to be linted + """ + paths = [ + system, + ] + + for directory in DIRECTORIES_INNER: + try: + directories = os.listdir(os.path.join(system, directory)) + except OSError: + pass + else: + paths.extend([ + directory + for directory in directories + if os.path.isdir(os.path.join(system, directory, directory)) + ]) + + path_list = ' '.join(paths) + return path_list + + +def _get_python_path_prefix(directory_system): + """ + Build a string to specify the PYTHONPATH environment variable + + :param directory_system: the system directory to search; e.g. 'lms', 'cms' + :returns str: a PYTHONPATH environment string for the command line + """ + paths = { + directory_system, + } + directories_all = set(DIRECTORIES_TOP_LEVEL_COMMON) + directories_all.add(directory_system) + for system in directories_all: + for subsystem in DIRECTORIES_INNER: + path = os.path.join(system, subsystem) + paths.add(path) + paths = ':'.join(paths) + environment_python_path = "PYTHONPATH={paths}".format( + paths=paths, + ) + return environment_python_path + + +def _parse(options): + """ + Parse command line options, setting sane defaults + + :param options: a Paver Options object + :returns dict: containing: errors, system, limit + """ + return { + 'errors': getattr(options, 'errors', False), + 'systems': getattr(options, 'system', ','.join(DIRECTORIES_TOP_LEVEL_ALL)).split(','), + 'limit': int(getattr(options, 'limit', -1)), + } @task @@ -19,44 +104,28 @@ def find_fixme(options): """ Run pylint on system code, only looking for fixme items. """ - num_fixme = 0 - systems = getattr(options, 'system', ALL_SYSTEMS).split(',') + count = 0 + options = _parse(options) - for system in systems: - # Directory to put the pylint report in. - # This makes the folder if it doesn't already exist. + for system in options['systems']: report_dir = (Env.REPORT_DIR / system).makedirs_p() - - apps = [system] - - for directory in ['djangoapps', 'lib']: - dirs = os.listdir(os.path.join(system, directory)) - apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))]) - - apps_list = ' '.join(apps) - - pythonpath_prefix = ( - "PYTHONPATH={system}:{system}/lib" - "common/djangoapps:common/lib".format( - system=system - ) - ) - + path_list = _get_path_list(system) + environment_python_path = _get_python_path_prefix(system) sh( - "{pythonpath_prefix} pylint --disable R,C,W,E --enable=fixme " - "--msg-template={msg_template} {apps} " + "{environment_python_path} pylint --disable R,C,W,E --enable=fixme " + "--msg-template={msg_template} {path_list} " "| tee {report_dir}/pylint_fixme.report".format( - pythonpath_prefix=pythonpath_prefix, + environment_python_path=environment_python_path, msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"', - apps=apps_list, + path_list=path_list, report_dir=report_dir ) ) + count += _count_pylint_violations( + "{report_dir}/pylint_fixme.report".format(report_dir=report_dir) + ) - num_fixme += _count_pylint_violations( - "{report_dir}/pylint_fixme.report".format(report_dir=report_dir)) - - print("Number of pylint fixmes: " + str(num_fixme)) + print("Number of pylint fixmes: " + str(count)) @task @@ -72,52 +141,34 @@ def run_pylint(options): fail the task if too many violations are found. """ num_violations = 0 - violations_limit = int(getattr(options, 'limit', -1)) - errors = getattr(options, 'errors', False) - systems = getattr(options, 'system', ALL_SYSTEMS).split(',') + options = _parse(options) - for system in systems: - # Directory to put the pylint report in. - # This makes the folder if it doesn't already exist. + for system in options['systems']: report_dir = (Env.REPORT_DIR / system).makedirs_p() flags = [] - if errors: + if options['errors']: flags.append("--errors-only") - apps = [system] - - for directory in ['lib']: - dirs = os.listdir(os.path.join(system, directory)) - apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))]) - - apps_list = ' '.join(apps) - - pythonpath_prefix = ( - "PYTHONPATH={system}:{system}/djangoapps:{system}/" - "lib:common/djangoapps:common/lib".format( - system=system - ) - ) - + path_list = _get_path_list(system) + environment_python_path = _get_python_path_prefix(system) sh( - "{pythonpath_prefix} pylint {flags} --msg-template={msg_template} {apps} | " + "{environment_python_path} pylint {flags} --msg-template={msg_template} {path_list} | " "tee {report_dir}/pylint.report".format( - pythonpath_prefix=pythonpath_prefix, - flags=" ".join(flags), + environment_python_path=environment_python_path, + flags=' '.join(flags), msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"', - apps=apps_list, + path_list=path_list, report_dir=report_dir ) ) - num_violations += _count_pylint_violations( "{report_dir}/pylint.report".format(report_dir=report_dir)) print("Number of pylint violations: " + str(num_violations)) - if num_violations > violations_limit > -1: + if num_violations > options['limit'] > -1: raise Exception("Failed. Too many pylint violations. " - "The limit is {violations_limit}.".format(violations_limit=violations_limit)) + "The limit is {violations_limit}.".format(violations_limit=options['limit'])) def _count_pylint_violations(report_file): @@ -150,24 +201,19 @@ def run_pep8(options): Run pep8 on system code. When violations limit is passed in, fail the task if too many violations are found. """ - num_violations = 0 - systems = getattr(options, 'system', ALL_SYSTEMS).split(',') - violations_limit = int(getattr(options, 'limit', -1)) + options = _parse(options) + count = 0 - for system in systems: - # Directory to put the pep8 report in. - # This makes the folder if it doesn't already exist. + for system in options['systems']: report_dir = (Env.REPORT_DIR / system).makedirs_p() - sh('pep8 {system} | tee {report_dir}/pep8.report'.format(system=system, report_dir=report_dir)) - num_violations = num_violations + _count_pep8_violations( - "{report_dir}/pep8.report".format(report_dir=report_dir)) + count += _count_pep8_violations( + "{report_dir}/pep8.report".format(report_dir=report_dir) + ) - print("Number of pep8 violations: " + str(num_violations)) - # Fail the task if the violations limit has been reached - if num_violations > violations_limit > -1: - raise Exception("Failed. Too many pep8 violations. " - "The limit is {violations_limit}.".format(violations_limit=violations_limit)) + print("Number of pep8 violations: {count}".format(count=count)) + if count: + raise Exception("Failed. Too many pep8 violations.") def _count_pep8_violations(report_file): @@ -238,17 +284,17 @@ def run_quality(options): pylint_files = get_violations_reports("pylint") pylint_reports = u' '.join(pylint_files) - pythonpath_prefix = ( + environment_python_path = ( "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:" "common:common/djangoapps:common/lib" ) try: sh( - "{pythonpath_prefix} diff-quality --violations=pylint " + "{environment_python_path} diff-quality --violations=pylint " "{pylint_reports} {percentage_string} {compare_branch_string} " "--html-report {dquality_dir}/diff_quality_pylint.html ".format( - pythonpath_prefix=pythonpath_prefix, + environment_python_path=environment_python_path, pylint_reports=pylint_reports, percentage_string=percentage_string, compare_branch_string=compare_branch_string, @@ -272,10 +318,7 @@ def is_percentage_failure(error_message): paver with a subprocess return code error. If the subprocess exits with anything other than 1, raise a paver exception. """ - if "Subprocess return code: 1" not in error_message: - return False - else: - return True + return "Subprocess return code: 1" in error_message def get_violations_reports(violations_type): diff --git a/pavelib/utils/test/utils.py b/pavelib/utils/test/utils.py index 3a68599c06..fe1f78a21a 100644 --- a/pavelib/utils/test/utils.py +++ b/pavelib/utils/test/utils.py @@ -1,6 +1,7 @@ """ Helper functions for test tasks """ +from __future__ import print_function from paver.easy import sh, task, cmdopts from pavelib.utils.envs import Env import os From 41318981800789d374112a390cab185a24fbf6d0 Mon Sep 17 00:00:00 2001 From: stv Date: Thu, 26 Feb 2015 21:08:06 -0800 Subject: [PATCH 15/15] Remove "nopep" pragma --- cms/urls.py | 3 ++- common/djangoapps/heartbeat/urls.py | 4 +++- common/djangoapps/pipeline_js/urls.py | 4 +++- lms/djangoapps/class_dashboard/urls.py | 4 +++- lms/djangoapps/django_comment_client/base/urls.py | 4 +++- lms/djangoapps/django_comment_client/forum/urls.py | 4 +++- lms/djangoapps/django_comment_client/urls.py | 4 +++- lms/djangoapps/instructor/views/api_urls.py | 4 +++- lms/djangoapps/shoppingcart/urls.py | 4 +++- lms/djangoapps/survey/urls.py | 4 +++- lms/urls.py | 4 +++- 11 files changed, 32 insertions(+), 11 deletions(-) diff --git a/cms/urls.py b/cms/urls.py index 38cfef37d7..a505636afb 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -12,7 +12,8 @@ COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format( # Pattern to match a library key only LIBRARY_KEY_PATTERN = r'(?Plibrary-v1:[^/+]+\+[^/+]+)' -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'), url(r'^transcripts/download$', 'contentstore.views.download_transcripts', name='download_transcripts'), diff --git a/common/djangoapps/heartbeat/urls.py b/common/djangoapps/heartbeat/urls.py index 6a0be757c9..c2fbdaa73e 100644 --- a/common/djangoapps/heartbeat/urls.py +++ b/common/djangoapps/heartbeat/urls.py @@ -1,5 +1,7 @@ from django.conf.urls import url, patterns -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', + url(r'^$', 'heartbeat.views.heartbeat', name='heartbeat'), ) diff --git a/common/djangoapps/pipeline_js/urls.py b/common/djangoapps/pipeline_js/urls.py index 1806680b56..1fc732bcec 100644 --- a/common/djangoapps/pipeline_js/urls.py +++ b/common/djangoapps/pipeline_js/urls.py @@ -3,7 +3,9 @@ URL patterns for Javascript files used to load all of the XModule JS in one wad. """ from django.conf.urls import url, patterns -urlpatterns = patterns('pipeline_js.views', # nopep8 +urlpatterns = patterns( + 'pipeline_js.views', + url(r'^files\.json$', 'xmodule_js_files', name='xmodule_js_files'), url(r'^xmodule\.js$', 'requirejs_xmodule', name='requirejs_xmodule'), ) diff --git a/lms/djangoapps/class_dashboard/urls.py b/lms/djangoapps/class_dashboard/urls.py index fbb3eaf207..ca78be6a30 100644 --- a/lms/djangoapps/class_dashboard/urls.py +++ b/lms/djangoapps/class_dashboard/urls.py @@ -6,7 +6,9 @@ from django.conf.urls import patterns, url from django.conf import settings COURSE_ID_PATTERN = settings.COURSE_ID_PATTERN -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', + # Json request data for metrics for entire course url(r'^{}/all_sequential_open_distrib$'.format(settings.COURSE_ID_PATTERN), 'class_dashboard.views.all_sequential_open_distrib', name="all_sequential_open_distrib"), diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index bb850e420b..e49278ad7c 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -1,6 +1,8 @@ from django.conf.urls.defaults import url, patterns -urlpatterns = patterns('django_comment_client.base.views', # nopep8 +urlpatterns = patterns( + 'django_comment_client.base.views', + url(r'upload$', 'upload', name='upload'), url(r'threads/(?P[\w\-]+)/update$', 'update_thread', name='update_thread'), url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), diff --git a/lms/djangoapps/django_comment_client/forum/urls.py b/lms/djangoapps/django_comment_client/forum/urls.py index 863267fde9..cb11d99571 100644 --- a/lms/djangoapps/django_comment_client/forum/urls.py +++ b/lms/djangoapps/django_comment_client/forum/urls.py @@ -1,6 +1,8 @@ from django.conf.urls.defaults import url, patterns -urlpatterns = patterns('django_comment_client.forum.views', # nopep8 +urlpatterns = patterns( + 'django_comment_client.forum.views', + url(r'users/(?P\w+)/followed$', 'followed_threads', name='followed_threads'), url(r'users/(?P\w+)$', 'user_profile', name='user_profile'), url(r'^(?P[\w\-.]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), diff --git a/lms/djangoapps/django_comment_client/urls.py b/lms/djangoapps/django_comment_client/urls.py index 98700da4ab..eee27ada4b 100644 --- a/lms/djangoapps/django_comment_client/urls.py +++ b/lms/djangoapps/django_comment_client/urls.py @@ -1,6 +1,8 @@ from django.conf.urls.defaults import url, patterns, include -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', + url(r'forum/?', include('django_comment_client.forum.urls')), url(r'', include('django_comment_client.base.urls')), ) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 7fcc76923d..24c0df81a9 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -5,7 +5,9 @@ Instructor API endpoint urls. from django.conf.urls import patterns, url -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', + url(r'^students_update_enrollment$', 'instructor.views.api.students_update_enrollment', name="students_update_enrollment"), url(r'^register_and_enroll_students$', diff --git a/lms/djangoapps/shoppingcart/urls.py b/lms/djangoapps/shoppingcart/urls.py index 79e83bc7e5..9363521ddc 100644 --- a/lms/djangoapps/shoppingcart/urls.py +++ b/lms/djangoapps/shoppingcart/urls.py @@ -1,7 +1,9 @@ from django.conf.urls import patterns, url from django.conf import settings -urlpatterns = patterns('shoppingcart.views', # nopep8 +urlpatterns = patterns( + 'shoppingcart.views', + url(r'^postpay_callback/$', 'postpay_callback'), # Both the ~accept and ~reject callback pages are handled here url(r'^receipt/(?P[0-9]*)/$', 'show_receipt'), url(r'^donation/$', 'donate', name='donation'), diff --git a/lms/djangoapps/survey/urls.py b/lms/djangoapps/survey/urls.py index 727602e9d4..ad986159e4 100644 --- a/lms/djangoapps/survey/urls.py +++ b/lms/djangoapps/survey/urls.py @@ -5,7 +5,9 @@ URL mappings for the Survey feature from django.conf.urls import patterns, url -urlpatterns = patterns('survey.views', # nopep8 +urlpatterns = patterns( + 'survey.views', + url(r'^(?P[0-9A-Za-z]+)/$', 'view_survey', name='view_survey'), url(r'^(?P[0-9A-Za-z]+)/answers/$', 'submit_answers', name='submit_answers'), ) diff --git a/lms/urls.py b/lms/urls.py index dbe41e1f3e..0b324ca9aa 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -12,7 +12,9 @@ if settings.DEBUG or settings.FEATURES.get('ENABLE_DJANGO_ADMIN_SITE'): # Use urlpatterns formatted as within the Django docs with first parameter "stuck" to the open parenthesis # pylint: disable=bad-continuation -urlpatterns = ('', # nopep8 +urlpatterns = ( + '', + # certificate view url(r'^update_certificate$', 'certificates.views.update_certificate'), url(r'^request_certificate$', 'certificates.views.request_certificate'),