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

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

lint/tests/dummy/broken.html

@@ -0,0 +1 @@
+THIS LINE HAS TRAILING WHITESPACE    

lint/tests/dummy/broken_ignored.html

@@ -0,0 +1 @@
+THIS LINE HAS TRAILING WHITESPACE    

lint/tests/dummy/lint.whitelist

@@ -0,0 +1 @@
+*:broken_ignored.html

lint/tests/dummy/okay.html

@@ -0,0 +1 @@
+THIS LINE HAS NO TRAILING WHITESPACE

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,20 +40,14 @@ def test_parse_whitelist():
 *:resources/*
 """)
 
-    expected = {
-        '*.pdf': {
-            '*': {None},
-        },
+    expected_data = {
         '.gitmodules': {
             'INDENT TABS': {None},
         },
         'app-uri/*': {
             '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 == ""

tox.ini

@@ -10,6 +10,7 @@ deps =
   {toxinidir}/html5lib
   pytest-travis-fold
   coverage
+  mock
 
 commands =
   coverage run -m pytest