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

@@ -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'

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
 

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) {

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