quality.py 15.6 KB
Newer Older
1 2 3
"""
Check code quality using pep8, pylint, and diff_quality.
"""
4
from paver.easy import sh, task, cmdopts, needs, BuildFailure
5
import os
6 7
import re

8 9
from .utils.envs import Env

10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30
ALL_SYSTEMS = 'lms,cms,common,openedx,pavelib'


def top_python_dirs(dirname):
    """
    Find the directories to start from in order to find all the Python files in `dirname`.
    """
    top_dirs = []

    dir_init = os.path.join(dirname, "__init__.py")
    if os.path.exists(dir_init):
        top_dirs.append(dirname)

    for directory in ['djangoapps', 'lib']:
        subdir = os.path.join(dirname, directory)
        subdir_init = os.path.join(subdir, "__init__.py")
        if os.path.exists(subdir) and not os.path.exists(subdir_init):
            dirs = os.listdir(subdir)
            top_dirs.extend(d for d in dirs if os.path.isdir(os.path.join(subdir, d)))

    return top_dirs
31

32

33 34 35 36
@task
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
    ("system=", "s", "System to act on"),
Christine Lytwynec committed
37 38 39 40 41
])
def find_fixme(options):
    """
    Run pylint on system code, only looking for fixme items.
    """
42 43
    num_fixme = 0
    systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
Christine Lytwynec committed
44

45 46 47
    for system in systems:
        # Directory to put the pylint report in.
        # This makes the folder if it doesn't already exist.
Christine Lytwynec committed
48
        report_dir = (Env.REPORT_DIR / system).makedirs_p()
49

50
        apps_list = ' '.join(top_python_dirs(system))
51 52 53 54 55 56 57 58

        pythonpath_prefix = (
            "PYTHONPATH={system}:{system}/lib"
            "common/djangoapps:common/lib".format(
                system=system
            )
        )

Christine Lytwynec committed
59
        sh(
60 61
            "{pythonpath_prefix} pylint --disable R,C,W,E --enable=fixme "
            "--msg-template={msg_template} {apps} "
David Baumgold committed
62
            "| tee {report_dir}/pylint_fixme.report".format(
63
                pythonpath_prefix=pythonpath_prefix,
David Baumgold committed
64
                msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"',
65
                apps=apps_list,
Christine Lytwynec committed
66 67 68 69
                report_dir=report_dir
            )
        )

70 71 72
        num_fixme += _count_pylint_violations(
            "{report_dir}/pylint_fixme.report".format(report_dir=report_dir))

73
    print "Number of pylint fixmes: " + str(num_fixme)
Christine Lytwynec committed
74 75 76 77 78 79


@task
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
    ("system=", "s", "System to act on"),
80
    ("errors", "e", "Check for errors only"),
81
    ("limit=", "l", "limit for number of acceptable violations"),
82 83 84
])
def run_pylint(options):
    """
85 86
    Run pylint on system code. When violations limit is passed in,
    fail the task if too many violations are found.
87
    """
88
    num_violations = 0
89 90 91
    violations_limit = int(getattr(options, 'limit', -1))
    errors = getattr(options, 'errors', False)
    systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
92

93 94
    # Make sure the metrics subdirectory exists
    Env.METRICS_DIR.makedirs_p()
95

96 97 98
    for system in systems:
        # Directory to put the pylint report in.
        # This makes the folder if it doesn't already exist.
99
        report_dir = (Env.REPORT_DIR / system).makedirs_p()
100

101
        flags = []
102
        if errors:
103
            flags.append("--errors-only")
104

105
        apps_list = ' '.join(top_python_dirs(system))
106 107 108 109 110 111 112 113

        pythonpath_prefix = (
            "PYTHONPATH={system}:{system}/djangoapps:{system}/"
            "lib:common/djangoapps:common/lib".format(
                system=system
            )
        )

114
        sh(
115
            "{pythonpath_prefix} pylint {flags} --msg-template={msg_template} {apps} | "
116
            "tee {report_dir}/pylint.report".format(
117 118
                pythonpath_prefix=pythonpath_prefix,
                flags=" ".join(flags),
David Baumgold committed
119
                msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"',
120
                apps=apps_list,
121 122
                report_dir=report_dir
            )
123
        )
124

125 126 127
        num_violations += _count_pylint_violations(
            "{report_dir}/pylint.report".format(report_dir=report_dir))

128 129
    # Print number of violations to log
    violations_count_str = "Number of pylint violations: " + str(num_violations)
130
    print violations_count_str
131 132 133 134 135 136

    # Also write the number of violations to a file
    with open(Env.METRICS_DIR / "pylint", "w") as f:
        f.write(violations_count_str)

    # Fail number of violations is greater than the limit
137
    if num_violations > violations_limit > -1:
138
        raise Exception("Failed. Too many pylint violations. "
139
                        "The limit is {violations_limit}.".format(violations_limit=violations_limit))
140

141

142 143 144 145 146 147 148 149
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:
    # common/lib/xmodule/xmodule/tests/test_conditional.py:21: [C0111(missing-docstring), DummySystem] Missing docstring
    # More examples can be found in the unit tests for this method
