Fix all mypy type checking errors

- Add return type annotation to PatchtestParser.get_parser()
- Add type annotations to patchtest_result decorator and wrapper
- Add type annotations to MboxReader methods (__exit__, __iter__, __next__)
- Add type annotation for 'lines' list in MboxReader.__next__()
- Fix Patch.__init__ to handle Message.get_payload() union type
- Add None checks for email header fields
- Fix TargetRepo.can_be_merged to return bool consistently
- Fix git.execute() call to use proper keyword argument
- Add None checks for module spec and loader in dynamic import
- Add type annotations to all test functions in core.py and oe.py
- Add type: ignore comments for untyped unidiff imports
- Add types-unidiff to dev dependencies
- Import Tuple from typing for test function signatures

Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>

Author: threexc

pyproject.toml

@@ -40,6 +40,7 @@ dev = [
   "pytest>=7.4.0",
   "pytest-cov>=4.1.0",
   "types-setuptools",
+  "types-unidiff",
 ]
 test = [
   "pytest>=7.4.0",

src/patchtest2/mbox.py

@@ -15,7 +15,7 @@
 import re
 from dataclasses import dataclass
 from email.message import Message
-from typing import List, Optional
+from typing import List, Optional, Iterator, Any
 
 import git
 
@@ -29,14 +29,14 @@ def __init__(self, filepath: str) -> None:
     def __enter__(self) -> "MboxReader":
         return self
 
-    def __exit__(self, exc_type, exc_value, exc_traceback) -> None:
+    def __exit__(self, exc_type: Any, exc_value: Any, exc_traceback: Any) -> None:
         self.handle.close()
 
-    def __iter__(self):
+    def __iter__(self) -> Iterator[Message]:
         return iter(self.__next__())
 
-    def __next__(self):
-        lines = []
+    def __next__(self) -> Iterator[Message]:
+        lines: List[bytes] = []
         while True:
             line = self.handle.readline()
             if line == b"" or line.startswith(b"From "):
@@ -51,13 +51,19 @@ def __next__(self):
 class Patch:
     def __init__(self, data: Message) -> None:
         self.data = data
-        self.author: str = data["From"]
-        self.to: str = data["To"]
-        self.cc: str = data["Cc"]
-        self.subject: str = data["Subject"]
-        self.split_body: List[str] = re.split("---", data.get_payload(), maxsplit=1)
+        self.author: str = data["From"] or ""
+        self.to: str = data["To"] or ""
+        self.cc: str = data["Cc"] or ""
+        self.subject: str = data["Subject"] or ""
+        payload = data.get_payload()
+        # Handle payload that might be a list or string
+        if isinstance(payload, list):
+            payload_str = str(payload[0]) if payload else ""
+        else:
+            payload_str = str(payload)
+        self.split_body: List[str] = re.split("---", payload_str, maxsplit=1)
         self.commit_message: str = self.split_body[0]
-        self.diff: str = self.split_body[1]
+        self.diff: str = self.split_body[1] if len(self.split_body) > 1 else ""
         # get the shortlog, but make sure to exclude bracketed prefixes
         # before the colon, and remove extra whitespace/newlines
         self.shortlog: str = (
@@ -138,20 +144,16 @@ def checkout_working_branch(self, target_branch: str) -> None:
             ["git", "checkout", "-B", self.working_branch, target_branch]
         )
 
-    def can_be_merged(self, patchfile: str):
+    def can_be_merged(self, patchfile: str) -> bool:
         # We don't actually want to propagate the error if the patch
         # can't merge, so put a try-except around it. However, if the
-        # check fails, return the error message so that it can be parsed
-        # to identify the failing message in the series
+        # check fails, return False
         try:
             self.merge_patch(patchfile)
-            result = True
-
-        except git.exc.GitCommandError as ce:
-            result = ce
+            return True
+        except git.exc.GitCommandError:
             self.abort_merge()
-
-        return result
+            return False
 
     def merge_patch(self, patchfile: str) -> None:
         self.repo.git.execute(

src/patchtest2/parser.py

@@ -23,7 +23,7 @@ class PatchtestParser(object):
     """Abstract the patchtest argument parser"""
 
     @classmethod
-    def get_parser(cls):
+    def get_parser(cls) -> argparse.ArgumentParser:
         parser = argparse.ArgumentParser()
 
         target_patch_group = parser.add_mutually_exclusive_group(required=True)

src/patchtest2/patchtest.py

@@ -64,6 +64,12 @@ def _load_test_modules(self) -> None:
                     spec = importlib.util.spec_from_file_location(
                         f"patchtest2.suites.{suite_name}", module_file
                     )
+                    if spec is None or spec.loader is None:
+                        logger.warning(
+                            f"Could not load spec for suite '{suite_name}' from {module_file}"
+                        )
+                        continue
+
                     module = importlib.util.module_from_spec(spec)
 
                     # Add to sys.modules to handle imports within the module

src/patchtest2/results.py

@@ -1,9 +1,9 @@
 import functools
 import inspect
-from typing import Any, Tuple
+from typing import Any, Tuple, Callable
 
 
-def patchtest_result(func):
+def patchtest_result(func: Callable[..., Tuple[str, str, str]]) -> Callable[..., str]:
     """Decorator that formats test results consistently.
 
     Test functions must return a tuple of (subject, result, reason) where:
@@ -13,7 +13,7 @@ def patchtest_result(func):
     """
 
     @functools.wraps(func)
-    def wrapper(*args, **kwargs):
+    def wrapper(*args: Any, **kwargs: Any) -> str:
         result = func(*args, **kwargs)
 
         # Validate return value

src/patchtest2/suites/core.py

@@ -3,12 +3,14 @@
 import pyparsing
 import unidiff
 
-import patchtest2.patterns as patterns
+import patchtest2.patterns as patterns  # type: ignore[import-untyped]
 from patchtest2.results import patchtest_result
+from patchtest2.mbox import Patch
+from typing import Tuple
 
 
 @patchtest_result
-def test_mbox_has_signed_off_by(target):
+def test_mbox_has_signed_off_by(target: Patch) -> Tuple[str, str, str]:
     result = "FAIL"
     if patterns.signed_off_by.search_string(target.commit_message):
         result = "PASS"
@@ -17,7 +19,7 @@ def test_mbox_has_signed_off_by(target):
 
 
 @patchtest_result
-def test_mbox_shortlog_format(target):
+def test_mbox_shortlog_format(target: Patch) -> Tuple[str, str, str]:
     result = "PASS"
     reason = None
     if not target.shortlog.strip():
@@ -38,7 +40,7 @@ def test_mbox_shortlog_format(target):
 
 
 @patchtest_result
-def test_mbox_shortlog_length(target):
+def test_mbox_shortlog_length(target: Patch) -> Tuple[str, str, str]:
     shortlog = re.sub("^(\[.*?\])+ ", "", target.shortlog)
     shortlog_len = len(shortlog)
     result = "PASS"
@@ -55,7 +57,7 @@ def test_mbox_shortlog_length(target):
 
 
 @patchtest_result
-def test_mbox_has_commit_message(target):
+def test_mbox_has_commit_message(target: Patch) -> Tuple[str, str, str]:
     result = "PASS"
     reason = "Please include a commit message on your patch explaining the change"
 
@@ -74,7 +76,7 @@ def test_mbox_has_commit_message(target):
 
 
 @patchtest_result
-def test_mbox_unidiff_parse_error(target):
+def test_mbox_unidiff_parse_error(target: Patch) -> Tuple[str, str, str]:
     result = "PASS"
     reason = f'Patch "{target.shortlog}" contains malformed diff lines.'
 
@@ -87,7 +89,7 @@ def test_mbox_unidiff_parse_error(target):
 
 
 @patchtest_result
-def test_mbox_revert_signed_off_by_exception(target):
+def test_mbox_revert_signed_off_by_exception(target: Patch) -> Tuple[str, str, str]:
     """Skip signed-off-by test for revert commits"""
     result = "PASS"
     reason = None
@@ -106,7 +108,7 @@ def test_mbox_revert_signed_off_by_exception(target):
 
 # Additional helper test that might be useful
 @patchtest_result
-def test_mbox_shortlog_revert_format(target):
+def test_mbox_shortlog_revert_format(target: Patch) -> Tuple[str, str, str]:
     """Test revert commit shortlog format"""
     result = "PASS"
     reason = None

src/patchtest2/suites/oe.py

@@ -3,12 +3,14 @@
 import pyparsing
 import unidiff
 
-import patchtest2.patterns as patterns
+import patchtest2.patterns as patterns  # type: ignore[import-untyped]
 from patchtest2.results import patchtest_result
+from patchtest2.mbox import Patch
+from typing import Tuple
 
 
 @patchtest_result
-def test_mbox_commit_message_user_tags(target):
+def test_mbox_commit_message_user_tags(target: Patch) -> Tuple[str, str, str]:
     """Test for GitHub-style username tags (@username) in commit messages"""
     result = "PASS"
     reason = "Mbox includes one or more GitHub-style username tags. Ensure that any '@' symbols are stripped out of usernames"
@@ -20,7 +22,7 @@ def test_mbox_commit_message_user_tags(target):
 
 
 @patchtest_result
-def test_mbox_non_auh_upgrade(target):
+def test_mbox_non_auh_upgrade(target: Patch) -> Tuple[str, str, str]:
     """Test that patch is not from AUH (Auto Upgrade Helper)"""
     result = "PASS"
     reason = f"Invalid author {patterns.auh_email}. Resend the series with a valid patch author"
@@ -32,7 +34,7 @@ def test_mbox_non_auh_upgrade(target):
 
 
 @patchtest_result
-def test_mbox_target_mailing_list_meta_project(target):
+def test_mbox_target_mailing_list_meta_project(target: Patch) -> Tuple[str, str, str]:
     """Check for meta-* project tags in subject line"""
     result = "PASS"
     reason = "Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists"
@@ -46,7 +48,7 @@ def test_mbox_target_mailing_list_meta_project(target):
 
 
 @patchtest_result
-def test_mbox_bugzilla_entry_format(target):
+def test_mbox_bugzilla_entry_format(target: Patch) -> Tuple[str, str, str]:
     """Test for proper Bugzilla entry format in commit messages"""
     result = "PASS"
     reason = None
@@ -63,7 +65,7 @@ def test_mbox_bugzilla_entry_format(target):
 
 
 @patchtest_result
-def test_mbox_author_valid(target):
+def test_mbox_author_valid(target: Patch) -> Tuple[str, str, str]:
     """Test for valid patch author"""
     result = "PASS"
     reason = (