quality.py 13 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
ALL_SYSTEMS = 'lms,cms,common'
11

12

13 14 15 16
@task
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
    ("system=", "s", "System to act on"),
Christine Lytwynec committed
17 18 19 20 21
])
def find_fixme(options):
    """
    Run pylint on system code, only looking for fixme items.
    """
22 23
    num_fixme = 0
    systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
Christine Lytwynec committed
24

25 26 27
    for system in systems:
        # Directory to put the pylint report in.
        # This makes the folder if it doesn't already exist.
Christine Lytwynec committed
28
        report_dir = (Env.REPORT_DIR / system).makedirs_p()
29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44

        apps = [system]

        for directory in ['djangoapps', 'lib']:
            dirs = os.listdir(os.path.join(system, directory))
            apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))])

        apps_list = ' '.join(apps)

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

Christine Lytwynec committed
45
        sh(
46 47
            "{pythonpath_prefix} pylint --disable R,C,W,E --enable=fixme "
            "--msg-template={msg_template} {apps} "
David Baumgold committed
48
            "| tee {report_dir}/pylint_fixme.report".format(
49
                pythonpath_prefix=pythonpath_prefix,
David Baumgold committed
50
                msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"',
51
                apps=apps_list,
Christine Lytwynec committed
52 53 54 55
                report_dir=report_dir
            )
        )

56 57 58
        num_fixme += _count_pylint_violations(
            "{report_dir}/pylint_fixme.report".format(report_dir=report_dir))

59
    print "Number of pylint fixmes: " + str(num_fixme)
Christine Lytwynec committed
60 61 62 63 64 65


@task
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
    ("system=", "s", "System to act on"),
66
    ("errors", "e", "Check for errors only"),
67
    ("limit=", "l", "limit for number of acceptable violations"),
68 69 70
])
def run_pylint(options):
    """
71 72
    Run pylint on system code. When violations limit is passed in,
    fail the task if too many violations are found.
73
    """
74
    num_violations = 0
75 76 77
    violations_limit = int(getattr(options, 'limit', -1))
    errors = getattr(options, 'errors', False)
    systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
78

79 80
    # Make sure the metrics subdirectory exists
    Env.METRICS_DIR.makedirs_p()
81

82 83 84
    for system in systems:
        # Directory to put the pylint report in.
        # This makes the folder if it doesn't already exist.
85
        report_dir = (Env.REPORT_DIR / system).makedirs_p()
86

87
        flags = []
88
        if errors:
89
            flags.append("--errors-only")
90

91 92 93 94 95 96 97 98 99 100 101 102 103 104 105
        apps = [system]

        for directory in ['lib']:
            dirs = os.listdir(os.path.join(system, directory))
            apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))])

        apps_list = ' '.join(apps)

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

106
        sh(
107
            "{pythonpath_prefix} pylint {flags} --msg-template={msg_template} {apps} | "
108
            "tee {report_dir}/pylint.report".format(
109 110
                pythonpath_prefix=pythonpath_prefix,
                flags=" ".join(flags),
David Baumgold committed
111
                msg_template='"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"',
112
                apps=apps_list,
113 114
                report_dir=report_dir
            )
115
        )
116

117 118 119
        num_violations += _count_pylint_violations(
            "{report_dir}/pylint.report".format(report_dir=report_dir))

120 121
    # Print number of violations to log
    violations_count_str = "Number of pylint violations: " + str(num_violations)
122
    print violations_count_str
123 124 125 126 127 128

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

133

134 135 136 137 138 139 140 141
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
142
    pylint_pattern = re.compile(r".(\d+):\ \[(\D\d+.+\]).")
143 144 145 146 147 148 149 150

    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
151

152

153
def _get_pep8_violations():
154
    """
155 156 157
    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.
158
    """
159 160 161
    report_dir = (Env.REPORT_DIR / 'pep8')
    report_dir.rmtree(ignore_errors=True)
    report_dir.makedirs_p()
162

163 164
    # Make sure the metrics subdirectory exists
    Env.METRICS_DIR.makedirs_p()
165

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

168
    count, violations_list = _pep8_violations(
169 170
        "{report_dir}/pep8.report".format(report_dir=report_dir)
    )
171

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

199 200
    # Print number of violations to log
    violations_count_str = "Number of pep8 violations: {count}".format(count=count)
201 202
    print violations_count_str
    print violations_list
203 204 205

    # Also write the number of violations to a file
    with open(Env.METRICS_DIR / "pep8", "w") as f:
206
        f.write(violations_count_str + '\n\n')
207
        f.write(violations_list)
208 209

    # Fail if any violations are found
210
    if count:
211 212 213
        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)
214

215 216 217

@task
@needs('pavelib.prereqs.install_python_prereqs')
218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236
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
@needs('pavelib.prereqs.install_python_prereqs')
237
@cmdopts([
238
    ("compare-branch=", "b", "Branch to compare against, defaults to origin/master"),
239 240 241
    ("percentage=", "p", "fail if diff-quality is below this percentage"),
])
def run_quality(options):
242 243
    """
    Build the html diff quality reports, and print the reports to the console.
244
    :param: b, the branch to compare against, defaults to origin/master
245 246
    :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
247
        quality of the branch vs the compare branch is less than 80%, then this task will fail.
248
        This threshold would be applied to both pep8 and pylint.
249 250 251
    """
    # Directory to put the diff reports in.
    # This makes the folder if it doesn't already exist.
252
    dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p()
253
    diff_quality_percentage_failure = False
254

255
    def _pep8_output(count, violations_list, is_html=False):
256 257 258 259
        """
        Given a count & list of pep8 violations, pretty-print the pep8 output.
        If `is_html`, will print out with HTML markup.
        """
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
        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)

290
    # Run pep8 directly since we have 0 violations on master
291 292 293 294 295 296 297 298 299 300 301 302 303
    (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 -----
304 305 306 307 308 309
    # 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)

310 311
    # Set the string, if needed, to be used for the diff-quality --fail-under switch.
    diff_threshold = int(getattr(options, 'percentage', -1))
312
    percentage_string = ''
313
    if diff_threshold > -1:
314
        percentage_string = '--fail-under={0}'.format(diff_threshold)
315

316
    # Generate diff-quality html report for pylint, and print to console
317 318 319
    # If pylint reports exist, use those
    # Otherwise, `diff-quality` will call pylint itself

320
    pylint_files = get_violations_reports("pylint")
321 322
    pylint_reports = u' '.join(pylint_files)

323
    pythonpath_prefix = (
324 325 326 327
        "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:"
        "common:common/djangoapps:common/lib"
    )

328 329
    try:
        sh(
330
            "{pythonpath_prefix} diff-quality --violations=pylint "
331
            "{pylint_reports} {percentage_string} {compare_branch_string} "
332
            "--html-report {dquality_dir}/diff_quality_pylint.html ".format(
333
                pythonpath_prefix=pythonpath_prefix,
334
                pylint_reports=pylint_reports,
335
                percentage_string=percentage_string,
336
                compare_branch_string=compare_branch_string,
337
                dquality_dir=dquality_dir,
338
            )
339
        )
340 341 342 343 344 345
    except BuildFailure, error_message:
        if is_percentage_failure(error_message):
            diff_quality_percentage_failure = True
        else:
            raise BuildFailure(error_message)

346
    # If one of the diff-quality runs fails, then paver exits with an error when it is finished
347 348 349 350 351 352 353 354 355 356
    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.
    """
357 358 359 360
    if "Subprocess return code: 1" not in error_message:
        return False
    else:
        return True
361 362 363 364 365 366 367 368 369 370 371 372


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