Merge pull request #51 from edx/ammar/add-pylintrc

run pylint and pep8 quality tasks

.pep8

 [pep8]
 max-line-length = 120
-exclude = dependencies, watchdog.py, veda_deliver_xuetang.py, scripts
+exclude = dependencies, watchdog.py, veda_deliver_xuetang.py, scripts, settings, migrations

Makefile

 PACKAGES = VEDA VEDA_OS01 control frontend youtube_callback scripts
+PEP8_THRESHOLD=50
+PYLINT_THRESHOLD=855
 
 production-requirements:
 	pip install -r requirements.txt
@@ -12,7 +14,7 @@ migrate:
 static:
 	python manage.py collectstatic --noinput
 
-validate: test ## Run tests and quality checks
+validate: test quality
 
 test: clean
 	coverage run -m pytest --durations=10
@@ -22,6 +24,10 @@ test: clean
 clean:
 	coverage erase
 
-quality:
-	pep8 --config=.pep8 $(PACKAGES) *.py
-	pylint --rcfile=pylintrc $(PACKAGES) *.py
+quality: quality_pep8 quality_pylint
+
+quality_pep8:
+	paver run_pep8 -l ${PEP8_THRESHOLD}
+
+quality_pylint:
+	paver run_pylint -l ${PYLINT_THRESHOLD}

VEDA/tests/test_utils.py

@@ -64,7 +64,7 @@ class UtilTests(TestCase):
             'expected_url': 'http://api.cielo24/add/job?c=3&d=4'
         },
         {
-            'urls': ('http://api.cielo24','add/job'),
+            'urls': ('http://api.cielo24', 'add/job'),
             'params': {'p': 100},
             'expected_url': 'http://api.cielo24/add/job?p=100'
         },

control/tests/test_file_ingest.py

@@ -7,7 +7,7 @@ import unittest
 from django.test import TestCase
 import requests
 
