Add type-checking with mypy

Also fix the mishandling of the return type described in #84.

Fixes #84, #85.

Author: nicoddemus

.pre-commit-config.yaml

@@ -1,4 +1,5 @@
-exclude: '^($|.*\.bin)'
+default_language_version:
+  python: "3.10"
 repos:
   - repo: https://github.com/asottile/reorder_python_imports
     rev: v3.8.2
@@ -11,7 +12,6 @@ repos:
     hooks:
       - id: black
         args: [--safe, --quiet]
-        language_version: python3.7
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.2.0
     hooks:
@@ -25,3 +25,11 @@ repos:
         files: README.rst
         language: python
         additional_dependencies: [pygments, restructuredtext_lint]
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v0.971
+    hooks:
+    -   id: mypy
+        files: src/
+        args: []
+        additional_dependencies:
+          - pytest>=7

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 2.2.1 (UNRELEASED)
+
+- Fixed a problem while handling errors using``--gtest_filter``. The recommendation is to use pytest's own
+  filtering facilities (like `-k`) instead of passing filtering arguments to the underlying framework
+  ([#84](https://github.com/pytest-dev/pytest-cpp/issues/84)).
+
+
 # 2.2.0 (2022-08-22)
 
 - Dropped support for Python 3.6.

README.rst

@@ -99,6 +99,11 @@ pytest's ``-o`` option:
 
     $ pytest -o cpp_arguments='-v --log-dir=logs'
 
+**Important**: do not pass filtering arguments (for example ``--gtest_filter``), as this will conflict
+with the plugin functionality and behave incorrectly.
+
+To filter tests, use the standard pytest filtering facilities (such as ``-k``).
+
 cpp_ignore_py_files
 ^^^^^^^^^^^^^^^^^^^
 

mypy.ini

@@ -0,0 +1,17 @@
+[mypy]
+files = src
+check_untyped_defs = True
+disallow_any_generics = True
+ignore_missing_imports = True
+no_implicit_optional = True
+show_error_codes = True
+strict_equality = True
+warn_redundant_casts = True
+warn_return_any = True
+warn_unreachable = True
+warn_unused_configs = True
+no_implicit_reexport = True
+local_partial_types = True
+disallow_untyped_defs = True
+; workaround for https://github.com/python/mypy/issues/10709
+ignore_missing_imports_per_module = True

src/pytest_cpp/boost.py

@@ -1,20 +1,25 @@
+from __future__ import annotations
+
 import io
 import os
 import shutil
 import subprocess
 import tempfile
+from typing import Sequence
 from xml.etree import ElementTree
 
 from pytest_cpp.error import CppTestFailure
+from pytest_cpp.error import Markup
+from pytest_cpp.facade_abc import AbstractFacade
 
 
-class BoostTestFacade(object):
+class BoostTestFacade(AbstractFacade):
     """
     Facade for BoostTests.
     """
 
     @classmethod
-    def is_test_suite(cls, executable):
+    def is_test_suite(cls, executable: str) -> bool:
         try:
             output = subprocess.check_output(
                 [executable, "--help"],
@@ -26,25 +31,29 @@ def is_test_suite(cls, executable):
         else:
             return "--output_format" in output and "log_format" in output
 
-    def list_tests(self, executable):
+    def list_tests(self, executable: str) -> list[str]:
         # unfortunately boost doesn't provide us with a way to list the tests
         # inside the executable, so the test_id is a dummy placeholder :(
         return [os.path.basename(os.path.splitext(executable)[0])]
 
-    def run_test(self, executable, test_id, test_args=(), harness=None):
-        harness = harness or []
-
-        def read_file(name):
+    def run_test(
+        self,
+        executable: str,
+        test_id: str,
+        test_args: Sequence[str] = (),
+        harness: Sequence[str] = (),
+    ) -> tuple[Sequence[BoostTestFailure] | None, str]:
+        def read_file(name: str) -> str:
             try:
                 with io.open(name) as f:
                     return f.read()
             except IOError:
-                return None
+                return ""
 
         temp_dir = tempfile.mkdtemp()
         log_xml = os.path.join(temp_dir, "log.xml")
         report_xml = os.path.join(temp_dir, "report.xml")
-        args = harness + [
+        args = list(harness) + [
             executable,
             "--output_format=XML",
             "--log_sink=%s" % log_xml,
@@ -88,7 +97,7 @@ def read_file(name):
 
         return None, stdout
 
-    def _parse_log(self, log):
+    def _parse_log(self, log: str) -> list[BoostTestFailure]:
         """
         Parse the "log" section produced by BoostTest.
 
@@ -116,19 +125,19 @@ def _parse_log(self, log):
         for elem in parsed_elements:
             filename = elem.attrib["file"]
             linenum = int(elem.attrib["line"])
-            result.append(BoostTestFailure(filename, linenum, elem.text))
+            result.append(BoostTestFailure(filename, linenum, elem.text or ""))
         return result
 
 
 class BoostTestFailure(CppTestFailure):
-    def __init__(self, filename, linenum, contents):
+    def __init__(self, filename: str, linenum: int, contents: str) -> None:
         self.filename = filename
         self.linenum = linenum
         self.lines = contents.splitlines()
 
-    def get_lines(self):
+    def get_lines(self) -> list[tuple[str, Markup]]:
         m = ("red", "bold")
         return [(x, m) for x in self.lines]
 
-    def get_file_reference(self):
+    def get_file_reference(self) -> tuple[str, int]:
         return self.filename, self.linenum

src/pytest_cpp/catch2.py

@@ -1,20 +1,25 @@
+from __future__ import annotations
+
 import os
 import subprocess
 import tempfile
+from typing import Sequence
 from xml.etree import ElementTree
 
 import pytest
 
 from pytest_cpp.error import CppTestFailure
+from pytest_cpp.error import Markup
+from pytest_cpp.facade_abc import AbstractFacade
 
 
-class Catch2Facade(object):
+class Catch2Facade(AbstractFacade):
     """
     Facade for Catch2.
     """
 
     @classmethod
-    def is_test_suite(cls, executable):
+    def is_test_suite(cls, executable: str) -> bool:
         try:
             output = subprocess.check_output(
                 [executable, "--help"],
@@ -26,7 +31,7 @@ def is_test_suite(cls, executable):
         else:
             return "--list-test-names-only" in output
 
-    def list_tests(self, executable):
+    def list_tests(self, executable: str) -> list[str]:
         """
         Executes test with "--list-test-names-only" and gets list of tests
         parsing output like this:
@@ -49,10 +54,15 @@ def list_tests(self, executable):
 
         return result
 
-    def run_test(self, executable, test_id="", test_args=(), harness=None):
-        harness = harness or []
+    def run_test(
+        self,
+        executable: str,
+        test_id: str = "",
+        test_args: Sequence[str] = (),
+        harness: Sequence[str] = (),
+    ) -> tuple[Sequence[Catch2Failure] | None, str]:
         xml_filename = self._get_temp_xml_filename()
-        args = harness + [
+        args = list(harness) + [
             executable,
             test_id,
             "--success",
@@ -103,16 +113,20 @@ def run_test(self, executable, test_id="", test_args=(), harness=None):
                 else:
                     return None, output
 
-        msg = "Internal Error: could not find test " "{test_id} in results:\n{results}"
+        msg = "Internal Error: could not find test {test_id} in results:\n{results}"
 
-        results_list = "\n".join("\n".join(x) for (n, x, f) in results)
-        failure = Catch2Failure(msg.format(test_id=test_id, results=results_list))
+        results_list = "\n".join(n for (n, x, f) in results)
+        failure = Catch2Failure(
+            msg.format(test_id=test_id, results=results_list), 0, ""
+        )
         return [failure], output
 
-    def _get_temp_xml_filename(self):
+    def _get_temp_xml_filename(self) -> str:
         return tempfile.mktemp()
 
-    def _parse_xml(self, xml_filename):
+    def _parse_xml(
+        self, xml_filename: str
+    ) -> Sequence[tuple[str, Sequence[tuple[str, int, str]], bool]]:
         root = ElementTree.parse(xml_filename)
         result = []
         for test_suite in root.findall("Group"):
@@ -121,14 +135,16 @@ def _parse_xml(self, xml_filename):
                 test_name = test_case.attrib["name"]
                 test_result = test_case.find("OverallResult")
                 failures = []
-                if test_result.attrib["success"] == "false":
+                if test_result is not None and test_result.attrib["success"] == "false":
                     test_checks = test_case.findall("Expression")
                     for check in test_checks:
                         file_name = check.attrib["filename"]
-                        line_num = check.attrib["line"]
-                        if check.attrib["success"] == "false":
-                            expected = check.find("Original").text
-                            actual = check.find("Expanded").text
+                        line_num = int(check.attrib["line"])
+                        if check is not None and check.attrib["success"] == "false":
+                            item = check.find("Original")
+                            expected = item.text if item is not None else ""
+                            item = check.find("Expanded")
+                            actual = item.text if item is not None else ""
                             fail_msg = "Expected: {expected}\nActual: {actual}".format(
                                 expected=expected, actual=actual
                             )
@@ -146,14 +162,14 @@ def _parse_xml(self, xml_filename):
 
 
 class Catch2Failure(CppTestFailure):
-    def __init__(self, filename, linenum, lines):
+    def __init__(self, filename: str, linenum: int, lines: str):
         self.lines = lines.splitlines()
         self.filename = filename
-        self.linenum = int(linenum)
+        self.linenum = linenum
 
-    def get_lines(self):
+    def get_lines(self) -> list[tuple[str, Markup]]:
         m = ("red", "bold")
         return [(x, m) for x in self.lines]
 
-    def get_file_reference(self):
+    def get_file_reference(self) -> tuple[str, int]:
         return self.filename, self.linenum

src/pytest_cpp/error.py

@@ -1,18 +1,24 @@
+from __future__ import annotations
+
 import os
 import string
+from typing import Sequence
+from typing import Tuple
 
 from _pytest._code.code import ReprFileLocation
+from _pytest._io import TerminalWriter
 
 
 class CppFailureError(Exception):
     """
     Should be raised by test Facades when a test fails.
-
-    Contains a list of `CppFailure` instances.
     """
 
-    def __init__(self, failures):
-        self.failures = failures
+    def __init__(self, failures: Sequence[CppTestFailure]) -> None:
+        self.failures = list(failures)
+
+
+Markup = Tuple[str, ...]
 
 
 class CppTestFailure(object):
@@ -22,7 +28,7 @@ class CppTestFailure(object):
     message that will be displayed in the terminal.
     """
 
-    def get_lines(self):
+    def get_lines(self) -> list[tuple[str, Markup]]:
         """
         Returns list of (line, markup) that will be displayed to the user,
         where markup can be a sequence of color codes from
@@ -34,7 +40,7 @@ def get_lines(self):
         """
         raise NotImplementedError  # pragma: no cover
 
-    def get_file_reference(self):
+    def get_file_reference(self) -> tuple[str, int]:
         """
         Return tuple of filename, linenum of the failure.
         """
@@ -49,22 +55,22 @@ class CppFailureRepr(object):
 
     failure_sep = "---"
 
-    def __init__(self, failures):
-        self.failures = failures
+    def __init__(self, failures: Sequence[CppTestFailure]) -> None:
+        self.failures = list(failures)
 
-    def __str__(self):
+    def __str__(self) -> str:
         reprs = []
         for failure in self.failures:
             pure_lines = "\n".join(x[0] for x in failure.get_lines())
             repr_loc = self._get_repr_file_location(failure)
             reprs.append("%s\n%s" % (pure_lines, repr_loc))
         return self.failure_sep.join(reprs)
 
-    def _get_repr_file_location(self, failure):
+    def _get_repr_file_location(self, failure: CppTestFailure) -> ReprFileLocation:
         filename, linenum = failure.get_file_reference()
         return ReprFileLocation(filename, linenum, "C++ failure")
 
-    def toterminal(self, tw):
+    def toterminal(self, tw: TerminalWriter) -> None:
         for index, failure in enumerate(self.failures):
             filename, linenum = failure.get_file_reference()
             code_lines = get_code_context_around_line(filename, linenum)
@@ -84,7 +90,7 @@ def toterminal(self, tw):
                 tw.line(self.failure_sep, cyan=True)
 
 
-def get_code_context_around_line(filename, linenum):
+def get_code_context_around_line(filename: str, linenum: int) -> list[str]:
     """
     return code context lines, with the last line being the line at
     linenum.
@@ -98,7 +104,7 @@ def get_code_context_around_line(filename, linenum):
     return []
 
 
-def get_left_whitespace(line):
+def get_left_whitespace(line: str) -> str:
     result = ""
     for c in line:
         if c in string.whitespace: