quality.py 18.2 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

        pythonpath_prefix = (
53
            "PYTHONPATH={system}/djangoapps:common/djangoapps:common/lib".format(
54 55 56 57
                system=system
            )
        )

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

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

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


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

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

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

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

104
        apps_list = ' '.join(top_python_dirs(system))
105 106

        pythonpath_prefix = (
107
            "PYTHONPATH={system}/djangoapps:common/djangoapps:common/lib".format(
108 109 110 111
                system=system
            )
        )

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

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

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

    # 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
135
    if num_violations > violations_limit > -1:
136
        raise Exception("Failed. Too many pylint violations. "
137
                        "The limit is {violations_limit}.".format(violations_limit=violations_limit))
138

139

140 141 142 143 144 145 146 147
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
148
    pylint_pattern = re.compile(r".(\d+):\ \[(\D\d+.+\]).")
149 150 151 152 153 154 155 156

    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
157

158

159
def _get_pep8_violations():
160
    """
161 162 163
    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.
164
    """
165 166 167
    report_dir = (Env.REPORT_DIR / 'pep8')
    report_dir.rmtree(ignore_errors=True)
    report_dir.makedirs_p()
168

169 170
    # Make sure the metrics subdirectory exists
    Env.METRICS_DIR.makedirs_p()
171

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

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

178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196
    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"),
])
197
def run_pep8(options):  # pylint: disable=unused-argument
198 199 200 201 202
    """
    Run pep8 on system code.
    Fail the task if any violations are found.
    """
    (count, violations_list) = _get_pep8_violations()
203
    violations_list = ''.join(violations_list)
204

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

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

    # Fail if any violations are found
216
    if count:
217 218 219
        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)
220

221 222 223

@task
@needs('pavelib.prereqs.install_python_prereqs')
224 225 226 227 228 229
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/'
230 231 232 233 234 235 236 237
    complexity_report_dir = (Env.REPORT_DIR / "complexity")
    complexity_report = complexity_report_dir / "python_complexity.log"

    # Ensure directory structure is in place: metrics dir, and an empty complexity report dir.
    Env.METRICS_DIR.makedirs_p()
    _prepare_report_dir(complexity_report_dir)

    print "--> Calculating cyclomatic complexity of python files..."
238 239
    try:
        sh(
240 241 242
            "radon cc {system_string} --total-average > {complexity_report}".format(
                system_string=system_string,
                complexity_report=complexity_report
243 244
            )
        )
245 246 247 248 249 250 251 252
        complexity_metric = _get_count_from_last_line(complexity_report, "python_complexity")
        _write_metric(
            complexity_metric,
            (Env.METRICS_DIR / "python_complexity")
        )
        print "--> Python cyclomatic complexity report complete."
        print "radon cyclomatic complexity score: {metric}".format(metric=str(complexity_metric))

253 254 255 256 257
    except BuildFailure:
        print "ERROR: Unable to calculate python-only code-complexity."


@task
258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273
@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)

    sh(
274 275
        "jshint . --config .jshintrc >> {jshint_report}".format(
            jshint_report=jshint_report
276 277 278 279
        ),
        ignore_error=True
    )

280 281 282 283 284 285 286 287
    try:
        num_violations = int(_get_count_from_last_line(jshint_report, "jshint"))
    except TypeError:
        raise BuildFailure(
            "Error. Number of jshint violations could not be found in {jshint_report}".format(
                jshint_report=jshint_report
            )
        )
288 289

    # Record the metric
290
    _write_metric(num_violations, (Env.METRICS_DIR / "jshint"))
291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307

    # 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:
308
        metric_file.write(str(metric))
309 310 311 312 313 314 315 316 317 318 319 320 321 322


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.
    """
323 324 325 326 327 328 329 330
    file_not_found_message = "The following log file could not be found: {file}".format(file=filename)
    if os.path.isfile(filename):
        with open(filename, 'r') as report_file:
            lines = report_file.readlines()
            return lines[len(lines) - 1]
    else:
        # Raise a build error if the file is not found
        raise BuildFailure(file_not_found_message)
331 332


333
def _get_count_from_last_line(filename, file_type):
334
    """
335 336
    This will return the number in the last line of a file.
    It is returning only the value (as a floating number).
