diff --git a/docs/en_us/developers/source/testing/code-quality.rst b/docs/en_us/developers/source/testing/code-quality.rst index bbcb180a1b0fda922e274cd0adb988be228d75f7..d1b4a044ab7ac9db8286529e91f0afd85c691cfe 100644 --- a/docs/en_us/developers/source/testing/code-quality.rst +++ b/docs/en_us/developers/source/testing/code-quality.rst @@ -18,7 +18,10 @@ in the process of introducing other changes. To check the quality of your pull request, just go to the top level of the edx-platform codebase and run:: - $ rake quality + $ paver run_quality + +Note: The rake commands ``rake quality``, ``rake pep8``, and ``rake pylint`` are now deprecated +to ``paver run_quality``, ``paver run_pep8``, and ``paver run_pylint`` respectively. This will print a report of the quality violations that your branch has made. diff --git a/pavelib/__init__.py b/pavelib/__init__.py index f70eb31bd22fd67760d3024256748e2233ce82f9..1f1af5c3af9053f518163b31a851ef28da0fafae 100644 --- a/pavelib/__init__.py +++ b/pavelib/__init__.py @@ -1,5 +1,5 @@ """ paver commands """ -__all__ = ["assets", "servers", "docs", "prereqs"] -from . import assets, servers, docs, prereqs +__all__ = ["assets", "servers", "docs", "prereqs", "quality"] +from . import assets, servers, docs, prereqs, quality diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 753233f6b2bbebfcf485e0204481bb1026ca38ca..f4ab708e8aa2d2bd78c19dcfa531545ae8e7514d 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -104,6 +104,7 @@ def install_node_prereqs(): sh('npm install') +@task def install_python_prereqs(): """ Installs Python prerequisites diff --git a/pavelib/quality.py b/pavelib/quality.py new file mode 100644 index 0000000000000000000000000000000000000000..3850039d4c2ad8f2e5324bf5e86ddaa9272d1fff --- /dev/null +++ b/pavelib/quality.py @@ -0,0 +1,153 @@ +""" +Check code quality using pep8, pylint, and diff_quality. +""" +from paver.easy import sh, task, cmdopts, needs +import os +import errno +from .utils.envs import Env + + +def get_or_make_dir(directory_path): + """ + Ensure that a directory exists, and return its path + """ + try: + os.makedirs(directory_path) + except OSError as err: + if err.errno != errno.EEXIST: + # If we get an error other than one that says + # that the file already exists + raise + + return directory_path + + +@task +@needs('pavelib.prereqs.install_python_prereqs') +@cmdopts([ + ("system=", "s", "System to act on"), + ("errors", "e", "Check for errors only"), +]) +def run_pylint(options): + """ + Run pylint on system code + """ + system = getattr(options, 'system', 'lms') + errors = getattr(options, 'errors', False) + + # Directory to put the pylint report in. + # This makes the folder if it doesn't already exist. + report_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, system)) + + flags = '-E' if errors else '' + + 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}/djangoapps:{system}/" + "lib:common/djangoapps:common/lib".format( + system=system + ) + ) + + sh( + "{pythonpath_prefix} pylint {flags} -f parseable {apps} | " + "tee {report_dir}/pylint.report".format( + pythonpath_prefix=pythonpath_prefix, + flags=flags, + apps=apps_list, + report_dir=report_dir + ) + ) + + +@task +@needs('pavelib.prereqs.install_python_prereqs') +@cmdopts([ + ("system=", "s", "System to act on"), +]) +def run_pep8(options): + """ + Run pep8 on system code + """ + system = getattr(options, 'system', 'lms') + + # Directory to put the pep8 report in. + # This makes the folder if it doesn't already exist. + report_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, system)) + + sh('pep8 {system} | tee {report_dir}/pep8.report'.format(system=system, report_dir=report_dir)) + + +@task +@needs('pavelib.prereqs.install_python_prereqs') +def run_quality(): + """ + Build the html diff quality reports, and print the reports to the console. + """ + + # Directory to put the diff reports in. + # This makes the folder if it doesn't already exist. + dquality_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, "diff_quality")) + + # Generage diff-quality html report for pep8, and print to console + # If pep8 reports exist, use those + # Otherwise, `diff-quality` will call pep8 itself + + pep8_files = [] + for subdir, _dirs, files in os.walk(os.path.join(Env.REPORT_DIR)): + for f in files: + if f == "pep8.report": + pep8_files.append(os.path.join(subdir, f)) + + pep8_reports = u' '.join(pep8_files) + + sh( + "diff-quality --violations=pep8 --html-report {dquality_dir}/" + "diff_quality_pep8.html {pep8_reports}".format( + dquality_dir=dquality_dir, pep8_reports=pep8_reports) + ) + + sh( + "diff-quality --violations=pep8 {pep8_reports}".format( + pep8_reports=pep8_reports) + ) + + # Generage diff-quality html report for pylint, and print to console + # If pylint reports exist, use those + # Otherwise, `diff-quality` will call pylint itself + + pylint_files = [] + for subdir, _dirs, files in os.walk(os.path.join(Env.REPORT_DIR)): + for f in files: + if f == "pylint.report": + pylint_files.append(os.path.join(subdir, f)) + + pylint_reports = u' '.join(pylint_files) + + pythonpath_prefix = ( + "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:" + "common:common/djangoapps:common/lib" + ) + + sh( + "{pythonpath_prefix} diff-quality --violations=pylint --html-report " + "{dquality_dir}/diff_quality_pylint.html {pylint_reports}".format( + pythonpath_prefix=pythonpath_prefix, + dquality_dir=dquality_dir, + pylint_reports=pylint_reports + ) + ) + + sh( + "{pythonpath_prefix} diff-quality --violations=pylint {pylint_reports}".format( + pythonpath_prefix=pythonpath_prefix, + pylint_reports=pylint_reports + ) + ) diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index 0f6930750308e1112725bec7286b623899b14e18..2e4b96e5cd6035b9d74d68dc5ca5cda41223dd3d 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -17,6 +17,9 @@ class Env(object): # Root of the git repository (edx-platform) REPO_ROOT = path(__file__).abspath().parent.parent.parent + # Reports Directory + REPORT_DIR = REPO_ROOT / 'reports' + # Service variant (lms, cms, etc.) configured with an environment variable # We use this to determine which envs.json file to load. SERVICE_VARIANT = os.environ.get('SERVICE_VARIANT', None) diff --git a/rakelib/quality.rake b/rakelib/quality.rake deleted file mode 100644 index 2624e5e7c23848a7d563caf4acca94b8999f25d7..0000000000000000000000000000000000000000 --- a/rakelib/quality.rake +++ /dev/null @@ -1,72 +0,0 @@ -def run_pylint(system, report_dir, flags='') - apps = Dir["#{system}", "#{system}/djangoapps/*"] - if system != 'lms' - apps += Dir["#{system}/lib/*"] - end - - apps = apps.map do |app| - File.basename(app) - end.select do |app| - app !=~ /.pyc$/ - end.map do |app| - if app =~ /.py$/ - app.gsub('.py', '') - else - app - end - end - - pythonpath_prefix = "PYTHONPATH=#{system}:#{system}/djangoapps:#{system}/lib:common/djangoapps:common/lib" - sh("#{pythonpath_prefix} pylint #{flags} -f parseable #{apps.join(' ')} | tee #{report_dir}/pylint.report") -end - - -[:lms, :cms, :common].each do |system| - report_dir = report_dir_path(system) - directory report_dir - - namespace :pylint do - namespace system do - desc "Run pylint checking for #{system} checking for errors only, and aborting if there are any" - task :errors do - run_pylint(system, report_dir, '-E') - end - end - - desc "Run pylint on all #{system} code" - task system => [report_dir, :install_python_prereqs] do - run_pylint(system, report_dir) - end - end - task :pylint => :"pylint:#{system}" - - namespace :pep8 do - desc "Run pep8 on all #{system} code" - task system => [report_dir, :install_python_prereqs] do - sh("pep8 #{system} | tee #{report_dir}/pep8.report") - end - end - task :pep8 => :"pep8:#{system}" -end - -dquality_dir = File.join(REPORT_DIR, "diff_quality") -directory dquality_dir - -desc "Build the html diff quality reports, and print the reports to the console." -task :quality => [dquality_dir, :install_python_prereqs] do - - # Generage diff-quality html report for pep8, and print to console - # If pep8 reports exist, use those - # Otherwise, `diff-quality` will call pep8 itself - pep8_reports = FileList[File.join(REPORT_DIR, '**/pep8.report')].join(' ') - sh("diff-quality --violations=pep8 --html-report #{dquality_dir}/diff_quality_pep8.html #{pep8_reports}") - sh("diff-quality --violations=pep8 #{pep8_reports}") - - # Generage diff-quality html report for pylint, and print to console - # If pylint reports exist, use those - # Otherwise, `diff-quality` will call pylint itself - pylint_reports = FileList[File.join(REPORT_DIR, '**/pylint.report')].join(' ') - pythonpath_prefix = "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:common:common/djangoapps:common/lib" - sh("#{pythonpath_prefix} diff-quality --violations=pylint --html-report #{dquality_dir}/diff_quality_pylint.html #{pylint_reports}") - sh("#{pythonpath_prefix} diff-quality --violations=pylint #{pylint_reports}") -end diff --git a/rakelib/quality_deprecated.rake b/rakelib/quality_deprecated.rake new file mode 100644 index 0000000000000000000000000000000000000000..6f5eead3754a132d57929c752cb57fe7c121d0e9 --- /dev/null +++ b/rakelib/quality_deprecated.rake @@ -0,0 +1,39 @@ +# quality tasks deprecated to paver + +require 'colorize' + +def deprecated(deprecated, deprecated_by) + + task deprecated do + + # Need to install paver dependencies for the commands to work! + sh("pip install -r requirements/edx/paver.txt") + + if deprecated.include? "extract" and ARGV.last.downcase == 'extract' + new_cmd = deprecated_by + " --extract" + else + new_cmd = deprecated_by + end + + puts("Task #{deprecated} has been deprecated. Use #{deprecated_by} instead.".red) + sh(new_cmd) + exit + end +end + +deprecated("pep8:cms", "paver run_pep8 --system=cms") +deprecated("pep8:lms", "paver run_pep8 --system=lms") +deprecated("pep8:common", "paver run_pep8 --system=common") +deprecated("pep8", "paver run_pep8") + +deprecated("pylint:cms", "paver run_pylint --system=cms") +deprecated("pylint:lms", "paver run_pylint --system=lms") +deprecated("pylint:common", "paver run_pylint --system=common") +deprecated("pylint", "paver run_pylint") + +deprecated("pylint:cms:errors", "paver run_pylint --system=cms --errors") +deprecated("pylint:lms:errors", "paver run_pylint --system=lms --errors") +deprecated("pylint:common:errors", "paver run_pylint --system=common --errors") +deprecated("pylint:errors", "paver run_pylint --errors") + +deprecated("quality", "paver run_quality")