chore: strict type annotations of test directories

Author: regisb

tests/extensions/test_base_extensions.py

@@ -10,16 +10,16 @@
 
 
 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.
     """
@@ -30,7 +30,7 @@ def test_nothing_found():
         r.search(f)
 
 
-def test_strip_single_line_comment_tokens():
+def test_strip_single_line_comment_tokens() -> None:
     config = FakeConfig()
 
     extension = FakeExtension(config, VerboseEcho())

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

@@ -21,7 +21,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 +86,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,19 @@
 """
 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 stevedore import named
 
-from code_annotations.base import BaseSearch, VerboseEcho
+from code_annotations.base import AnnotationConfig, 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
@@ -34,30 +42,41 @@
 """.format(DEFAULT_FAKE_SAFELIST_PATH)
 
 
-class FakeConfig:
+class FakeConfig(AnnotationConfig):
     """
     Simple config for testing without reading a config file.
     """
 
-    annotations: dict[str, str] = {}
-    annotation_regexes: list[str] = []
-    annotation_tokens: list[str] = []
-    groups: list[str] = []
-    echo = VerboseEcho()
+    def __init__(self) -> None:  # pylint: disable=super-init-not-called
+        """
+        Override the base __init__ to skip reading and parsing the config.
+        """
+        self.groups: dict[str, list[str]] = {}
+        self.choices: dict[str, list[str]] = {}
+        self.optional_groups: list[str] = []
+        self.annotation_tokens: list[str] = []
+        self.annotation_regexes: list[str] = []
+        self.mgr: named.NamedExtensionManager | None = None
+        self.coverage_target: float | None = None
+        self.echo = VerboseEcho()
 
 
 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 +95,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 +127,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.
 
@@ -123,8 +142,6 @@ def call_script_isolated(
         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 +168,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 +182,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 +191,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,21 +1,23 @@
 """
 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)
     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'],
@@ -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
     """
@@ -86,7 +88,7 @@ def test_format_results_for_report():
     search = FakeSearch(config)
 
     # 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 = 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: 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(),

tests/test_django_generate_safelist.py

@@ -3,6 +3,7 @@
 Tests for seeding the safelist.
 """
 import os
+import typing as t
 from unittest.mock import DEFAULT, MagicMock, patch
 
 import pytest
@@ -34,19 +35,23 @@
         [],  # No non-local models to add to the safelist.
     ),
 ])
-def test_seeding_safelist(local_models, non_local_models, **kwargs):
+def test_seeding_safelist(
+    local_models: list[MagicMock],
+    non_local_models: list[MagicMock],
+    **kwargs: t.Any
+) -> None:
     """
     Test the success case for seeding the safelist.
     """
-    mock_get_models_requiring_annotations = kwargs['get_models_requiring_annotations']
+    mock_get_models_requiring_annotations: MagicMock = kwargs['get_models_requiring_annotations']
     mock_get_models_requiring_annotations.return_value = (
         local_models,
         non_local_models,
         0,  # Number of total models found, irrelevant here
         set()  # List of model ids that are eligible for annotation, irrelevant here
     )
 
-    def test_safelist_callback():
+    def test_safelist_callback() -> None:
         assert os.path.exists(DEFAULT_FAKE_SAFELIST_PATH)
         with open(DEFAULT_FAKE_SAFELIST_PATH) as fake_safelist_file:
             fake_safelist = fake_safelist_file.read()
@@ -58,7 +63,7 @@ def test_safelist_callback():
     result = call_script_isolated(
         ['django_find_annotations', '--config_file', 'test_config.yml', '--seed_safelist'],
         test_filesystem_cb=test_safelist_callback,
-        fake_safelist_data=None
+        fake_safelist_data=""
     )
     assert result.exit_code == EXIT_CODE_SUCCESS
     assert 'Successfully created safelist file' in result.output
@@ -68,11 +73,11 @@ def test_safelist_callback():
     'code_annotations.find_django.DjangoSearch',
     get_models_requiring_annotations=DEFAULT,
 )
-def test_safelist_exists(**kwargs):
+def test_safelist_exists(**kwargs: t.Any) -> None:
     """
     Test the success case for seeding the safelist.
     """
-    mock_get_models_requiring_annotations = kwargs['get_models_requiring_annotations']
+    mock_get_models_requiring_annotations: MagicMock = kwargs['get_models_requiring_annotations']
     mock_get_models_requiring_annotations.return_value = (set(), set(), 0, [])
 
     result = call_script_isolated(