150
    pylint_pattern = re.compile(r".(\d+):\ \[(\D\d+.+\]).")
151 152 153 154 155 156 157 158

    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
159

160

161
def _get_pep8_violations():
162
    """
163 164 165
    Runs pep8. Returns a tuple of (number_of_violations, violations_string)
    where violations_string is a string of all pep8 violations found, separated
    by new lines.
166
    """
167 168 169
    report_dir = (Env.REPORT_DIR / 'pep8')
    report_dir.rmtree(ignore_errors=True)
    report_dir.makedirs_p()
170

171 172
    # Make sure the metrics subdirectory exists
    Env.METRICS_DIR.makedirs_p()
173

174
    sh('pep8 . | tee {report_dir}/pep8.report -a'.format(report_dir=report_dir))
175

176
    count, violations_list = _pep8_violations(
177 178
        "{report_dir}/pep8.report".format(report_dir=report_dir)
    )
179

180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198
    return (count, violations_list)


def _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
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
    ("system=", "s", "System to act on"),
])
199
def run_pep8(options):  # pylint: disable=unused-argument
200 201 202 203 204
    """
    Run pep8 on system code.
    Fail the task if any violations are found.
    """
    (count, violations_list) = _get_pep8_violations()
205
    violations_list = ''.join(violations_list)
206

207 208
    # Print number of violations to log
    violations_count_str = "Number of pep8 violations: {count}".format(count=count)
209 210
    print violations_count_str
    print violations_list
211 212 213

    # Also write the number of violations to a file
    with open(Env.METRICS_DIR / "pep8", "w") as f:
214
        f.write(violations_count_str + '\n\n')
215
        f.write(violations_list)
216 217

    # Fail if any violations are found
218
    if count:
219 220 221
        failure_string = "Too many pep8 violations. " + violations_count_str
        failure_string += "\n\nViolations:\n{violations_list}".format(violations_list=violations_list)
        raise Exception(failure_string)
222

223 224 225

@task
@needs('pavelib.prereqs.install_python_prereqs')
226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243
def run_complexity():
    """
    Uses radon to examine cyclomatic complexity.
    For additional details on radon, see http://radon.readthedocs.org/
    """
    system_string = 'cms/ lms/ common/ openedx/'
    print "--> Calculating cyclomatic complexity of files..."
    try:
        sh(
            "radon cc {system_string} --total-average".format(
                system_string=system_string
            )
        )
    except BuildFailure:
        print "ERROR: Unable to calculate python-only code-complexity."


@task
244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323
@needs('pavelib.prereqs.install_node_prereqs')
@cmdopts([
    ("limit=", "l", "limit for number of acceptable violations"),
])
def run_jshint(options):
    """
    Runs jshint on static asset directories
    """

    violations_limit = int(getattr(options, 'limit', -1))

    jshint_report_dir = (Env.REPORT_DIR / "jshint")
    jshint_report = jshint_report_dir / "jshint.report"
    _prepare_report_dir(jshint_report_dir)

    jshint_directories = ["common/static/js", "cms/static/js", "lms/static/js"]

    sh(
        "jshint {list} --config .jshintrc >> {jshint_report}".format(
            list=(" ".join(jshint_directories)), jshint_report=jshint_report
        ),
        ignore_error=True
    )
    num_violations = _get_count_from_last_line(jshint_report)

    if not num_violations:
        raise BuildFailure("Error in calculating total number of violations.")

    # Record the metric
    _write_metric(str(num_violations), (Env.METRICS_DIR / "jshint"))

    # Fail if number of violations is greater than the limit
    if num_violations > violations_limit > -1:
        raise Exception(
            "JSHint Failed. Too many violations ({count}).\nThe limit is {violations_limit}.".format(
                count=num_violations, violations_limit=violations_limit
            )
        )


def _write_metric(metric, filename):
    """
    Write a given metric to a given file
    Used for things like reports/metrics/jshint, which will simply tell you the number of
    jshint violations found
    """
    with open(filename, "w") as metric_file:
        metric_file.write(metric)


def _prepare_report_dir(dir_name):
    """
    Sets a given directory to a created, but empty state
    """
    dir_name.rmtree_p()
    dir_name.mkdir_p()


def _get_last_report_line(filename):
    """
    Returns the last line of a given file. Used for getting output from quality output files.
    """
    with open(filename, 'r') as report_file:
        lines = report_file.readlines()
        return lines[len(lines) - 1]


def _get_count_from_last_line(filename):
    """
    This will return the number in a line that looks something like "3000 errors found". It is returning
    the digits only (as an integer).
    """
    last_line = _get_last_report_line(filename)
    try:
        return int(re.search(r'^\d+', last_line).group(0))
    except AttributeError:
        return None


