[Python] Improve typing

.github/workflows/mypy-validation.yml

@@ -18,18 +18,20 @@ on:
   push:
     branches: [ master ]
     paths:
-      - 'src/controller/python/matter/ChipDeviceCtrl.py'
-      - 'src/python_testing/matter_testing_infrastructure/**/*.py'
-      - 'scripts/tests/chipyaml/paths_finder.py'
-      - 'scripts/tests/chiptest/**/*.py'
-      - 'scripts/tests/run_test_suite.py'
+      - "scripts/tests/chiptest/**/*.py"
+      - "scripts/tests/chipyaml/paths_finder.py"
+      - "scripts/tests/run_test_suite.py"
+      - "src/controller/python/matter/ChipDeviceCtrl.py"
+      - "src/controller/python/matter/MatterTlvJson.py"
+      - "src/python_testing/matter_testing_infrastructure/**/*.py"
   pull_request:
     paths:
-      - 'src/controller/python/matter/ChipDeviceCtrl.py'
-      - 'src/python_testing/matter_testing_infrastructure/**/*.py'
-      - 'scripts/tests/chipyaml/paths_finder.py'
-      - 'scripts/tests/chiptest/**/*.py'
-      - 'scripts/tests/run_test_suite.py'
+      - "scripts/tests/chiptest/**/*.py"
+      - "scripts/tests/chipyaml/paths_finder.py"
+      - "scripts/tests/run_test_suite.py"
+      - "src/controller/python/matter/ChipDeviceCtrl.py"
+      - "src/controller/python/matter/MatterTlvJson.py"
+      - "src/python_testing/matter_testing_infrastructure/**/*.py"
 
 jobs:
   mypy-check:
@@ -38,7 +40,8 @@ jobs:
 
     container:
       image: ghcr.io/project-chip/chip-build:181
-      options: --privileged --sysctl "net.ipv6.conf.all.disable_ipv6=0 net.ipv4.conf.all.forwarding=1 net.ipv6.conf.all.forwarding=1"
+      options: --privileged --sysctl "net.ipv6.conf.all.disable_ipv6=0
+        net.ipv4.conf.all.forwarding=1 net.ipv6.conf.all.forwarding=1"
 
     steps:
       - name: Checkout
@@ -54,34 +57,12 @@ jobs:
           scripts/run_in_build_env.sh './scripts/build_python.sh --install_virtual_env out/venv'
           out/venv/bin/pip install mypy
 
-      - name: Run mypy validation (ChipDeviceCtrl.py)
-        shell: bash
-        run: |
-          set +e
-
-          OUTPUT=$(./scripts/run_in_python_env.sh out/venv "mypy --follow-imports=skip src/controller/python/matter/ChipDeviceCtrl.py" 2>&1)
-          STATUS=$?
-
-          echo "$OUTPUT"
-
-          ERRORS=$(echo "$OUTPUT" | grep -c '^src/controller/python/matter/ChipDeviceCtrl.py:[0-9]\+: error:')
-
-          if [ "$STATUS" -eq 0 ]; then
-            echo "No mypy errors found in ChipDeviceCtrl.py"
-            exit 0
-          elif [ "$ERRORS" -gt 0 ]; then
-            echo "Mypy found $ERRORS error(s) in ChipDeviceCtrl.py"
-            echo "$OUTPUT" | grep '^src/controller/python/matter/ChipDeviceCtrl.py:[0-9]\+: error:'
-            exit 1
-          else
-            echo "Mypy exited with error but no errors in ChipDeviceCtrl.py"
-            exit 0
-          fi
-
-      - name: Run mypy validation (MatterJsonTlv.py)
+      - name: Run mypy validation (without following imports)
         run: |
           ./scripts/run_in_python_env.sh out/venv "mypy --follow-imports=skip \
-            src/controller/python/matter/MatterTlvJson.py"
+            src/controller/python/matter/ChipDeviceCtrl.py \
+            src/controller/python/matter/MatterTlvJson.py \
+          "
 
       - name: Run mypy validation
         run: |
