From 060d3b8e8535f342bfc877a2d11d53af5a8fee2e Mon Sep 17 00:00:00 2001 From: Julia Eskew Date: Tue, 19 Feb 2019 13:19:14 -0500 Subject: [PATCH] Accept option for PII checker report dir. Add PII check to CI quality check. Add tests for run_pii_check paver command. --- pavelib/paver_tests/test_pii_check.py | 34 ++++++++++++++++ pavelib/quality.py | 57 ++++++++++++++++++++++++--- scripts/Jenkinsfiles/quality | 2 +- scripts/generic-ci-tests.sh | 2 + 4 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 pavelib/paver_tests/test_pii_check.py diff --git a/pavelib/paver_tests/test_pii_check.py b/pavelib/paver_tests/test_pii_check.py new file mode 100644 index 0000000000..83b716fe23 --- /dev/null +++ b/pavelib/paver_tests/test_pii_check.py @@ -0,0 +1,34 @@ +""" +Tests for Paver's PII checker task. +""" +from io import open +import six +from mock import patch +from paver.easy import call_task +from pavelib.utils.envs import Env + +import pavelib.quality + + +@patch.object(pavelib.quality.run_pii_check, 'needs') +@patch('pavelib.quality.sh') +def test_pii_check_report_dir_override(mock_paver_sh, mock_needs, tmpdir): + """ + run_pii_check succeeds with proper report dir + """ + # Make the expected stdout files. + report_dir = tmpdir.mkdir('pii_report') + cms_stdout_report = report_dir.join('pii_check_cms.report') + cms_stdout_report.write('Coverage found 33 uncovered models:\n') + lms_stdout_report = report_dir.join('pii_check_lms.report') + lms_stdout_report.write('Coverage found 66 uncovered models:\n') + + mock_needs.return_value = 0 + call_task('pavelib.quality.run_pii_check', options={"report_dir": six.text_type(report_dir)}) + mock_calls = [six.text_type(call) for call in mock_paver_sh.mock_calls] + assert len(mock_calls) == 2 + assert any(['lms.envs.test' in call for call in mock_calls]) + assert any(['cms.envs.test' in call for call in mock_calls]) + assert all([six.text_type(report_dir) in call for call in mock_calls]) + metrics_file = Env.METRICS_DIR / 'pii' + assert open(metrics_file, 'r').read() == '66' diff --git a/pavelib/quality.py b/pavelib/quality.py index 650cbde7d4..3676a7d864 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -5,9 +5,11 @@ Check code quality using pycodestyle, pylint, and diff_quality. """ from __future__ import print_function +import io import json import os import re +import six from datetime import datetime from xml.sax.saxutils import quoteattr @@ -325,8 +327,7 @@ def run_complexity(): 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() + # Ensure an empty complexity report dir is in place. _prepare_report_dir(complexity_report_dir) print("--> Calculating cyclomatic complexity of python files...") @@ -635,7 +636,7 @@ def _write_metric(metric, filename): Env.METRICS_DIR.makedirs_p() with open(filename, "w") as metric_file: - metric_file.write(str(metric)) + metric_file.write(six.text_type(metric)) def _prepare_report_dir(dir_name): @@ -756,27 +757,71 @@ def _get_xsscommitlint_count(filename): return None +def _extract_missing_pii_annotations(filename): + """ + Returns the number of uncovered models from the stdout report of django_find_annotations. + + Arguments: + filename: Filename where stdout of django_find_annotations was captured. + + Returns: + Number of uncovered models. + """ + uncovered_models = None + if os.path.isfile(filename): + with io.open(filename, 'r') as report_file: + lines = report_file.readlines() + uncovered_regex = re.compile(r'^Coverage found ([\d]+) uncovered') + for line in lines: + uncovered_match = uncovered_regex.match(line) + if uncovered_match: + uncovered_models = int(uncovered_match.groups()[0]) + break + else: + fail_quality('pii', u'FAILURE: Log file could not be found: {}'.format(filename)) + + return uncovered_models + + @task @needs('pavelib.prereqs.install_python_prereqs') +@cmdopts([ + ("report-dir=", "r", "Directory in which to put PII reports"), +]) @timed def run_pii_check(options): # pylint: disable=unused-argument """ Guarantee that all Django models are PII-annotated. """ + pii_report_name = 'pii' + default_report_dir = (Env.REPORT_DIR / pii_report_name) + report_dir = getattr(options, 'report_dir', default_report_dir) + output_file = os.path.join(report_dir, 'pii_check_{}.report') + uncovered_model_counts = [] for env_name, env_settings_file in (("CMS", "cms.envs.test"), ("LMS", "lms.envs.test")): try: print() print("Running {} PII Annotation check and report".format(env_name)) print("-" * 45) + run_output_file = six.text_type(output_file).format(env_name.lower()) sh( + "mkdir -p {} && " "export DJANGO_SETTINGS_MODULE={}; " "code_annotations django_find_annotations " - "--config_file .pii_annotations.yml --report_path pii_report/ " - "--lint --report --coverage".format(env_settings_file) + "--config_file .pii_annotations.yml --report_path {} " + "--lint --report --coverage | tee {}".format( + report_dir, env_settings_file, report_dir, run_output_file + ) ) + uncovered_model_counts.append(_extract_missing_pii_annotations(run_output_file)) except BuildFailure as error_message: - fail_quality('pii_check', 'FAILURE: {}'.format(error_message)) + fail_quality(pii_report_name, 'FAILURE: {}'.format(error_message)) + + uncovered_count = max(uncovered_model_counts) + if uncovered_count is None: + uncovered_count = 0 + _write_metric(uncovered_count, (Env.METRICS_DIR / pii_report_name)) return True diff --git a/scripts/Jenkinsfiles/quality b/scripts/Jenkinsfiles/quality index 146d6f532b..dd6dd813f2 100644 --- a/scripts/Jenkinsfiles/quality +++ b/scripts/Jenkinsfiles/quality @@ -198,7 +198,7 @@ pipeline { // Publish Quality report publishHTML([allowMissing: true, alwaysLinkToLastBuild: false, keepAll: true, reportDir: 'reports/metrics/', - reportFiles: 'pylint/*view*/,pep8/*view*/,python_complexity/*view*/,xsscommitlint/*view*/,xsslint/*view*/,eslint/*view*/', + reportFiles: 'pylint/*view*/,pep8/*view*/,python_complexity/*view*/,xsscommitlint/*view*/,xsslint/*view*/,eslint/*view*/,pii/*view*/', reportName: 'Quality Report', reportTitles: '']) } finally { if (env.ghprbPullId != null) { diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index fecdb520bd..5077b33887 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -137,6 +137,8 @@ case "$TEST_SUITE" in run_paver_quality run_xsslint -t $XSSLINT_THRESHOLDS || { EXIT=1; } echo "Running safe commit linter report." run_paver_quality run_xsscommitlint || { EXIT=1; } + echo "Running PII checker on all Django models..." + run_paver_quality run_pii_check || { EXIT=1; } ;; esac