From 0baebf4d8588b8d83eda68aac2712f7b1b879c26 Mon Sep 17 00:00:00 2001 From: Geoffrey Sneddon Date: Wed, 14 Sep 2016 10:53:46 +0100 Subject: [PATCH] Don't bother running lints if we're going to ignore them (#111) * Don't bother running lints if we're going to ignore them * Remove now dead branch, given * is handled elsewhere now * Add some basic tests for lint.lint --- lint/lint.py | 18 +++-- lint/tests/dummy/broken.html | 1 + lint/tests/dummy/broken_ignored.html | 1 + lint/tests/dummy/lint.whitelist | 1 + lint/tests/dummy/okay.html | 1 + lint/tests/test_lint.py | 110 ++++++++++++++++++++++++--- tox.ini | 1 + 7 files changed, 117 insertions(+), 16 deletions(-) create mode 100644 lint/tests/dummy/broken.html create mode 100644 lint/tests/dummy/broken_ignored.html create mode 100644 lint/tests/dummy/lint.whitelist create mode 100644 lint/tests/dummy/okay.html diff --git a/lint/lint.py b/lint/lint.py index 8d7559169dd0c3..200e446af0dcff 100644 --- a/lint/lint.py +++ b/lint/lint.py @@ -52,6 +52,7 @@ def parse_whitelist(f): """ data = defaultdict(lambda:defaultdict(set)) + ignored_files = set() for line in f: line = line.strip() @@ -64,9 +65,13 @@ def parse_whitelist(f): parts[-1] = int(parts[-1]) error_type, file_match, line_number = parts - data[file_match][error_type].add(line_number) - return data + if error_type == "*": + ignored_files.add(file_match) + else: + data[file_match][error_type].add(line_number) + + return data, ignored_files def filter_whitelist_errors(data, path, errors): @@ -79,9 +84,7 @@ def filter_whitelist_errors(data, path, errors): for file_match, whitelist_errors in iteritems(data): if fnmatch.fnmatch(path, file_match): for i, (error_type, msg, path, line) in enumerate(errors): - if "*" in whitelist_errors: - whitelisted[i] = True - elif error_type in whitelist_errors: + if error_type in whitelist_errors: allowed_lines = whitelist_errors[error_type] if None in allowed_lines or line in allowed_lines: whitelisted[i] = True @@ -365,7 +368,7 @@ def lint(repo_root, paths, output_json): last = None with open(os.path.join(repo_root, "lint.whitelist")) as f: - whitelist = parse_whitelist(f) + whitelist, ignored_files = parse_whitelist(f) if output_json: output_errors = output_errors_json @@ -398,6 +401,9 @@ def process_errors(path, errors): if not os.path.exists(abs_path): continue + if any(fnmatch.fnmatch(path, file_match) for file_match in ignored_files): + continue + errors = check_path(repo_root, path) last = process_errors(path, errors) or last diff --git a/lint/tests/dummy/broken.html b/lint/tests/dummy/broken.html new file mode 100644 index 00000000000000..74793c43caf016 --- /dev/null +++ b/lint/tests/dummy/broken.html @@ -0,0 +1 @@ +THIS LINE HAS TRAILING WHITESPACE diff --git a/lint/tests/dummy/broken_ignored.html b/lint/tests/dummy/broken_ignored.html new file mode 100644 index 00000000000000..74793c43caf016 --- /dev/null +++ b/lint/tests/dummy/broken_ignored.html @@ -0,0 +1 @@ +THIS LINE HAS TRAILING WHITESPACE diff --git a/lint/tests/dummy/lint.whitelist b/lint/tests/dummy/lint.whitelist new file mode 100644 index 00000000000000..a763e4432e5afa --- /dev/null +++ b/lint/tests/dummy/lint.whitelist @@ -0,0 +1 @@ +*:broken_ignored.html diff --git a/lint/tests/dummy/okay.html b/lint/tests/dummy/okay.html new file mode 100644 index 00000000000000..a3178a3c83a383 --- /dev/null +++ b/lint/tests/dummy/okay.html @@ -0,0 +1 @@ +THIS LINE HAS NO TRAILING WHITESPACE diff --git a/lint/tests/test_lint.py b/lint/tests/test_lint.py index 6a551051d3a299..81475aa7df34df 100644 --- a/lint/tests/test_lint.py +++ b/lint/tests/test_lint.py @@ -1,9 +1,23 @@ from __future__ import unicode_literals -from ..lint import filter_whitelist_errors, parse_whitelist +import os + +import mock +import pytest import six -def test_lint(): +from .. import lint as lint_mod +from ..lint import filter_whitelist_errors, parse_whitelist, lint + +_dummy_repo = os.path.join(os.path.dirname(__file__), "dummy") + + +def _mock_lint(name): + wrapped = getattr(lint_mod, name) + return mock.patch(lint_mod.__name__ + "." + name, wraps=wrapped) + + +def test_filter_whitelist_errors(): filtered = filter_whitelist_errors({}, '', []) assert filtered == [] @@ -26,10 +40,7 @@ def test_parse_whitelist(): *:resources/* """) - expected = { - '*.pdf': { - '*': {None}, - }, + expected_data = { '.gitmodules': { 'INDENT TABS': {None}, }, @@ -37,9 +48,6 @@ def test_parse_whitelist(): 'TRAILING WHITESPACE': {None}, 'INDENT TABS': {None}, }, - 'resources/*': { - '*': {None}, - }, 'streams/resources/test-utils.js': { 'CONSOLE': {12}, 'CR AT EOL': {None}, @@ -51,4 +59,86 @@ def test_parse_whitelist(): 'CR AT EOL': {None}, }, } - assert parse_whitelist(input_buffer) == expected + expected_ignored = {"*.pdf", "resources/*"} + data, ignored = parse_whitelist(input_buffer) + assert data == expected_data + assert ignored == expected_ignored + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_no_files(capsys): + rv = lint(_dummy_repo, [], False) + assert rv == 0 + out, err = capsys.readouterr() + assert out == "" + assert err == "" + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_ignored_file(capsys): + with _mock_lint("check_path") as mocked_check_path: + with _mock_lint("check_file_contents") as mocked_check_file_contents: + rv = lint(_dummy_repo, ["broken_ignored.html"], False) + assert rv == 0 + assert not mocked_check_path.called + assert not mocked_check_file_contents.called + out, err = capsys.readouterr() + assert out == "" + assert err == "" + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_not_existing_file(capsys): + with _mock_lint("check_path") as mocked_check_path: + with _mock_lint("check_file_contents") as mocked_check_file_contents: + # really long path-linted filename + name = "a" * 256 + ".html" + rv = lint(_dummy_repo, [name], False) + assert rv == 0 + assert not mocked_check_path.called + assert not mocked_check_file_contents.called + out, err = capsys.readouterr() + assert out == "" + assert err == "" + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_passing(capsys): + with _mock_lint("check_path") as mocked_check_path: + with _mock_lint("check_file_contents") as mocked_check_file_contents: + rv = lint(_dummy_repo, ["okay.html"], False) + assert rv == 0 + assert mocked_check_path.call_count == 1 + assert mocked_check_file_contents.call_count == 1 + out, err = capsys.readouterr() + assert out == "" + assert err == "" + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_failing(capsys): + with _mock_lint("check_path") as mocked_check_path: + with _mock_lint("check_file_contents") as mocked_check_file_contents: + rv = lint(_dummy_repo, ["broken.html"], False) + assert rv == 1 + assert mocked_check_path.call_count == 1 + assert mocked_check_file_contents.call_count == 1 + out, err = capsys.readouterr() + assert "TRAILING WHITESPACE" in out + assert "broken.html 1 " in out + assert err == "" + + +@pytest.mark.skipif(six.PY3, reason="lint.lint doesn't support Python 3") +def test_lint_passing_and_failing(capsys): + with _mock_lint("check_path") as mocked_check_path: + with _mock_lint("check_file_contents") as mocked_check_file_contents: + rv = lint(_dummy_repo, ["broken.html", "okay.html"], False) + assert rv == 1 + assert mocked_check_path.call_count == 2 + assert mocked_check_file_contents.call_count == 2 + out, err = capsys.readouterr() + assert "TRAILING WHITESPACE" in out + assert "broken.html 1 " in out + assert "okay.html" not in out + assert err == "" diff --git a/tox.ini b/tox.ini index fdd8cf47f4ceef..6638cc7cbdc3c5 100644 --- a/tox.ini +++ b/tox.ini @@ -10,6 +10,7 @@ deps = {toxinidir}/html5lib pytest-travis-fold coverage + mock commands = coverage run -m pytest