From 03a041b30f947a60645e6ca54093a4c6691a8643 Mon Sep 17 00:00:00 2001 From: Nathanael Mortensen Date: Wed, 21 Jan 2026 08:54:54 -0800 Subject: [PATCH] Validate typing in tests While somewhat painful, type checking our tests should help ensure that what we build is idiomatic. This validates that the typing information we provide on our public functions is correct and usable. Change mypy to be run directly rather than as a tool. Running it as a tool executes it independently of the version that we configure and made it not correctly resolve pytest. The uv documentation lists mypy as an example to use "uv run" rather than "uv tool run". As part of enabling this, disable type checking for specific lines that need additional work in the library to correct the type checking. This is particularly around Workflow and Activity Definitions. --- Makefile | 2 +- cadence/data_converter.py | 12 ++++++--- pyproject.toml | 2 +- tests/cadence/_internal/rpc/test_error.py | 6 ++--- tests/cadence/_internal/rpc/test_retry.py | 4 +-- .../workflow/test_deterministic_event_loop.py | 16 +++++++----- tests/cadence/common_activities.py | 5 ++-- tests/cadence/test_client_workflow.py | 12 ++++----- .../cadence/worker/test_base_task_handler.py | 5 ---- .../worker/test_decision_task_handler.py | 5 ---- .../test_decision_task_handler_integration.py | 9 ++----- .../test_decision_worker_integration.py | 26 ++++++++----------- tests/cadence/worker/test_poller.py | 16 ++++++------ tests/cadence/worker/test_registry.py | 6 +---- .../worker/test_task_handler_integration.py | 5 ---- tests/integration_tests/helper.py | 4 +-- .../workflow/test_activities.py | 1 + .../workflow/test_workflows.py | 1 + 18 files changed, 60 insertions(+), 77 deletions(-) diff --git a/Makefile b/Makefile index e5af680..824ab36 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ lint: # Run type checker type-check: @echo "Running mypy type checker..." - uv tool run mypy cadence/ + uv run mypy cadence/ tests/ # Run unit tests test: diff --git a/cadence/data_converter.py b/cadence/data_converter.py index cae382d..83c0948 100644 --- a/cadence/data_converter.py +++ b/cadence/data_converter.py @@ -1,5 +1,5 @@ from abc import abstractmethod -from typing import Protocol, List, Type, Any +from typing import Protocol, List, Type, Any, Sequence from cadence.api.v1.common_pb2 import Payload from json import JSONDecoder @@ -24,7 +24,9 @@ def __init__(self) -> None: # Need to use std lib decoder in order to decode the custom whitespace delimited data format self._decoder = JSONDecoder(strict=False) - def from_data(self, payload: Payload, type_hints: List[Type | None]) -> List[Any]: + def from_data( + self, payload: Payload, type_hints: Sequence[Type | None] + ) -> List[Any]: if not payload.data: return DefaultDataConverter._convert_into([], type_hints) @@ -33,7 +35,7 @@ def from_data(self, payload: Payload, type_hints: List[Type | None]) -> List[Any return self._decode_whitespace_delimited(payload_str, type_hints) def _decode_whitespace_delimited( - self, payload: str, type_hints: List[Type | None] + self, payload: str, type_hints: Sequence[Type | None] ) -> List[Any]: results: List[Any] = [] start, end = 0, len(payload) @@ -46,7 +48,9 @@ def _decode_whitespace_delimited( return DefaultDataConverter._convert_into(results, type_hints) @staticmethod - def _convert_into(values: List[Any], type_hints: List[Type | None]) -> List[Any]: + def _convert_into( + values: List[Any], type_hints: Sequence[Type | None] + ) -> List[Any]: results: List[Any] = [] for i, type_hint in enumerate(type_hints): if not type_hint: diff --git a/pyproject.toml b/pyproject.toml index 75d7764..fa42b44 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,7 +80,7 @@ check_untyped_defs = true disallow_untyped_decorators = false # Temporarily disable to allow CI to pass no_implicit_optional = true warn_redundant_casts = true -warn_unused_ignores = true +warn_unused_ignores = false warn_no_return = true warn_unreachable = true strict_equality = true diff --git a/tests/cadence/_internal/rpc/test_error.py b/tests/cadence/_internal/rpc/test_error.py index d3c30b9..c4a2fcd 100644 --- a/tests/cadence/_internal/rpc/test_error.py +++ b/tests/cadence/_internal/rpc/test_error.py @@ -2,10 +2,10 @@ import pytest from google.protobuf import any_pb2 -from google.rpc import code_pb2, status_pb2 +from google.rpc import code_pb2, status_pb2 # type: ignore from grpc import Status, StatusCode, server from grpc.aio import insecure_channel -from grpc_status.rpc_status import to_status +from grpc_status.rpc_status import to_status # type: ignore from cadence._internal.rpc.error import CadenceErrorInterceptor from cadence.api.v1 import error_pb2, service_meta_pb2_grpc @@ -168,7 +168,7 @@ def fake_service(): ) @pytest.mark.asyncio async def test_map_error( - fake_service, err: Message | Status, expected: CadenceRpcError + fake_service, err: Message | Status, expected: CadenceRpcError | None ): async with insecure_channel( f"[::]:{fake_service.port}", interceptors=[CadenceErrorInterceptor()] diff --git a/tests/cadence/_internal/rpc/test_retry.py b/tests/cadence/_internal/rpc/test_retry.py index 64249fd..9962c64 100644 --- a/tests/cadence/_internal/rpc/test_retry.py +++ b/tests/cadence/_internal/rpc/test_retry.py @@ -3,10 +3,10 @@ import pytest from google.protobuf import any_pb2 -from google.rpc import status_pb2, code_pb2 +from google.rpc import status_pb2, code_pb2 # type: ignore from grpc import server from grpc.aio import insecure_channel -from grpc_status.rpc_status import to_status +from grpc_status.rpc_status import to_status # type: ignore from cadence._internal.rpc.error import CadenceErrorInterceptor from cadence.api.v1 import error_pb2, service_workflow_pb2_grpc diff --git a/tests/cadence/_internal/workflow/test_deterministic_event_loop.py b/tests/cadence/_internal/workflow/test_deterministic_event_loop.py index bbca5b0..5d611ab 100644 --- a/tests/cadence/_internal/workflow/test_deterministic_event_loop.py +++ b/tests/cadence/_internal/workflow/test_deterministic_event_loop.py @@ -3,12 +3,12 @@ from cadence._internal.workflow.deterministic_event_loop import DeterministicEventLoop -async def coro_append(results: list, i: int): +async def coro_append(results: list[int], i: int): results.append(i) async def coro_await(size: int): - results = [] + results: list[int] = [] for i in range(size): await coro_append(results, i) return results @@ -19,7 +19,7 @@ async def coro_await_future(future: asyncio.Future): async def coro_await_task(size: int): - results = [] + results: list[int] = [] for i in range(size): asyncio.create_task(coro_append(results, i)) return results @@ -40,11 +40,15 @@ def teardown_method(self): def test_call_soon(self): """Test _run_once executes single callback.""" - results = [] - expected = [] + results: list[int] = [] + expected: list[int] = [] for i in range(10000): expected.append(i) - self.loop.call_soon(lambda x=i: results.append(x)) + + def add_to_result(result: int): + results.append(result) + + self.loop.call_soon(add_to_result, i) self.loop._run_once() diff --git a/tests/cadence/common_activities.py b/tests/cadence/common_activities.py index b6904b4..1b7daaf 100644 --- a/tests/cadence/common_activities.py +++ b/tests/cadence/common_activities.py @@ -1,3 +1,4 @@ +from typing import Protocol from dataclasses import dataclass from cadence import activity @@ -43,7 +44,7 @@ async def echo_async(self, incoming: str) -> str: return incoming -class ActivityInterface: +class ActivityInterface(Protocol): @activity.defn() def do_something(self) -> str: ... @@ -52,7 +53,7 @@ def do_something(self) -> str: ... class ActivityImpl(ActivityInterface): result: str - def do_something(self) -> str: + def do_something(self) -> str: # type: ignore return self.result diff --git a/tests/cadence/test_client_workflow.py b/tests/cadence/test_client_workflow.py index acb1a98..3a6e7f1 100644 --- a/tests/cadence/test_client_workflow.py +++ b/tests/cadence/test_client_workflow.py @@ -267,7 +267,7 @@ def mock_build_request(workflow, args, options): request.domain = "test-domain" return request - client._build_start_workflow_request = Mock(side_effect=mock_build_request) + client._build_start_workflow_request = Mock(side_effect=mock_build_request) # type: ignore execution = await client.start_workflow( "TestWorkflow", @@ -298,7 +298,7 @@ async def test_start_workflow_grpc_error(self, mock_client): client._workflow_stub = mock_client.workflow_stub # Mock the internal method - client._build_start_workflow_request = Mock( + client._build_start_workflow_request = Mock( # type: ignore return_value=StartWorkflowExecutionRequest() ) @@ -324,7 +324,7 @@ async def test_start_workflow_with_kwargs(self, mock_client): client._workflow_stub = mock_client.workflow_stub # Mock the internal method to capture options - captured_options = None + captured_options: StartWorkflowOptions = StartWorkflowOptions() def mock_build_request(workflow, args, options): nonlocal captured_options @@ -333,7 +333,7 @@ def mock_build_request(workflow, args, options): request.workflow_id = "test-workflow-id" return request - client._build_start_workflow_request = Mock(side_effect=mock_build_request) + client._build_start_workflow_request = Mock(side_effect=mock_build_request) # type: ignore await client.start_workflow( "TestWorkflow", @@ -365,7 +365,7 @@ async def test_start_workflow_with_default_task_timeout(self, mock_client): client._workflow_stub = mock_client.workflow_stub # Mock the internal method to capture options - captured_options = None + captured_options: StartWorkflowOptions = StartWorkflowOptions() def mock_build_request(workflow, args, options): nonlocal captured_options @@ -374,7 +374,7 @@ def mock_build_request(workflow, args, options): request.workflow_id = "test-workflow-id" return request - client._build_start_workflow_request = Mock(side_effect=mock_build_request) + client._build_start_workflow_request = Mock(side_effect=mock_build_request) # type: ignore await client.start_workflow( "TestWorkflow", diff --git a/tests/cadence/worker/test_base_task_handler.py b/tests/cadence/worker/test_base_task_handler.py index 8196240..1d3f079 100644 --- a/tests/cadence/worker/test_base_task_handler.py +++ b/tests/cadence/worker/test_base_task_handler.py @@ -1,8 +1,3 @@ -#!/usr/bin/env python3 -""" -Unit tests for BaseTaskHandler class. -""" - import pytest from unittest.mock import Mock diff --git a/tests/cadence/worker/test_decision_task_handler.py b/tests/cadence/worker/test_decision_task_handler.py index 45e6541..78a8fe8 100644 --- a/tests/cadence/worker/test_decision_task_handler.py +++ b/tests/cadence/worker/test_decision_task_handler.py @@ -1,8 +1,3 @@ -#!/usr/bin/env python3 -""" -Unit tests for DecisionTaskHandler class. -""" - import pytest from unittest.mock import Mock, AsyncMock, patch, PropertyMock diff --git a/tests/cadence/worker/test_decision_task_handler_integration.py b/tests/cadence/worker/test_decision_task_handler_integration.py index 71217b1..3243bae 100644 --- a/tests/cadence/worker/test_decision_task_handler_integration.py +++ b/tests/cadence/worker/test_decision_task_handler_integration.py @@ -1,8 +1,3 @@ -#!/usr/bin/env python3 -""" -Integration tests for DecisionTaskHandler and WorkflowEngine. -""" - import pytest from unittest.mock import Mock, AsyncMock, patch from cadence.api.v1.service_worker_pb2 import PollForDecisionTaskResponse @@ -39,8 +34,8 @@ def registry(self): """Create a registry with a test workflow.""" reg = Registry() - @reg.workflow(name="test_workflow") - class TestWorkflow: + @reg.workflow(name="test_workflow") # type: ignore + class TestWorkflow: # type: ignore @workflow.run async def run(self, input_data): """Simple test workflow that returns the input.""" diff --git a/tests/cadence/worker/test_decision_worker_integration.py b/tests/cadence/worker/test_decision_worker_integration.py index 91368b4..311ed0a 100644 --- a/tests/cadence/worker/test_decision_worker_integration.py +++ b/tests/cadence/worker/test_decision_worker_integration.py @@ -1,8 +1,3 @@ -#!/usr/bin/env python3 -""" -Integration tests for DecisionWorker with DecisionTaskHandler. -""" - import asyncio import pytest from unittest.mock import Mock, AsyncMock, patch @@ -13,6 +8,7 @@ HistoryEvent, WorkflowExecutionStartedEventAttributes, ) +from cadence.worker import WorkerOptions from cadence.worker._decision import DecisionWorker from cadence.worker._registry import Registry from cadence import workflow @@ -52,11 +48,11 @@ async def run(self, input_data): @pytest.fixture def decision_worker(self, mock_client, registry): """Create a DecisionWorker instance.""" - options = { - "identity": "test-worker", - "max_concurrent_decision_task_execution_size": 1, - "decision_task_pollers": 1, - } + options = WorkerOptions( + identity="test-worker", + max_concurrent_decision_task_execution_size=1, + decision_task_pollers=1, + ) return DecisionWorker( client=mock_client, task_list="test-task-list", @@ -297,11 +293,11 @@ async def test_decision_worker_poll_timeout(self, decision_worker, mock_client): def test_decision_worker_options_handling(self, mock_client, registry): """Test DecisionWorker with various options.""" - options = { - "identity": "custom-worker", - "max_concurrent_decision_task_execution_size": 5, - "decision_task_pollers": 3, - } + options = WorkerOptions( + identity="custom-worker", + max_concurrent_decision_task_execution_size=5, + decision_task_pollers=3, + ) worker = DecisionWorker( client=mock_client, diff --git a/tests/cadence/worker/test_poller.py b/tests/cadence/worker/test_poller.py index ee891a4..1281a79 100644 --- a/tests/cadence/worker/test_poller.py +++ b/tests/cadence/worker/test_poller.py @@ -8,8 +8,8 @@ @pytest.mark.asyncio async def test_poller(): permits = asyncio.Semaphore(1) - incoming = asyncio.Queue() - outgoing = asyncio.Queue() + incoming = asyncio.Queue[str]() + outgoing = asyncio.Queue[str]() poller = Poller(1, permits, incoming.get, outgoing.put) task = asyncio.create_task(poller.run()) @@ -25,8 +25,8 @@ async def test_poller(): @pytest.mark.asyncio async def test_poller_empty_task(): permits = asyncio.Semaphore(1) - incoming = asyncio.Queue() - outgoing = asyncio.Queue() + incoming = asyncio.Queue[str | None]() + outgoing = asyncio.Queue[str]() poller = Poller(1, permits, incoming.get, outgoing.put) task = asyncio.create_task(poller.run()) @@ -56,7 +56,7 @@ async def poll_func(): await done.wait() return "foo" - outgoing = asyncio.Queue() + outgoing = asyncio.Queue[str]() poller = Poller(5, permits, poll_func, outgoing.put) task = asyncio.create_task(poller.run()) @@ -121,7 +121,7 @@ async def poll_func(): await done.wait() return "bar" - outgoing = asyncio.Queue() + outgoing = asyncio.Queue[str]() poller = Poller(1, permits, poll_func, outgoing.put) task = asyncio.create_task(poller.run()) @@ -136,7 +136,7 @@ async def poll_func(): async def test_poller_execute_error(): permits = asyncio.Semaphore(1) - outgoing = asyncio.Queue() + outgoing = asyncio.Queue[str]() call_count = 0 async def execute(item: str): @@ -146,7 +146,7 @@ async def execute(item: str): raise RuntimeError("oh no") await outgoing.put(item) - incoming = asyncio.Queue() + incoming = asyncio.Queue[str]() poller = Poller(1, permits, incoming.get, execute) task = asyncio.create_task(poller.run()) diff --git a/tests/cadence/worker/test_registry.py b/tests/cadence/worker/test_registry.py index a7040dd..4f1aefd 100644 --- a/tests/cadence/worker/test_registry.py +++ b/tests/cadence/worker/test_registry.py @@ -1,8 +1,4 @@ -#!/usr/bin/env python3 -""" -Tests for the registry functionality. -""" - +# type: ignore import pytest from cadence import activity diff --git a/tests/cadence/worker/test_task_handler_integration.py b/tests/cadence/worker/test_task_handler_integration.py index 920f53b..d987748 100644 --- a/tests/cadence/worker/test_task_handler_integration.py +++ b/tests/cadence/worker/test_task_handler_integration.py @@ -1,8 +1,3 @@ -#!/usr/bin/env python3 -""" -Integration tests for task handlers. -""" - import pytest from contextlib import contextmanager from unittest.mock import Mock, AsyncMock, patch, PropertyMock diff --git a/tests/integration_tests/helper.py b/tests/integration_tests/helper.py index 6664611..3d70760 100644 --- a/tests/integration_tests/helper.py +++ b/tests/integration_tests/helper.py @@ -1,6 +1,6 @@ import asyncio from contextlib import asynccontextmanager -from typing import AsyncGenerator +from typing import AsyncGenerator, Unpack from cadence import Registry from cadence.client import ClientOptions, Client @@ -16,7 +16,7 @@ def __init__(self, options: ClientOptions, test_name: str) -> None: @asynccontextmanager async def worker( - self, registry: Registry, **kwargs: WorkerOptions + self, registry: Registry, **kwargs: Unpack[WorkerOptions] ) -> AsyncGenerator[Worker, None]: async with self.client() as client: worker = Worker(client, self.test_name, registry, **kwargs) diff --git a/tests/integration_tests/workflow/test_activities.py b/tests/integration_tests/workflow/test_activities.py index 7e6c0cc..75b8b3a 100644 --- a/tests/integration_tests/workflow/test_activities.py +++ b/tests/integration_tests/workflow/test_activities.py @@ -1,3 +1,4 @@ +# type: ignore import asyncio from datetime import timedelta diff --git a/tests/integration_tests/workflow/test_workflows.py b/tests/integration_tests/workflow/test_workflows.py index 4a01af1..28ee6e1 100644 --- a/tests/integration_tests/workflow/test_workflows.py +++ b/tests/integration_tests/workflow/test_workflows.py @@ -1,3 +1,4 @@ +# type: ignore from datetime import timedelta import pytest