Unverified Commit bf146de0 by John Eskew Committed by GitHub

Merge pull request #16585 from edx/jeskew/enable_pylint_again

Fix the systems parsing code for pylint.
parents d2d76439 c78ccf8f
......@@ -8,7 +8,7 @@ import unittest
import paver.easy
import paver.tasks
from ddt import ddt, file_data
from ddt import ddt, file_data, data, unpack
from mock import MagicMock, mock_open, patch
from path import Path as path
from paver.easy import BuildFailure
......@@ -60,6 +60,34 @@ class TestPaverQualityViolations(unittest.TestCase):
self.assertEqual(num, 2)
@ddt
class TestPaverQualityOptions(unittest.TestCase):
"""
Tests the paver pylint command-line options parsing.
"""
@data(
({'limit': '5500'}, (-1, 5500, False, pavelib.quality.ALL_SYSTEMS.split(','))),
({'limit': '1000:5500'}, (1000, 5500, False, pavelib.quality.ALL_SYSTEMS.split(','))),
({'limit': '1:2:3:4:5'}, (1, 2, False, pavelib.quality.ALL_SYSTEMS.split(','))),
({'system': 'lms,cms'}, (-1, -1, False, ['lms', 'cms'])),
(
{'limit': '2000:5000', 'errors': True, 'system': 'lms,cms,openedx'},
(2000, 5000, True, ['lms', 'cms', 'openedx'])
),
)
@unpack
def test_pylint_parser_other_string(self, options, expected_values):
class PaverOptions(object):
"""
Simple options class to mimick paver's Namespace object.
"""
def __init__(self, d):
self.__dict__ = d
paver_options = PaverOptions(options)
returned_values = pavelib.quality._parse_pylint_options(paver_options) # pylint: disable=protected-access
self.assertEqual(returned_values, expected_values)
class TestPaverReportViolationsCounts(unittest.TestCase):
"""
For testing utility functions for getting counts from reports for
......
......@@ -6,7 +6,6 @@ Check code quality using pep8, pylint, and diff_quality.
import json
import os
import re
from string import join
from paver.easy import BuildFailure, call_task, cmdopts, needs, sh, task
......@@ -15,13 +14,7 @@ from openedx.core.djangolib.markup import HTML
from .utils.envs import Env
from .utils.timer import timed
ALL_SYSTEMS = [
'cms',
'common',
'lms',
'openedx',
'pavelib',
]
ALL_SYSTEMS = 'lms,cms,common,openedx,pavelib'
def top_python_dirs(dirname):
......@@ -55,7 +48,7 @@ def find_fixme(options):
Run pylint on system code, only looking for fixme items.
"""
num_fixme = 0
systems = getattr(options, 'system', '').split(',') or ALL_SYSTEMS
systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
for system in systems:
# Directory to put the pylint report in.
......@@ -92,7 +85,7 @@ def find_fixme(options):
@cmdopts([
("system=", "s", "System to act on"),
("errors", "e", "Check for errors only"),
("limit=", "l", "limit for number of acceptable violations"),
("limit=", "l", "Limits for number of acceptable violations - either <upper> or <lower>:<upper>"),
])
@timed
def run_pylint(options):
......@@ -100,14 +93,12 @@ 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)
systems = getattr(options, 'system', '').split(',') or ALL_SYSTEMS
lower_violations_limit, upper_violations_limit, errors, systems = _parse_pylint_options(options)
# Make sure the metrics subdirectory exists
Env.METRICS_DIR.makedirs_p()
num_violations = 0
for system in systems:
# Directory to put the pylint report in.
# This makes the folder if it doesn't already exist.
......@@ -147,10 +138,45 @@ def run_pylint(options):
with open(Env.METRICS_DIR / "pylint", "w") as f:
f.write(violations_count_str)
# Fail number of violations is greater than the limit
if num_violations > violations_limit > -1:
raise BuildFailure("Failed. Too many pylint violations. "
"The limit is {violations_limit}.".format(violations_limit=violations_limit))
# Fail when number of violations is less than the lower limit,
# which likely means that pylint did not run successfully.
# If pylint *did* run successfully, then great! Modify the lower limit.
if num_violations < lower_violations_limit > -1:
raise BuildFailure(
"Failed. Too few pylint violations. "
"Expected to see at least {lower_limit} pylint violations. "
"Either pylint is not running correctly -or- "
"the limits should be lowered and/or the lower limit should be removed.".format(
lower_limit=lower_violations_limit
)
)
# Fail when number of violations is greater than the upper limit.
if num_violations > upper_violations_limit > -1:
raise BuildFailure(
"Failed. Too many pylint violations. "
"The limit is {upper_limit}.".format(upper_limit=upper_violations_limit)
)
def _parse_pylint_options(options):
"""
Parse the options passed to run_pylint.
"""
lower_violations_limit = upper_violations_limit = -1
violations_limit = getattr(options, 'limit', '').split(':')
if violations_limit[0]:
# Limit was specified.
if len(violations_limit) == 1:
# Only upper limit was specified.
upper_violations_limit = int(violations_limit[0])
else:
# Upper and lower limits were both specified.
lower_violations_limit = int(violations_limit[0])
upper_violations_limit = int(violations_limit[1])
errors = getattr(options, 'errors', False)
systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
return lower_violations_limit, upper_violations_limit, errors, systems
def _count_pylint_violations(report_file):
......@@ -244,7 +270,7 @@ def run_complexity():
Uses radon to examine cyclomatic complexity.
For additional details on radon, see http://radon.readthedocs.org/
"""
system_string = join(ALL_SYSTEMS, '/ ') + '/'
system_string = '/ '.join(ALL_SYSTEMS.split(',')) + '/'
complexity_report_dir = (Env.REPORT_DIR / "complexity")
complexity_report = complexity_report_dir / "python_complexity.log"
......
......@@ -11,7 +11,8 @@ set -e
###############################################################################
# Violations thresholds for failing the build
export PYLINT_THRESHOLD=3600
export LOWER_PYLINT_THRESHOLD=1000
export UPPER_PYLINT_THRESHOLD=5335
export ESLINT_THRESHOLD=9134
export STYLELINT_THRESHOLD=973
......
......@@ -57,7 +57,7 @@ else
echo "Finding pylint violations and storing in report..."
# HACK: we need to print something to the console, otherwise circleci
# fails and aborts the job because nothing is displayed for > 10 minutes.
paver run_pylint -l $PYLINT_THRESHOLD | tee pylint.log || EXIT=1
paver run_pylint -l $LOWER_PYLINT_THRESHOLD:$UPPER_PYLINT_THRESHOLD | tee pylint.log || EXIT=1
mkdir -p reports
PATH=$PATH:node_modules/.bin
......
......@@ -84,7 +84,7 @@ case "$TEST_SUITE" in
echo "Finding pep8 violations and storing report..."
paver run_pep8 > pep8.log || { cat pep8.log; EXIT=1; }
echo "Finding pylint violations and storing in report..."
paver run_pylint -l $PYLINT_THRESHOLD > pylint.log || { echo 'Too many pylint violations. You can view them in pylint.log'; EXIT=1; }
paver run_pylint -l $LOWER_PYLINT_THRESHOLD:$UPPER_PYLINT_THRESHOLD > pylint.log || { echo 'Too many pylint violations. You can view them in pylint.log'; EXIT=1; }
mkdir -p reports
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment