chore: strict type annotations of test directories

Author: regisb

tests/extensions/test_base_extensions.py

@@ -10,30 +10,30 @@
 
 
 class FakeExtension(SimpleRegexAnnotationExtension):
-    extension_name = 'fake_extension'
+    extension_name: str = 'fake_extension'
 
-    lang_comment_definition = {
+    lang_comment_definition: dict[str, str] = {
         'multi_start': re.escape('foo'),
         'multi_end': re.escape('bar'),
         'single': re.escape('baz')
     }
 
 
-def test_nothing_found():
+def test_nothing_found() -> None:
     """
     Make sure nothing fails when no annotation is found.
     """
     config = FakeConfig()
 
-    r = FakeExtension(config, VerboseEcho())
+    r = FakeExtension(config, VerboseEcho())  # type: ignore
     with open('tests/extensions/base_test_files/empty.foo') as f:
         r.search(f)
 
 
-def test_strip_single_line_comment_tokens():
+def test_strip_single_line_comment_tokens() -> None:
     config = FakeConfig()
 
-    extension = FakeExtension(config, VerboseEcho())
+    extension = FakeExtension(config, VerboseEcho())  # type: ignore
     text = """baz line1
   baz line2
 bazline3

tests/extensions/test_extension_javascript.py

@@ -1,6 +1,7 @@
 """
 Tests for the Javascript static extension
 """
+
 import pytest
 
 from tests.helpers import EXIT_CODE_FAILURE, EXIT_CODE_SUCCESS, call_script
@@ -20,7 +21,7 @@
     ('choice_failures_4.js', EXIT_CODE_FAILURE, '"terrible" is already present in this annotation'),
     ('choice_failures_5.js', EXIT_CODE_FAILURE, 'no value found for ".. ignored:"'),
 ])
-def test_grouping_and_choice_failures(test_file, expected_exit_code, expected_message):
+def test_grouping_and_choice_failures(test_file: str, expected_exit_code: int, expected_message: str) -> None:
     result = call_script((
         'static_find_annotations',
         '--config_file',

tests/extensions/test_extension_python.py

@@ -1,6 +1,8 @@
 """
 Tests for the Python static extension
 """
+from typing import List, Tuple
+
 import pytest
 
 from code_annotations.base import AnnotationConfig
@@ -21,7 +23,7 @@
     ('choice_failures_4.pyt', EXIT_CODE_FAILURE, '"terrible" is already present in this annotation'),
     ('choice_failures_5.pyt', EXIT_CODE_FAILURE, 'no value found for ".. ignored:"'),
 ])
-def test_grouping_and_choice_failures(test_file, expected_exit_code, expected_message):
+def test_grouping_and_choice_failures(test_file: str, expected_exit_code: int, expected_message: str) -> None:
     result = call_script((
         'static_find_annotations',
         '--config_file',
@@ -86,7 +88,7 @@ def test_grouping_and_choice_failures(test_file, expected_exit_code, expected_me
         ]
     ),
 ])
-def test_multi_line_annotations(test_file, annotations):
+def test_multi_line_annotations(test_file: str, annotations: List[Tuple[str, str]]) -> None:
     config = AnnotationConfig('tests/test_configurations/.annotations_test')
     annotator = PythonAnnotationExtension(config, VerboseEcho())
 

tests/helpers.py

@@ -3,11 +3,18 @@
 """
 import os
 import re
+import typing as t
+from collections.abc import Callable, Sequence
 
 from click.testing import CliRunner
+from click.testing import Result as ClickTestResult
 
-from code_annotations.base import BaseSearch, VerboseEcho
+from code_annotations.base import BaseSearch
 from code_annotations.cli import entry_point
+from code_annotations.helpers import VerboseEcho
+
+# Re-export Result for convenience
+Result = ClickTestResult
 
 EXIT_CODE_SUCCESS = 0
 EXIT_CODE_FAILURE = 1
@@ -39,10 +46,10 @@ class FakeConfig:
     Simple config for testing without reading a config file.
     """
 
-    annotations: dict[str, str] = {}
+    annotations: dict[str, t.Any] = {}
     annotation_regexes: list[str] = []
     annotation_tokens: list[str] = []
-    groups: list[str] = []
+    groups: t.Union[list[str], dict[str, list[str]]] = []
     echo = VerboseEcho()
 
 
@@ -51,13 +58,17 @@ class FakeSearch(BaseSearch):
     Simple test class for directly testing BaseSearch since it's abstract.
     """
 
-    def search(self):
+    def search(self) -> dict[str, list[dict[str, t.Any]]]:
         """
         Override for abstract base method.
+
+        Returns:
+            Empty dict to satisfy the abstract method requirement
         """
+        return {}
 
 
-def delete_report_files(file_extension):
+def delete_report_files(file_extension: str) -> None:
     """
     Delete all files with the given extension from the test_reports directory.
 
@@ -76,7 +87,7 @@ def delete_report_files(file_extension):
         pass
 
 
-def call_script(args_list, delete_test_reports=True, delete_test_docs=True):
+def call_script(args_list: Sequence[str], delete_test_reports: bool = True, delete_test_docs: bool = True) -> Result:
     """
     Call the code_annotations script with the given params and a generic config file.
 
@@ -108,11 +119,11 @@ def call_script(args_list, delete_test_reports=True, delete_test_docs=True):
 
 
 def call_script_isolated(
-        args_list,
-        test_filesystem_cb=None,
-        test_filesystem_report_cb=None,
-        fake_safelist_data="{}"
-):
+        args_list: list[str],
+        test_filesystem_cb: Callable[[], None] | None = None,
+        test_filesystem_report_cb: Callable[[str], None] | None = None,
+        fake_safelist_data: str = "{}"
+) -> Result:
     """
     Call the code_annotations script with the given params and a generic config file.
 
@@ -122,9 +133,6 @@ def call_script_isolated(
             cleared. Use this if you need access to non-report files in the temp filesystem.
         test_filesystem_report_cb: Callback function, called after the command is run, before the temp filesystem
             is cleared. Callback is called with the raw text contents of the report file.
-        fake_safelist_data: Raw text to write to the safelist file before the command is called.
-        safelist_path: File path to write the safelist to. Used when writing a fake safelist, but not automatically
-            passed to the command.
 
     Returns:
         click.testing.Result: Result from the `CliRunner.invoke()` call.
@@ -151,7 +159,9 @@ def call_script_isolated(
 
         if test_filesystem_report_cb:
             try:
-                report_file = re.search(r'Generating report to (.*)', result.output).groups()[0]
+                report_match = re.search(r'Generating report to (.*)', result.output)
+                assert report_match is not None
+                report_file = report_match.groups()[0]
                 with open(report_file) as f:
                     report_contents = f.read()
 
@@ -163,7 +173,7 @@ def call_script_isolated(
     return result
 
 
-def get_report_filename_from_output(output):
+def get_report_filename_from_output(output: str) -> str | None:
     """
     Find the report filename in a find_static or find_django output and return it.
 
