diff --git a/pavelib/quality.py b/pavelib/quality.py index b08fe067b2..5b9b6efb96 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -150,17 +150,13 @@ def _count_pylint_violations(report_file): return num_violations_report -@task -@needs('pavelib.prereqs.install_python_prereqs') -@cmdopts([ - ("system=", "s", "System to act on"), -]) -def run_pep8(options): +def _get_pep8_violations(): """ - Run pep8 on system code. - Fail the task if any violations are found. + 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. """ - systems = getattr(options, 'system', ALL_SYSTEMS).split(',') + systems = ALL_SYSTEMS.split(',') report_dir = (Env.REPORT_DIR / 'pep8') report_dir.rmtree(ignore_errors=True) @@ -172,17 +168,40 @@ def run_pep8(options): for system in systems: sh('pep8 {system} | tee {report_dir}/pep8.report -a'.format(system=system, report_dir=report_dir)) - count = _count_pep8_violations( + count, violations_list = _pep8_violations( "{report_dir}/pep8.report".format(report_dir=report_dir) ) + 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"), +]) +def run_pep8(options): + """ + Run pep8 on system code. + Fail the task if any violations are found. + """ + (count, violations_list) = _get_pep8_violations() + violations_str = ''.join(violations_list) + # Print number of violations to log violations_count_str = "Number of pep8 violations: {count}".format(count=count) print(violations_count_str) - violations = '' - with open("{report_dir}/pep8.report".format(report_dir=report_dir), 'r') as f: - violations_list = f.readlines() - violations_str = '\n'.join(violations_list) print(violations_str) # Also write the number of violations to a file @@ -199,11 +218,6 @@ def run_pep8(options): ) -def _count_pep8_violations(report_file): - num_lines = sum(1 for line in open(report_file)) - return num_lines - - @task @needs('pavelib.prereqs.install_python_prereqs') def run_complexity(): @@ -238,12 +252,57 @@ def run_quality(options): quality of the branch vs the compare branch is less than 80%, then this task will fail. This threshold would be applied to both pep8 and pylint. """ - # Directory to put the diff reports in. # This makes the folder if it doesn't already exist. dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p() diff_quality_percentage_failure = False + # Run pep8 directly since we have 0 violations on master + def _pep8_output(count, violations_list, is_html=False): + + if is_html: + lines = ['\n'] + sep = '-------------
\n' + title = "

Quality Report: pep8

\n" + violations_bullets = ''.join( + ['
  • {violation}

  • \n'.format(violation=violation) for violation in violations_list] + ) + violations_str = '\n'.format(bullets=violations_bullets) + violations_count_str = "Violations: {count}
    \n" + fail_line = "FAILURE: pep8 count should be 0
    \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('') + + return ''.join(lines) + + (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 ----- # 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 = '' @@ -256,29 +315,6 @@ def run_quality(options): if diff_threshold > -1: percentage_string = '--fail-under={0}'.format(diff_threshold) - # Generate diff-quality html report for pep8, and print to console - # If pep8 reports exist, use those - # Otherwise, `diff-quality` will call pep8 itself - - pep8_files = get_violations_reports("pep8") - pep8_reports = u' '.join(pep8_files) - - try: - sh( - "diff-quality --violations=pep8 {pep8_reports} {percentage_string} " - "{compare_branch_string} --html-report {dquality_dir}/diff_quality_pep8.html".format( - pep8_reports=pep8_reports, - percentage_string=percentage_string, - compare_branch_string=compare_branch_string, - dquality_dir=dquality_dir - ) - ) - except BuildFailure, error_message: - if is_percentage_failure(error_message): - diff_quality_percentage_failure = True - else: - raise BuildFailure(error_message) - # Generate diff-quality html report for pylint, and print to console # If pylint reports exist, use those # Otherwise, `diff-quality` will call pylint itself