Skip to content

E
edx-platform
Commit a51fbc81 by Sarina Canelake
Options

Refactor pep8 to run instead of diff-quality pep8

parent 1b65afdd
Showing with 79 additions and 43 deletions
pavelib/quality.py
...@@ -150,17 +150,13 @@ def _count_pylint_violations(report_file): ...@@ -150,17 +150,13 @@ def _count_pylint_violations(report_file):
return num_violations_report return num_violations_report
@task def _get_pep8_violations():
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
("system=", "s", "System to act on"),
])
def run_pep8(options):
""" """
Run pep8 on system code. Runs pep8. Returns a tuple of (number_of_violations, violations_string)
Fail the task if any violations are found. 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 = (Env.REPORT_DIR / 'pep8')
report_dir.rmtree(ignore_errors=True) report_dir.rmtree(ignore_errors=True)
...@@ -172,17 +168,40 @@ def run_pep8(options): ...@@ -172,17 +168,40 @@ def run_pep8(options):
for system in systems: for system in systems:
sh('pep8 {system} | tee {report_dir}/pep8.report -a'.format(system=system, report_dir=report_dir)) 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) "{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 # Print number of violations to log
violations_count_str = "Number of pep8 violations: {count}".format(count=count) violations_count_str = "Number of pep8 violations: {count}".format(count=count)
print(violations_count_str) 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) print(violations_str)
# Also write the number of violations to a file # Also write the number of violations to a file
...@@ -199,11 +218,6 @@ def run_pep8(options): ...@@ -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 @task
@needs('pavelib.prereqs.install_python_prereqs') @needs('pavelib.prereqs.install_python_prereqs')
def run_complexity(): def run_complexity():
...@@ -238,12 +252,57 @@ def run_quality(options): ...@@ -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. 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. This threshold would be applied to both pep8 and pylint.
""" """
# Directory to put the diff reports in. # Directory to put the diff reports in.
# This makes the folder if it doesn't already exist. # This makes the folder if it doesn't already exist.
dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p() dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p()
diff_quality_percentage_failure = False 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 = ['<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)
(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. # Set the string, if needed, to be used for the diff-quality --compare-branch switch.
compare_branch = getattr(options, 'compare_branch', None) compare_branch = getattr(options, 'compare_branch', None)
compare_branch_string = '' compare_branch_string = ''
...@@ -256,29 +315,6 @@ def run_quality(options): ...@@ -256,29 +315,6 @@ def run_quality(options):
if diff_threshold > -1: if diff_threshold > -1:
percentage_string = '--fail-under={0}'.format(diff_threshold) 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 # Generate diff-quality html report for pylint, and print to console
# If pylint reports exist, use those # If pylint reports exist, use those
# Otherwise, `diff-quality` will call pylint itself # Otherwise, `diff-quality` will call pylint itself
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment