Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: remove xss-commitlint test (#30025)

Browse Source
This commit is contained in:
Rebecca Graber
2022-03-08 14:18:45 -05:00
committed by GitHub
parent 0940fc5e49
commit eba76109c4
5 changed files with 1 additions and 275 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
pavelib/paver_tests/test_paver_quality.py
View File

@@ -67,7 +67,7 @@ class TestPaverQualityViolations(unittest.TestCase):
class TestPaverReportViolationsCounts(unittest.TestCase):
"""
For testing utility functions for getting counts from reports for
run_eslint, run_xsslint, and run_xsscommitlint.
run_eslint and run_xsslint.
"""
def setUp(self):
@@ -158,52 +158,6 @@ class TestPaverReportViolationsCounts(unittest.TestCase):
'total': None,
})
def test_get_xsscommitlint_count_happy(self):
"""
Test happy path getting violation count from xsscommitlint report.
"""
report = textwrap.dedent("""
Linting lms/templates/navigation.html:
2 violations total
Linting scripts/tests/templates/test.underscore:
3 violations total
""")
with open(self.f.name, 'w') as f:
f.write(report)
count = pavelib.quality._get_xsscommitlint_count(self.f.name) # pylint: disable=protected-access
assert count == 5
def test_get_xsscommitlint_count_bad_counts(self):
"""
Test getting violation count from truncated xsscommitlint report.
"""
report = textwrap.dedent("""
Linting lms/templates/navigation.html:
""")
with open(self.f.name, 'w') as f:
f.write(report)
count = pavelib.quality._get_xsscommitlint_count(self.f.name) # pylint: disable=protected-access
assert count is None
def test_get_xsscommitlint_count_no_files(self):
"""
Test getting violation count from xsscommitlint report where no files were
linted.
"""
report = textwrap.dedent("""
No files linted.
""")
with open(self.f.name, 'w') as f:
f.write(report)
count = pavelib.quality._get_xsscommitlint_count(self.f.name) # pylint: disable=protected-access
assert count == 0
class TestPrepareReportDir(unittest.TestCase):
"""

46
pavelib/paver_tests/test_xsscommitlint.py
View File

@@ -1,46 +0,0 @@
"""
Tests for paver xsscommitlint quality tasks
"""
from unittest.mock import patch
import pytest
from paver.easy import call_task
import pavelib.quality
from .utils import PaverTestCase
class PaverXSSCommitLintTest(PaverTestCase):
"""
Test run_xsscommitlint with a mocked environment in order to pass in
opts.
"""
def setUp(self):
super().setUp()
self.reset_task_messages()
@patch.object(pavelib.quality, '_write_metric')
@patch.object(pavelib.quality, '_prepare_report_dir')
@patch.object(pavelib.quality, '_get_xsscommitlint_count')
def test_xsscommitlint_violation_number_not_found(self, _mock_count, _mock_report_dir, _mock_write_metric):
"""
run_xsscommitlint encounters an error parsing the xsscommitlint output
log.
"""
_mock_count.return_value = None
with pytest.raises(SystemExit):
call_task('pavelib.quality.run_xsscommitlint')
@patch.object(pavelib.quality, '_write_metric')
@patch.object(pavelib.quality, '_prepare_report_dir')
@patch.object(pavelib.quality, '_get_xsscommitlint_count')
def test_xsscommitlint_vanilla(self, _mock_count, _mock_report_dir, _mock_write_metric):
"""
run_xsscommitlint finds violations.
"""
_mock_count.return_value = 0
call_task('pavelib.quality.run_xsscommitlint')

83
pavelib/quality.py
View File

@@ -505,61 +505,6 @@ def run_xsslint(options):
write_junit_xml('xsslint')
@task
@needs('pavelib.prereqs.install_python_prereqs')
@timed
def run_xsscommitlint():
"""
Runs xss-commit-linter.sh on the current branch.
"""
xsscommitlint_script = "xss-commit-linter.sh"
xsscommitlint_report_dir = (Env.REPORT_DIR / "xsscommitlint")
xsscommitlint_report = xsscommitlint_report_dir / "xsscommitlint.report"
_prepare_report_dir(xsscommitlint_report_dir)
sh(
"{repo_root}/scripts/{xsscommitlint_script} -v | tee {xsscommitlint_report}".format(
repo_root=Env.REPO_ROOT,
xsscommitlint_script=xsscommitlint_script,
xsscommitlint_report=xsscommitlint_report,
),
ignore_error=True
)
xsscommitlint_count = _get_xsscommitlint_count(xsscommitlint_report)
try:
num_violations = int(xsscommitlint_count)
except TypeError:
fail_quality(
'xsscommitlint',
"FAILURE: Number of {xsscommitlint_script} violations could not be found in {xsscommitlint_report}".format(
xsscommitlint_script=xsscommitlint_script, xsscommitlint_report=xsscommitlint_report
)
)
# Print number of violations to log.
violations_count_str = "Number of {xsscommitlint_script} violations: {num_violations}\n".format(
xsscommitlint_script=xsscommitlint_script, num_violations=num_violations
)
# Record the metric
metrics_report = (Env.METRICS_DIR / "xsscommitlint")
_write_metric(violations_count_str, metrics_report)
# Output report to console.
sh(f"cat {metrics_report}", ignore_error=True)
if num_violations:
fail_quality(
'xsscommitlint',
"FAILURE: XSSCommitLinter Failed.\n{error_message}\n"
"See {xsscommitlint_report} or run the following command to hone in on the problem:\n"
" ./scripts/xss-commit-linter.sh -h".format(
error_message=violations_count_str, xsscommitlint_report=xsscommitlint_report
)
)
write_junit_xml("xsscommitlint")
def _write_metric(metric, filename):
"""
Write a given metric to a given file
@@ -663,34 +608,6 @@ def _get_xsslint_counts(filename):
return violations
def _get_xsscommitlint_count(filename):
"""
Returns the violation count from the xsscommitlint report.
Arguments:
filename: The name of the xsscommitlint report.
Returns:
The count of xsscommitlint violations, or None if there is a problem.
"""
report_contents = _get_report_contents(filename, 'xsscommitlint')
if 'No files linted' in report_contents:
return 0
file_count_regex = re.compile(r"^(?P<count>\d+) violations total", re.MULTILINE)
try:
validation_count = None
for count_match in file_count_regex.finditer(report_contents):
if validation_count is None:
validation_count = 0
validation_count += int(count_match.group('count'))
return validation_count
except ValueError:
return None
def _extract_missing_pii_annotations(filename):
"""
Returns the number of uncovered models from the stdout report of django_find_annotations.

6
scripts/generic-ci-tests.sh
View File

@@ -133,12 +133,6 @@ case "$TEST_SUITE" in
run_paver_quality run_stylelint || { EXIT=1; }
echo "Running xss linter report."
run_paver_quality run_xsslint -t $XSSLINT_THRESHOLDS || { EXIT=1; }
if [[ ! -z "$TARGET_BRANCH" ]]; then
echo "Running safe commit linter report."
run_paver_quality run_xsscommitlint || { EXIT=1; }
else
echo "Skipping safe commit linter report."
fi
echo "Running PII checker on all Django models..."
run_paver_quality run_pii_check || { EXIT=1; }
echo "Running reserved keyword checker on all Django models..."

93
scripts/xss-commit-linter.sh
View File

@@ -1,93 +0,0 @@
#!/usr/bin/env bash
set -e
###############################################################################
#
# xss-commit-linter.sh
#
# Executes xsslint/xss_linter.py on the set of files in a particular git commit.
#
###############################################################################
show_help() {
echo "Usage: xss-commit-linter.sh [OPTION]"
echo "Runs the XSS Linter against all files in a git commit."
echo ""
echo "Mandatory arguments to long options are mandatory for short options too."
echo " -h, --help Output this help."
echo " -m, --main-branch=COMMIT Run against files changed between the"
echo " current branch and this commit."
echo " Defaults to origin/master."
echo " -v, --verbose Output details of git commands run."
echo ""
echo "This scripts does not give a grand total. Be sure to check for"
echo "0 violations on each file."
echo ""
echo "For more help using the xss linter, including details on how to"
echo "understand and fix any violations, read the docs here:"
echo ""
echo " https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/preventing_xss/index.html"
}
show_verbose() {
echo "Files linted is based on the following:"
echo "- Current commit: ${current_branch_hash}"
echo "- Main commit: ${MAIN_COMMIT}"
echo "- Target branch: ${TARGET_BRANCH}"
echo "- Merge base command: ${merge_base_command}"
echo "- Merge base: ${merge_base}"
echo "- Diff command: ${diff_command}"
}
for i in "$@"; do
case $i in
-m=*|--main-branch=*)
MAIN_COMMIT="${i#*=}"
shift # past argument=value
;;
-v|--verbose)
show_verbose
;;
-h|--help|*)
# help or unknown option
show_help
exit 0
;;
esac
done
current_branch_hash=`git rev-parse HEAD`
if [ -z "${MAIN_COMMIT+x}" ]; then
if [ -z ${TARGET_BRANCH+x} ]; then
# if commit is not set and no target branch, get hash of current branch
MAIN_COMMIT="origin/master"
else
if [[ $TARGET_BRANCH == origin/* ]]; then
MAIN_COMMIT=$TARGET_BRANCH
else
MAIN_COMMIT=origin/$TARGET_BRANCH
fi
fi
fi
merge_base_command="git merge-base $current_branch_hash $MAIN_COMMIT"
merge_base=$(${merge_base_command})
diff_command="git diff --name-only --diff-filter=ACM $merge_base $current_branch_hash"
diff_files=$(${diff_command})
if [ "$diff_files" = "" ]; then
# When no files are found, automatically display verbose details to help
# understand why.
show_verbose
echo ""
echo "No files linted."
else
for f in $diff_files; do
echo ""
echo "Linting $f:"
./scripts/xsslint/xss_linter.py --config=scripts.xsslint_config $f
done
fi