Make check_coverage.py more testable and add tests.

scripts/dev/check_coverage.py

@@ -77,29 +77,33 @@ PERFECT_FILES = [
 ]
 
 
-def check_coverage():
+class Skipped(Exception):
+
+    """Exception raised when skipping coverage checks."""
+
+    def __init__(self, reason):
+        self.reason = reason
+        super().__init__("Skipping coverage checks " + reason)
+
+
+def check(fileobj, perfect_files):
     """Main entry point which parses/checks coverage.xml if applicable."""
     if sys.platform != 'linux':
-        print("Skipping coverage checks on non-Linux system.")
+        raise Skipped("on non-Linux system.")
-        return 0
     elif '-k' in sys.argv[1:]:
-        print("Skipping coverage checks because -k is given.")
+        raise Skipped("because -k is given.")
-        return 0
     elif '-m' in sys.argv[1:]:
-        print("Skipping coverage checks because -m is given.")
+        raise Skipped("because -m is given.")
-        return 0
     elif any(arg.startswith('tests' + os.sep) for arg in sys.argv[1:]):
-        print("Skipping coverage checks because a filename is given.")
+        raise Skipped("because a filename is given.")
-        return 0
 
-    for path in PERFECT_FILES:
+    for path in perfect_files:
-        assert os.path.exists(os.path.join(*path.split('/'))), path
+        assert os.path.exists(path)
 
-    with open('.coverage.xml', encoding='utf-8') as f:
+    tree = ElementTree.parse(fileobj)
-        tree = ElementTree.parse(f)
     classes = tree.getroot().findall('./packages/package/classes/class')
 
-    status = 0
+    messages = []
 
     for klass in classes:
         filename = klass.attrib['filename']
@@ -109,7 +113,6 @@ def check_coverage():
         assert 0 <= line_cov <= 100, line_cov
         assert 0 <= branch_cov <= 100, branch_cov
         assert '\\' not in filename, filename
-        assert '/' in filename, filename
 
         # Files without any branches have 0% coverage
         has_branches = klass.find('./lines/line[@branch="true"]') is not None
@@ -118,16 +121,14 @@ def check_coverage():
         else:
             is_bad = line_cov < 100
 
-        if filename in PERFECT_FILES and is_bad:
+        if filename in perfect_files and is_bad:
-            status = 1
+            messages.append(("{} has {}% line and {}% branch coverage!".format(
-            print("{} has {}% line and {}% branch coverage!".format(
+                filename, line_cov, branch_cov)))
-                filename, line_cov, branch_cov))
+        elif filename not in perfect_files and not is_bad:
-        elif filename not in PERFECT_FILES and not is_bad:
+            messages.append("{} has 100% coverage but is not in "
-            status = 1
+                            "perfect_files!".format(filename))
-            print("{} has 100% coverage but is not in PERFECT_FILES!".format(
-                filename))
 
-    return status
+    return messages
 
 
 def main():