@@ -172,9 +182,10 @@ def get_report_filename_from_output(output):
 
     Returns:
         Filename of the found report, or None of no name is found
-
     """
+    match = re.search(r'Generating report to (.*)', output)
+    assert match is not None
     try:
-        return re.search(r'Generating report to (.*)', output).groups()[0]
+        return match.groups()[0]
     except IndexError:
         return None

tests/test_base.py

@@ -1,27 +1,29 @@
 """
 Tests for code_annotations/base.py
 """
+import typing as t
 from collections import OrderedDict
 
 import pytest
 
-from code_annotations.base import AnnotationConfig, ConfigurationException
+from code_annotations.base import AnnotationConfig
+from code_annotations.exceptions import ConfigurationException
 from tests.helpers import FakeConfig, FakeSearch
 
 
-def test_get_group_for_token_missing_token():
+def test_get_group_for_token_missing_token() -> None:
     config = FakeConfig()
-    search = FakeSearch(config)
+    search = FakeSearch(config)  # type: ignore
     assert search._get_group_for_token('foo') is None  # pylint: disable=protected-access
 
 
-def test_get_group_for_token_multiple_groups():
+def test_get_group_for_token_multiple_groups() -> None:
     config = FakeConfig()
     config.groups = {
         'group1': ['token1'],
         'group2': ['token2', 'foo']
     }
-    search = FakeSearch(config)
+    search = FakeSearch(config)  # type: ignore
     assert search._get_group_for_token('foo') == 'group2'  # pylint: disable=protected-access
 
 
@@ -30,7 +32,7 @@ def test_get_group_for_token_multiple_groups():
     ('.annotations_test_missing_report_path', "report_path"),
     ('.annotations_test_missing_safelist_path', "safelist_path"),
 ])
-def test_missing_config(test_config, expected_message):
+def test_missing_config(test_config: str, expected_message: str) -> None:
     with pytest.raises(ConfigurationException) as exception:
         AnnotationConfig(f'tests/test_configurations/{test_config}', None, 3)
 
@@ -44,15 +46,15 @@ def test_missing_config(test_config, expected_message):
     ('.annotations_test_coverage_over_100', "Invalid coverage target. 150.0 is not between 0 and 100."),
     ('.annotations_test_coverage_nan', 'Coverage target must be a number between 0 and 100 not "not a number".'),
 ])
-def test_bad_coverage_targets(test_config, expected_message):
+def test_bad_coverage_targets(test_config: str, expected_message: str) -> None:
     with pytest.raises(ConfigurationException) as exception:
         AnnotationConfig(f'tests/test_configurations/{test_config}', None, 3)
 
     exc_msg = str(exception.value)
     assert expected_message in exc_msg
 
 
-def test_coverage_target_int():
+def test_coverage_target_int() -> None:
     # We just care that this doesn't throw an exception
     AnnotationConfig('tests/test_configurations/{}'.format('.annotations_test_coverage_int'), None, 3)
 
@@ -64,15 +66,15 @@ def test_coverage_target_int():
     ('.annotations_test_group_one_token', 'Group "pii_group" must have more than one annotation.'),
     ('.annotations_test_group_bad_type', "{'.. pii:': ['bad', 'type']} is an unknown annotation type."),
 ])
-def test_annotation_configuration_errors(test_config, expected_message):
+def test_annotation_configuration_errors(test_config: str, expected_message: str) -> None:
     with pytest.raises(ConfigurationException) as exception:
         AnnotationConfig(f'tests/test_configurations/{test_config}', None, 3)
 
     exc_msg = str(exception.value)
     assert expected_message in exc_msg
 
 
-def test_format_results_for_report():
+def test_format_results_for_report() -> None:
     """
     Test that report formatting puts annotations into groups correctly
     """
@@ -83,10 +85,10 @@ def test_format_results_for_report():
         'group2': ['token2', 'foo']
     }
 
-    search = FakeSearch(config)
+    search = FakeSearch(config)  # type: ignore
 
     # Create a fake result set for _format_results_for_report to work on
-    fake_results = OrderedDict()
+    fake_results: OrderedDict[str, list[dict[str, t.Any]]] = OrderedDict()
 
     # First file has 6 annotations. expected_group_id is a special key for this test, allowing us to loop through
     # these below and know what group each result should be in.

tests/test_django_coverage.py

@@ -1,7 +1,8 @@
 """
 Tests for the DjangoSearch coverage functionality.
 """
-from unittest.mock import DEFAULT, patch
+import typing as t
+from unittest.mock import DEFAULT, MagicMock, patch
 
 import pytest
 
@@ -21,7 +22,10 @@
 )
 from tests.helpers import EXIT_CODE_FAILURE, EXIT_CODE_SUCCESS, call_script_isolated
 
-ALL_FAKE_MODELS = (
+# Type for our fake model classes
+FakeModelClass = t.Type[t.Any]
+
+ALL_FAKE_MODELS: tuple[FakeModelClass, ...] = (
     FakeBaseModelAbstract,
     FakeBaseModelBoring,
     FakeBaseModelBoringWithAnnotations,
@@ -40,10 +44,15 @@
 @patch('code_annotations.find_django.DjangoSearch.setup_django')
 @patch('code_annotations.find_django.DjangoSearch.is_non_local')
 @patch('code_annotations.find_django.django.apps.apps.get_app_configs')
-def test_coverage_all_models(mock_get_app_configs, mock_is_non_local, mock_setup_django, mock_issubclass):
+def test_coverage_all_models(
+    mock_get_app_configs: MagicMock,
+    mock_is_non_local: MagicMock,
+    mock_setup_django: MagicMock,
+    mock_issubclass: MagicMock
+) -> None:
     # Lots of fakery going on here. This class mocks Django AppConfigs to deliver our fake models.
     class FakeAppConfig:
-        def get_models(self):
+        def get_models(self) -> tuple[FakeModelClass, ...]:
             return ALL_FAKE_MODELS
 
     # This lets us deterministically decide that one model is local, and the other isn't, for testing both branches.
@@ -107,8 +116,13 @@ def get_models(self):
         "Coverage is 100.0%"
     ),
 ])
-def test_coverage_thresholds(local_models, should_succeed, expected_message, **kwargs):
-    mock_get_models_requiring_annotations = kwargs['get_models_requiring_annotations']
+def test_coverage_thresholds(
+    local_models: t.List[FakeModelClass],
+    should_succeed: bool,
+    expected_message: str,
+    **kwargs: t.Any
+) -> None:
+    mock_get_models_requiring_annotations: MagicMock = kwargs['get_models_requiring_annotations']
     mock_get_models_requiring_annotations.return_value = (
         set(local_models),
         set(),