quality.py 18.3 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
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/'
232 233 234 235 236 237 238 239
    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..."
240 241
    try:
        sh(
242 243 244
            "radon cc {system_string} --total-average > {complexity_report}".format(
                system_string=system_string,
                complexity_report=complexity_report
245 246
            )
        )
247 248 249 250 251 252 253 254
        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))

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


@task
260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283
@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
    )

284 285 286 287 288 289 290 291
    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
            )
        )
292 293

    # Record the metric
294
    _write_metric(num_violations, (Env.METRICS_DIR / "jshint"))
295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311

    # 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:
312
        metric_file.write(str(metric))
313 314 315 316 317 318 319 320 321 322 323 324 325 326


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.
    """
327 328 329 330 331 332 333 334
    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)
335 336


337
def _get_count_from_last_line(filename, file_type):
338
    """
339 340
    This will return the number in the last line of a file.
    It is returning only the value (as a floating number).
341 342
    """
    last_line = _get_last_report_line(filename)
343 344 345 346 347 348 349
    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+'

350
    try:
351 352 353 354
        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):
355 356 357 358
        return None


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

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

380
    def _pep8_output(count, violations_list, is_html=False):
381 382 383 384
        """
        Given a count & list of pep8 violations, pretty-print the pep8 output.
        If `is_html`, will print out with HTML markup.
        """
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 411 412 413 414
        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)

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

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

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

441
    # Generate diff-quality html report for pylint, and print to console
442 443 444
    # If pylint reports exist, use those
    # Otherwise, `diff-quality` will call pylint itself

445
    pylint_files = get_violations_reports("pylint")
446
    pylint_reports = u' '.join(pylint_files)
447 448
    jshint_files = get_violations_reports("jshint")
    jshint_reports = u' '.join(jshint_files)
449

450
    pythonpath_prefix = (
451 452 453 454
        "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:"
        "common:common/djangoapps:common/lib"
    )

455 456 457 458 459 460 461 462 463 464 465 466 467 468 469
    # 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,
470
            reports=jshint_reports,
471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489
            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.

    """
490 491
    try:
        sh(
492 493 494 495 496 497
            "{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,
498
                percentage_string=percentage_string,
499
                compare_branch_string=branch_string,
500
                dquality_dir=dquality_dir,
501
            )
502
        )
503
        return True
504 505
    except BuildFailure, error_message:
        if is_percentage_failure(error_message):
506
            return False
507 508 509 510 511 512 513 514 515 516
        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.
    """
517 518 519 520
    if "Subprocess return code: 1" not in error_message:
        return False
    else:
        return True
521 522 523 524 525 526 527 528 529 530 531 532


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