@@ -92,22 +73,22 @@ jobs:
           # Eventually we should just check all files in the chip/testing directory
 
           ./scripts/run_in_python_env.sh out/venv "mypy \
+            scripts/tests/chiptest/ \
+            scripts/tests/chipyaml/paths_finder.py \
+            scripts/tests/run_test_suite.py \
             src/python_testing/matter_testing_infrastructure/matter/testing/apps.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/tasks.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/taglist_and_topology_test.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/pics.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/runner.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/basic_composition.py \
             src/python_testing/matter_testing_infrastructure/matter/testing/choice_conformance.py \
             src/python_testing/matter_testing_infrastructure/matter/testing/commissioning.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/decorators.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/basic_composition.py \
             src/python_testing/matter_testing_infrastructure/matter/testing/conformance.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/spec_parsing.py \
-            src/python_testing/matter_testing_infrastructure/matter/testing/metadata.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/decorators.py \
             src/python_testing/matter_testing_infrastructure/matter/testing/matter_testing.py \
-            scripts/tests/chipyaml/paths_finder.py \
-            scripts/tests/chiptest/ \
-            scripts/tests/run_test_suite.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/metadata.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/pics.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/runner.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/spec_parsing.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/taglist_and_topology_test.py \
+            src/python_testing/matter_testing_infrastructure/matter/testing/tasks.py \
           "
 
           # Print a reminder about expanding coverage

pyproject.toml