-from veda_file_ingest import VedaIngest, VideoProto
+from control.veda_file_ingest import VedaIngest, VideoProto
 
 """
 This is an API connection test

control/tests/test_reporting.py

@@ -12,7 +12,7 @@ A basic unittest for the "Course Addition Tool"
 sys.path.append(
     os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 )
-from veda_utils import Report
+from control.veda_utils import Report
 
 
 class TestReporting(TestCase):

control/tests/test_val.py

@@ -10,8 +10,8 @@ import requests
 import responses
 
 from control.veda_val import VALAPICall
-from veda_file_ingest import VideoProto
 from VEDA import utils
+from control.veda_file_ingest import VideoProto
 
 
 requests.packages.urllib3.disable_warnings()

control/veda_deliver.py

@@ -33,7 +33,6 @@ except ImportError:
 LOGGER = logging.getLogger(__name__)
 
 
-
 try:
     boto.config.add_section('Boto')
 except:

control/veda_utils.py

@@ -15,6 +15,7 @@ from control.control_env import *
 from control.veda_encode import VedaEncode
 from VEDA.utils import get_config
 
+
 class EmailAlert():
 
     def __init__(self, **kwargs):

frontend/tests/test_abvid_reporting.py

@@ -15,6 +15,7 @@ sys.path.append(
 import abvid_reporting
 from frontend.course_validate import VEDACat
 
+
 class TestVariables(TestCase):
 
     def setUp(self):

pavelib/__init__.py

+"""
+paver commands
+"""
+from . import quality

pavelib/quality.py

+"""
+Check code quality
+"""
+import json
+import os
+import re
+from string import join
+from paver.easy import BuildFailure, call_task, cmdopts, needs, sh, task
+
+ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+REPORTS_DIR = os.path.join(ROOT_DIR, 'reports')
+
+PACKAGES = [
+    'VEDA',
+    'VEDA_OS01',
+    'control',
+    'frontend',
+    'youtube_callback',
+    'scripts',
+    'bin',
+]
+
+
+@task
+@cmdopts([
+    ("limit=", "l", "limit for number of acceptable violations"),
+])
+def run_pep8(options):  # pylint: disable=unused-argument
+    """
+    Run pep8 on system code.
+    Fail the task if any violations are found.
+    """
+    violations_limit = int(getattr(options, 'limit', -1))
+
+    sh('pep8 . | tee {report_dir}/pep8.report'.format(report_dir=REPORTS_DIR))
+
+    num_violations, __ = _count_pep8_violations(
+        '{report_dir}/pep8.report'.format(report_dir=REPORTS_DIR)
+    )
+
+    violations_message = '{violations_base_message}{violations_limit_message}'.format(
+        violations_base_message='Too many pep8 violations. Number of pep8 violations: {}. '.format(num_violations),
+        violations_limit_message='The limit is {violations_limit}. '.format(violations_limit=violations_limit),
+    )
+    print violations_message
+
+    # Fail if number of violations is greater than the limit
+    if num_violations > violations_limit > -1:
+        raise BuildFailure(violations_message)
+
+
+def _count_pep8_violations(report_file):
+    """
+    Returns a tuple of (num_violations, violations_list) for all
+    pep8 violations in the given report_file.
+    """
+    with open(report_file) as f:
+        violations_list = f.readlines()
+
+    num_lines = len(violations_list)
+    return num_lines, violations_list
+
+
+@task
+@cmdopts([
+    ('errors', 'e', 'Check for errors only'),
+    ('limit=', 'l', 'limit for number of acceptable violations'),
+])
+def run_pylint(options):
+    """
+    Run pylint on system code. When violations limit is passed in,
+    fail the task if too many violations are found.
+    """
+    num_violations = 0
+    violations_limit = int(getattr(options, 'limit', -1))
+    errors = getattr(options, 'errors', False)
+
+    flags = []
+    if errors:
+        flags.append('--errors-only')
+
+    sh(
+        'PYTHONPATH={python_path} pylint {packages} {flags} --msg-template={msg_template} | '
+        'tee {report_dir}/pylint.report'.format(
+            python_path=ROOT_DIR,
+            packages=' '.join(PACKAGES),
+            flags=' '.join(flags),
+            msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"',
+            report_dir=REPORTS_DIR
+        )
+    )
+
+    num_violations = _count_pylint_violations(
+        '{report_dir}/pylint.report'.format(report_dir=REPORTS_DIR)
+    )
+
+    violations_message = '{violations_base_message}{violations_limit_message}'.format(
+        violations_base_message='Too many pylint violations. Number of pylint violations: {}. '.format(num_violations),
+        violations_limit_message='The limit is {violations_limit}.'.format(violations_limit=violations_limit)
+    )
+    print violations_message
+
+    # Fail if number of violations is greater than the limit
+    if num_violations > violations_limit > -1:
+        raise BuildFailure(violations_message)
+
+
+def _count_pylint_violations(report_file):
+    """
+    Parses a pylint report line-by-line and determines the number of violations reported
+    """
+    num_violations_report = 0
+    # An example string:
+    # scripts/reencode_crawler.py:57: [C0303(trailing-whitespace), ] Trailing whitespace
+    pylint_pattern = re.compile(r'.(\d+):\ \[(\D\d+.+\]).')
+
+    for line in open(report_file):
+        violation_list_for_line = pylint_pattern.split(line)
+        # If the string is parsed into four parts, then we've found a violation. Example of split parts:
+        # test file, line number, violation name, violation details
+        if len(violation_list_for_line) == 4:
+            num_violations_report += 1
+
+    return num_violations_report

pavement.py

+import os
+import sys
+
+# Ensure that we can import pavelib
+sys.path.insert(0, os.path.dirname(__file__))
+
+from pavelib import *

test_requirements.txt

@@ -9,3 +9,8 @@ responses==0.6.1
 pytest==3.0.6
 pytest-django==3.1.2
 pytest-django-ordering==1.0.1
+Paver==1.2.4
+pylint==1.7.1
+pylint-celery==0.3
+pylint-django==0.7.2
+edx-lint==0.5.4