@task
324
@needs('pavelib.prereqs.install_python_prereqs')
325
@cmdopts([
326
    ("compare-branch=", "b", "Branch to compare against, defaults to origin/master"),
327 328 329
    ("percentage=", "p", "fail if diff-quality is below this percentage"),
])
def run_quality(options):
330 331
    """
    Build the html diff quality reports, and print the reports to the console.
332
    :param: b, the branch to compare against, defaults to origin/master
333 334
    :param: p, diff-quality will fail if the quality percentage calculated is
        below this percentage. For example, if p is set to 80, and diff-quality finds
335
        quality of the branch vs the compare branch is less than 80%, then this task will fail.
336
        This threshold would be applied to both pep8 and pylint.
337 338 339
    """
    # Directory to put the diff reports in.
    # This makes the folder if it doesn't already exist.
340
    dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p()
341
    diff_quality_percentage_failure = False
342

343
    def _pep8_output(count, violations_list, is_html=False):
344 345 346 347
        """
        Given a count & list of pep8 violations, pretty-print the pep8 output.
        If `is_html`, will print out with HTML markup.
        """
348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377
        if is_html:
            lines = ['<body>\n']
            sep = '-------------<br/>\n'
            title = "<h1>Quality Report: pep8</h1>\n"
            violations_bullets = ''.join(
                ['<li>{violation}</li><br/>\n'.format(violation=violation) for violation in violations_list]
            )
            violations_str = '<ul>\n{bullets}</ul>\n'.format(bullets=violations_bullets)
            violations_count_str = "<b>Violations</b>: {count}<br/>\n"
            fail_line = "<b>FAILURE</b>: pep8 count should be 0<br/>\n"
        else:
            lines = []
            sep = '-------------\n'
            title = "Quality Report: pep8\n"
            violations_str = ''.join(violations_list)
            violations_count_str = "Violations: {count}\n"
            fail_line = "FAILURE: pep8 count should be 0\n"

        violations_count_str = violations_count_str.format(count=count)

        lines.extend([sep, title, sep, violations_str, sep, violations_count_str])

        if count > 0:
            lines.append(fail_line)
        lines.append(sep + '\n')
        if is_html:
            lines.append('</body>')

        return ''.join(lines)

378
    # Run pep8 directly since we have 0 violations on master
379 380 381 382 383 384 385 386 387 388 389 390 391
    (count, violations_list) = _get_pep8_violations()

    # Print number of violations to log
    print _pep8_output(count, violations_list)

    # Also write the number of violations to a file
    with open(dquality_dir / "diff_quality_pep8.html", "w") as f:
        f.write(_pep8_output(count, violations_list, is_html=True))

    if count > 0:
        diff_quality_percentage_failure = True

    # ----- Set up for diff-quality pylint call -----
392 393 394 395 396 397
    # Set the string, if needed, to be used for the diff-quality --compare-branch switch.
    compare_branch = getattr(options, 'compare_branch', None)
    compare_branch_string = ''
    if compare_branch:
        compare_branch_string = '--compare-branch={0}'.format(compare_branch)

398 399
    # Set the string, if needed, to be used for the diff-quality --fail-under switch.
    diff_threshold = int(getattr(options, 'percentage', -1))
400
    percentage_string = ''
401
    if diff_threshold > -1:
402
        percentage_string = '--fail-under={0}'.format(diff_threshold)
403

404
    # Generate diff-quality html report for pylint, and print to console
405 406 407
    # If pylint reports exist, use those
    # Otherwise, `diff-quality` will call pylint itself

408
    pylint_files = get_violations_reports("pylint")
409 410
    pylint_reports = u' '.join(pylint_files)

411
    pythonpath_prefix = (
412 413 414 415
        "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:"
        "common:common/djangoapps:common/lib"
    )

416 417
    try:
        sh(
418
            "{pythonpath_prefix} diff-quality --violations=pylint "
419
            "{pylint_reports} {percentage_string} {compare_branch_string} "
420
            "--html-report {dquality_dir}/diff_quality_pylint.html ".format(
421
                pythonpath_prefix=pythonpath_prefix,
422
                pylint_reports=pylint_reports,
423
                percentage_string=percentage_string,
424
                compare_branch_string=compare_branch_string,
425
                dquality_dir=dquality_dir,
426
            )
427
        )
428 429 430 431 432 433
    except BuildFailure, error_message:
        if is_percentage_failure(error_message):
            diff_quality_percentage_failure = True
        else:
            raise BuildFailure(error_message)

434
    # If one of the diff-quality runs fails, then paver exits with an error when it is finished
435 436 437 438 439 440 441 442 443 444
    if diff_quality_percentage_failure:
        raise BuildFailure("Diff-quality failure(s).")


def is_percentage_failure(error_message):
    """
    When diff-quality is run with a threshold percentage, it ends with an exit code of 1. This bubbles up to
    paver with a subprocess return code error. If the subprocess exits with anything other than 1, raise
    a paver exception.
    """
445 446 447 448
    if "Subprocess return code: 1" not in error_message:
        return False
    else:
        return True
449 450 451 452 453 454 455 456 457 458 459 460


def get_violations_reports(violations_type):
    """
    Finds violations reports files by naming convention (e.g., all "pep8.report" files)
    """
    violations_files = []
    for subdir, _dirs, files in os.walk(os.path.join(Env.REPORT_DIR)):
        for f in files:
            if f == "{violations_type}.report".format(violations_type=violations_type):
                violations_files.append(os.path.join(subdir, f))
    return violations_files