@@ -137,9 +138,19 @@ def main():
         The return code to return.
     """
     utils.change_cwd()
-    status = check_coverage()
+
+    try:
+        with open('.coverage.xml', encoding='utf-8') as f:
+            messages = check(f, PERFECT_FILES)
+    except Skipped as e:
+        print(e)
+        messages = []
+
+    for msg in messages:
+        print(msg)
+
     os.remove('.coverage.xml')
-    return status
+    return 1 if messages else 0
 
 
 if __name__ == '__main__':

tests/unit/scripts/test_check_coverage.py

@@ -0,0 +1,168 @@
+#!/usr/bin/env python3
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2015 Florian Bruhin (The Compiler) <mail@qutebrowser.org>
+
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+import sys
+
+import pytest
+
+from scripts.dev import check_coverage
+
+
+pytest_plugins = 'pytester'
+
+
+class CovtestHelper:
+
+    """Helper object for covtest fixture.
+
+    Attributes:
+        _testdir: The testdir fixture from pytest.
+        _monkeypatch: The monkeypatch fixture from pytest.
+    """
+
+    def __init__(self, testdir, monkeypatch):
+        self._testdir = testdir
+        self._monkeypatch = monkeypatch
+
+    def makefile(self, code):
+        """Generate a module.py for the given code."""
+        self._testdir.makepyfile(module=code)
+
+    def run(self):
+        """Run pytest with coverage for the given module.py."""
+        coveragerc = str(self._testdir.tmpdir / 'coveragerc')
+        return self._testdir.runpytest('--cov=module',
+                                       '--cov-config={}'.format(coveragerc),
+                                       '--cov-report=xml')
+
+    def check(self, perfect_files=None):
+        """Run check_coverage.py and run its return value."""
+        coverage_file = self._testdir.tmpdir / 'coverage.xml'
+
+        if perfect_files is None:
+            perfect_files = ['module.py']
+
+        argv = [sys.argv[0]]
+        self._monkeypatch.setattr('scripts.dev.check_coverage.sys.argv', argv)
+
+        with self._testdir.tmpdir.as_cwd():
+            with coverage_file.open(encoding='utf-8') as f:
+                return check_coverage.check(f, perfect_files=perfect_files)
+
+    def check_skipped(self, args, reason):
+        """Run check_coverage.py and make sure it's skipped."""
+        argv = [sys.argv[0]] + list(args)
+        self._monkeypatch.setattr('scripts.dev.check_coverage.sys.argv', argv)
+        with pytest.raises(check_coverage.Skipped) as excinfo:
+            return check_coverage.check(None, perfect_files=[])
+        assert excinfo.value.reason == reason
+
+
+@pytest.fixture
+def covtest(testdir, monkeypatch):
+    """Fixture which provides a coveragerc and a test to call module.func."""
+    testdir.makefile(ext='', coveragerc="""
+        [run]
+        branch=True
+    """)
+    testdir.makepyfile(test_module="""
+        from module import func
+
+        def test_module():
+            func()
+    """)
+    return CovtestHelper(testdir, monkeypatch)
+
+
+def test_tested_no_branches(covtest):
+    covtest.makefile("""
+        def func():
+            pass
+    """)
+    covtest.run()
+    assert covtest.check() == []
+
+
+def test_tested_with_branches(covtest):
+    covtest.makefile("""
+        def func2(arg):
+            if arg:
+                pass
+            else:
+                pass
+
+        def func():
+            func2(True)
+            func2(False)
+    """)
+    covtest.run()
+    assert covtest.check() == []
+
+
+def test_untested(covtest):
+    covtest.makefile("""
+        def func():
+            pass
+
+        def untested():
+            pass
+    """)
+    covtest.run()
+    expected = 'module.py has 75.0% line and 0.0% branch coverage!'
+    assert covtest.check() == [expected]
+
+
+def test_untested_branches(covtest):
+    covtest.makefile("""
+        def func2(arg):
+            if arg:
+                pass
+
+        def func():
+            func2(True)
+    """)
+    covtest.run()
+    expected = 'module.py has 100.0% line and 50.0% branch coverage!'
+    assert covtest.check() == [expected]
+
+
+def test_tested_unlisted(covtest):
+    covtest.makefile("""
+        def func():
+            pass
+    """)
+    covtest.run()
+    expected = 'module.py has 100% coverage but is not in perfect_files!'
+    assert covtest.check(perfect_files=[]) == [expected]
+
+
+@pytest.mark.parametrize('args, reason', [
+    (['-k', 'foo'], "because -k is given."),
+    (['-m', 'foo'], "because -m is given."),
+    (['blah', '-m', 'foo'], "because -m is given."),
+    (['tests/foo'], "because a filename is given."),
+])
+def test_skipped_args(covtest, args, reason):
+    covtest.check_skipped(args, reason)
+
+
+def test_skipped_windows(covtest, monkeypatch):
+    monkeypatch.setattr('scripts.dev.check_coverage.sys.platform', 'toaster')
+    covtest.check_skipped([], "on non-Linux system.")