337 338
    """
    last_line = _get_last_report_line(filename)
339 340 341 342 343 344 345
    if file_type is "python_complexity":
        # Example of the last line of a complexity report: "Average complexity: A (1.93953443446)"
        regex = r'\d+.\d+'
    else:
        # Example of the last line of a jshint report (for example): "3482 errors"
        regex = r'^\d+'

346
    try:
347 348 349 350
        return float(re.search(regex, last_line).group(0))
    # An AttributeError will occur if the regex finds no matches.
    # A ValueError will occur if the returned regex cannot be cast as a float.
    except (AttributeError, ValueError):
351 352 353 354
        return None


@task
355
@needs('pavelib.prereqs.install_python_prereqs')
356
@cmdopts([
357
    ("compare-branch=", "b", "Branch to compare against, defaults to origin/master"),
358 359 360
    ("percentage=", "p", "fail if diff-quality is below this percentage"),
])
def run_quality(options):
361 362
    """
    Build the html diff quality reports, and print the reports to the console.
363
    :param: b, the branch to compare against, defaults to origin/master
364 365
    :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
366
        quality of the branch vs the compare branch is less than 80%, then this task will fail.
367
        This threshold would be applied to both pep8 and pylint.
368 369 370
    """
    # Directory to put the diff reports in.
    # This makes the folder if it doesn't already exist.
371
    dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p()
372 373 374

    # Save the pass variable. It will be set to false later if failures are detected.
    diff_quality_percentage_pass = True
375

376
    def _pep8_output(count, violations_list, is_html=False):
377 378 379 380
        """
        Given a count & list of pep8 violations, pretty-print the pep8 output.
        If `is_html`, will print out with HTML markup.
        """
381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410
        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)

411
    # Run pep8 directly since we have 0 violations on master
412 413 414 415 416 417 418 419 420 421
    (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:
422
        diff_quality_percentage_pass = False
423 424

    # ----- Set up for diff-quality pylint call -----
425 426
    # Set the string, if needed, to be used for the diff-quality --compare-branch switch.
    compare_branch = getattr(options, 'compare_branch', None)
427
    compare_branch_string = u''
428
    if compare_branch:
429
        compare_branch_string = u'--compare-branch={0}'.format(compare_branch)
430

431 432
    # Set the string, if needed, to be used for the diff-quality --fail-under switch.
    diff_threshold = int(getattr(options, 'percentage', -1))
433
    percentage_string = u''
434
    if diff_threshold > -1:
435
        percentage_string = u'--fail-under={0}'.format(diff_threshold)
436

437
    # Generate diff-quality html report for pylint, and print to console
438 439 440
    # If pylint reports exist, use those
    # Otherwise, `diff-quality` will call pylint itself

441
    pylint_files = get_violations_reports("pylint")
442
    pylint_reports = u' '.join(pylint_files)
443 444
    jshint_files = get_violations_reports("jshint")
    jshint_reports = u' '.join(jshint_files)
445

446
    pythonpath_prefix = (
447
        "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:cms:cms/djangoapps:"
448 449 450
        "common:common/djangoapps:common/lib"
    )

451 452 453 454 455 456 457 458 459 460 461 462 463 464 465
    # run diff-quality for pylint.
    if not run_diff_quality(
            violations_type="pylint",
            prefix=pythonpath_prefix,
            reports=pylint_reports,
            percentage_string=percentage_string,
            branch_string=compare_branch_string,
            dquality_dir=dquality_dir
    ):
        diff_quality_percentage_pass = False

    # run diff-quality for jshint.
    if not run_diff_quality(
            violations_type="jshint",
            prefix=pythonpath_prefix,
466
            reports=jshint_reports,
467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485
            percentage_string=percentage_string,
            branch_string=compare_branch_string,
            dquality_dir=dquality_dir
    ):
        diff_quality_percentage_pass = False

    # If one of the quality runs fails, then paver exits with an error when it is finished
    if not diff_quality_percentage_pass:
        raise BuildFailure("Diff-quality failure(s).")


def run_diff_quality(
        violations_type=None, prefix=None, reports=None, percentage_string=None, branch_string=None, dquality_dir=None
):
    """
    This executes the diff-quality commandline tool for the given violation type (e.g., pylint, jshint).
    If diff-quality fails due to quality issues, this method returns False.

    """
486 487
    try:
        sh(
488 489 490 491 492 493
            "{pythonpath_prefix} diff-quality --violations={type} "
            "{reports} {percentage_string} {compare_branch_string} "
            "--html-report {dquality_dir}/diff_quality_{type}.html ".format(
                type=violations_type,
                pythonpath_prefix=prefix,
                reports=reports,
494
                percentage_string=percentage_string,
495
                compare_branch_string=branch_string,
496
                dquality_dir=dquality_dir,
497
            )
498
        )
499
        return True
500 501
    except BuildFailure, error_message:
        if is_percentage_failure(error_message):
502
            return False
503 504 505 506 507 508 509 510 511 512
        else:
            raise BuildFailure(error_message)


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.
    """
513 514 515 516
    if "Subprocess return code: 1" not in error_message:
        return False
    else:
        return True
517 518 519 520 521 522 523 524 525 526 527 528


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