@@ -29,6 +29,11 @@ exclude = [
 
 [tool.pyright]
 reportMissingTypeStubs = "warning"
+extraPaths = [
+    "./src/python_testing/matter_testing_infrastructure",
+    "./src/controller/python",
+    "./scripts/py_matter_yamltests",
+]
 
 [tool.ruff.lint]
 select = [
@@ -62,8 +67,9 @@ ignore = [
 ]
 
 [tool.mypy]
-mypy_path = ["matter/typings"]
+check_untyped_defs = true
+disallow_incomplete_defs = true
+explicit_package_bases = true
+ignore_missing_imports = true
 namespace_packages = true
 warn_unused_configs = true
-ignore_missing_imports = true
-explicit_package_bases = true

scripts/py_matter_yamltests/matter/yamltests/parser.py

@@ -1453,8 +1453,8 @@ def __next__(self) -> TestStep:
 
 @dataclass
 class TestParserConfig:
-    pics: str = None
-    definitions: SpecDefinitions = None
+    pics: str | None = None
+    definitions: SpecDefinitions | None = None
     config_override: dict = field(default_factory=dict)
 
 

scripts/tests/chiptest/__init__.py

@@ -17,9 +17,9 @@
 import json
 import logging
 import os
+from collections.abc import Iterable
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Iterator, Set
 
 import yaml
 
@@ -77,15 +77,15 @@ def _IsValidYamlTest(name: str) -> bool:
     return name not in INVALID_TESTS
 
 
-def _LoadManualTestsJson(json_file_path: str) -> Iterator[str]:
+def _LoadManualTestsJson(json_file_path: str) -> Iterable[str]:
     with open(json_file_path) as f:
         data = json.load(f)
         for c in data["collection"]:
             for name in data[c]:
                 yield f"{name}.yaml"
 
 
-def _GetManualTests() -> Set[str]:
+def _GetManualTests() -> set[str]:
     manualtests: set[str] = set()
 
     # Flagged as manual from: src/app/tests/suites/manualTests.json
@@ -95,7 +95,7 @@ def _GetManualTests() -> Set[str]:
     return manualtests
 
 
-def _GetFlakyTests() -> Set[str]:
+def _GetFlakyTests() -> set[str]:
     """List of flaky tests.
 
     While this list is empty, it remains here in case we need to quickly add a new test
@@ -104,7 +104,7 @@ def _GetFlakyTests() -> Set[str]:
     return set()
 
 
-def _GetSlowTests() -> Set[str]:
+def _GetSlowTests() -> set[str]:
     """Generally tests using sleep() a bit too freely.
 
        10s seems like a good threshold to consider something slow
@@ -139,7 +139,7 @@ def _GetSlowTests() -> Set[str]:
     }
 
 
-def _GetExtraSlowTests() -> Set[str]:
+def _GetExtraSlowTests() -> set[str]:
     """Generally tests using sleep() so much they should never run in CI.
 
        1 minute seems like a good threshold to consider something extra slow
@@ -149,7 +149,7 @@ def _GetExtraSlowTests() -> Set[str]:
     }
 
 
-def _GetInDevelopmentTests() -> Set[str]:
+def _GetInDevelopmentTests() -> set[str]:
     """Tests that fail in YAML for some reason."""
     return {
         "Test_TC_PSCFG_1_1.yaml",  # Power source configuration cluster is deprecated and removed from all-clusters
@@ -169,14 +169,14 @@ def _GetInDevelopmentTests() -> Set[str]:
     }
 
 
-def _GetChipToolUnsupportedTests() -> Set[str]:
+def _GetChipToolUnsupportedTests() -> set[str]:
     """Tests that fail in chip-tool for some reason"""
     return {
         "TestDiagnosticLogsDownloadCommand",  # chip-tool does not implement a bdx download command.
     }
 
 
-def _GetDarwinFrameworkToolUnsupportedTests() -> Set[str]:
+def _GetDarwinFrameworkToolUnsupportedTests() -> set[str]:
     """Tests that fail in darwin-framework-tool for some reason"""
     return {
         "DL_LockUnlock",  # darwin-framework-tool does not currently support reading or subscribing to Events
@@ -233,7 +233,7 @@ def _GetDarwinFrameworkToolUnsupportedTests() -> Set[str]:
     }
 
 
-def _GetReplUnsupportedTests() -> Set[str]:
+def _GetReplUnsupportedTests() -> set[str]:
     """Tests that fail in matter-repl for some reason"""
     return {
         "Test_AddNewFabricFromExistingFabric.yaml",     # matter-repl does not support GetCommissionerRootCertificate and IssueNocChain command
@@ -255,7 +255,7 @@ def _GetReplUnsupportedTests() -> Set[str]:
     }
 
 
-def _GetPurposefulFailureTests() -> Set[str]:
+def _GetPurposefulFailureTests() -> set[str]:
     """Tests that fail in YAML on purpose."""
     return {
         "TestPurposefulFailureEqualities.yaml",
@@ -264,7 +264,7 @@ def _GetPurposefulFailureTests() -> Set[str]:
     }
 
 
-def _AllYamlTests():
+def _AllYamlTests() -> Iterable[Path]:
     yaml_test_suite_path = Path(_YAML_TEST_SUITE_PATH)
 
     if not yaml_test_suite_path.exists():
@@ -298,7 +298,8 @@ def _TargetsForYaml(yaml_path: Path) -> list[TestTarget]:
     return targets
 
 
-def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft_unsupported_as_in_development: bool, treat_chip_tool_unsupported_as_in_development: bool, use_short_run_name: bool):
+def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft_unsupported_as_in_development: bool,
+                       treat_chip_tool_unsupported_as_in_development: bool, use_short_run_name: bool) -> Iterable[TestDefinition]:
     """
     use_short_run_name should be true if we want the run_name to be "Test_ABC" instead of "some/path/Test_ABC.yaml"
     """
@@ -357,16 +358,16 @@ def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, treat_dft
         )
 
 
-def AllReplYamlTests():
+def AllReplYamlTests() -> Iterable[TestDefinition]:
     for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=True, treat_dft_unsupported_as_in_development=False, treat_chip_tool_unsupported_as_in_development=False, use_short_run_name=False):
         yield test
 
 
-def AllChipToolYamlTests(use_short_run_name: bool = True):
+def AllChipToolYamlTests(use_short_run_name: bool = True) -> Iterable[TestDefinition]:
     for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=False, treat_chip_tool_unsupported_as_in_development=True, use_short_run_name=use_short_run_name):
         yield test
 
 
-def AllDarwinFrameworkToolYamlTests():
+def AllDarwinFrameworkToolYamlTests() -> Iterable[TestDefinition]:
     for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, treat_dft_unsupported_as_in_development=True, treat_chip_tool_unsupported_as_in_development=False, use_short_run_name=True):
         yield test

scripts/tests/chiptest/darwin.py

@@ -12,13 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import subprocess
 from typing import IO, Any
 
 from .runner import Executor, LogPipe, SubprocessInfo
 
 
 class DarwinExecutor(Executor):
-    def run(self, subproc: SubprocessInfo, stdin: IO[Any] | None = None, stdout: IO[Any] | LogPipe | None = None, stderr: IO[Any] | LogPipe | None = None):
+    def run(self, subproc: SubprocessInfo, stdin: IO[Any] | None = None, stdout: IO[Any] | LogPipe | None = None,
+            stderr: IO[Any] | LogPipe | None = None) -> subprocess.Popen[bytes]:
         # Try harder to avoid any stdout buffering in our tests
         wrapped = subproc.wrap_with('stdbuf', '-o0', '-i0')
         return super().run(wrapped, stdin, stdout, stderr)

scripts/tests/chiptest/linux.py

@@ -20,6 +20,7 @@
 
 import logging
 import os
+import subprocess
 from typing import IO, Any
 
 from chiptest.runner import Executor, LogPipe, SubprocessInfo
@@ -53,7 +54,7 @@ def __init__(self, ns: IsolatedNetworkNamespace):
         self.ns = ns
 
     def run(self, subproc: SubprocessInfo, stdin: IO[Any] | None = None, stdout: IO[Any] | LogPipe | None = None,
-            stderr: IO[Any] | LogPipe | None = None):
+            stderr: IO[Any] | LogPipe | None = None) -> subprocess.Popen[bytes]:
         try:
             subprocess_ns = self.ns.netns_for_subprocess_kind(subproc.kind)
             wrapped = subproc.wrap_with(*subprocess_ns.netns_cmd_wrapper)

scripts/tests/chiptest/runner.py

@@ -23,7 +23,7 @@
 import subprocess
 import threading
 from contextlib import suppress
-from typing import IO, TYPE_CHECKING, Any, Protocol
+from typing import IO, TYPE_CHECKING, Any, Match, Protocol
 
 import python_path
 
@@ -68,18 +68,18 @@ def CapturedLogContains(self, txt: str, index: int = 0) -> tuple[bool, int]:
                 return True, index + i
         return False, len(self.captured_logs)
 
-    def FindLastMatchingLine(self, matcher: str):
+    def FindLastMatchingLine(self, matcher: str) -> Match[str] | None:
         for line in reversed(self.captured_logs):
             match = re.match(matcher, line)
             if match:
                 return match
         return None
 
-    def fileno(self):
+    def fileno(self) -> int:
         """Return the write file descriptor of the pipe."""
         return self.fd_write
 
-    def run(self):
+    def run(self) -> None:
         """Run the thread, logging everything."""
         while True:
             try:
@@ -96,7 +96,7 @@ def run(self):
                 self.capture_delegate.Log(self.name, line)
         self.reader.close()
 
-    def close(self):
+    def close(self) -> None:
         """Close the write end of the pipe."""
         os.close(self.fd_write)
 
@@ -115,7 +115,7 @@ def __init__(self, timeout_seconds: int | None):
         self.timeout_seconds = timeout_seconds
         self.timed_out = False
 
-    def __wait(self, process: Process, userdata: AppsRegister | None):
+    def __wait(self, process: Process, userdata: AppsRegister | None) -> None:
         if userdata is None:
             # We're the main process for this wait queue.
             timeout = self.timeout_seconds
@@ -130,12 +130,12 @@ def __wait(self, process: Process, userdata: AppsRegister | None):
             process.wait()
         self.queue.put((process, userdata))
 
-    def add_process(self, process: Process, userdata: AppsRegister | None = None):
+    def add_process(self, process: Process, userdata: AppsRegister | None = None) -> None:
         t = threading.Thread(target=self.__wait, args=(process, userdata))
         t.daemon = True
         t.start()
 
-    def get(self):
+    def get(self) -> tuple[Process, AppsRegister | None]:
         return self.queue.get()
 
 
@@ -145,7 +145,8 @@ class Executor:
     def __init__(self) -> None:
         self._processes: queue.Queue[subprocess.Popen[bytes]] = queue.Queue()
 
-    def run(self, subproc: SubprocessInfo, stdin: IO[Any] | None = None, stdout: IO[Any] | LogPipe | None = None, stderr: IO[Any] | LogPipe | None = None):
+    def run(self, subproc: SubprocessInfo, stdin: IO[Any] | None = None, stdout: IO[Any] | LogPipe | None = None,
+            stderr: IO[Any] | LogPipe | None = None) -> subprocess.Popen[bytes]:
         # Seems like LogPipe has all what Popen needs to perceive it as stdout/stderr,
         # but mypy doesn't think the same.
         self._processes.put(process := subprocess.Popen(subproc.to_cmd(), stdin=stdin,