Merge pull request #3708 from edx/clytwynec/rake2paver-quality

Switch quality tasks to paver

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.
 

pavelib/__init__.py

 """
 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

pavelib/prereqs.py

@@ -104,6 +104,7 @@ def install_node_prereqs():
     sh('npm install')
 
 
+@task
 def install_python_prereqs():
     """
     Installs Python prerequisites

pavelib/quality.py

+"""
+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
+        )
+    )

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)

rakelib/quality.rake

-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

rakelib/quality_deprecated.rake

+# 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")