From 7083a953e19a9ba4bfd33f3437f9e4127755d274 Mon Sep 17 00:00:00 2001 From: Escape0707 Date: Tue, 28 Jul 2026 23:04:56 +0900 Subject: [PATCH] test: enforce strict typing in migrated unit tests (#39689) --- api/tests/unit_tests/conftest.py | 19 ++- ...st_update_provider_when_message_created.py | 9 +- api/tests/unit_tests/pyrefly.toml | 3 - .../services/test_human_input_service.py | 150 ++++++++++++------ 4 files changed, 114 insertions(+), 67 deletions(-) diff --git a/api/tests/unit_tests/conftest.py b/api/tests/unit_tests/conftest.py index e0e4361d8cd..4edf0be3193 100644 --- a/api/tests/unit_tests/conftest.py +++ b/api/tests/unit_tests/conftest.py @@ -43,7 +43,7 @@ from models.account import Account, Tenant, TenantAccountJoin, TenantAccountRole from models.base import TypeBase -def _patch_redis_clients_on_loaded_modules(): +def _patch_redis_clients_on_loaded_modules() -> None: """Ensure any module-level redis_client references point to the shared redis_mock.""" import sys @@ -51,10 +51,9 @@ def _patch_redis_clients_on_loaded_modules(): for module in list(sys.modules.values()): if module is None: continue - if hasattr(module, "redis_client"): - module.redis_client = redis_mock - if hasattr(module, "_pubsub_redis_client"): - module.pubsub_redis_client = redis_mock + for client_attribute in ("redis_client", "_pubsub_redis_client"): + if hasattr(module, client_attribute): + setattr(module, client_attribute, redis_mock) @pytest.fixture @@ -63,13 +62,13 @@ def app() -> Flask: @pytest.fixture(autouse=True) -def _provide_app_context(app: Flask): +def _provide_app_context(app: Flask) -> Iterator[None]: with app.app_context(): yield @pytest.fixture(autouse=True) -def _patch_redis_clients(): +def _patch_redis_clients() -> Iterator[None]: """Patch redis_client to MagicMock only for unit test executions.""" with ( @@ -81,7 +80,7 @@ def _patch_redis_clients(): @pytest.fixture(autouse=True) -def reset_redis_mock(): +def reset_redis_mock() -> None: """reset the Redis mock before each test""" redis_mock.reset_mock() redis_mock.get.return_value = None @@ -91,7 +90,7 @@ def reset_redis_mock(): redis_mock.exists.return_value = False redis_mock.set.return_value = None redis_mock.expire.return_value = None - redis_mock.hgetall.return_value = {} + redis_mock.hgetall.return_value = dict[bytes, bytes]() redis_mock.hdel.return_value = None redis_mock.incr.return_value = 1 @@ -100,7 +99,7 @@ def reset_redis_mock(): @pytest.fixture(autouse=True) -def reset_secret_key(): +def reset_secret_key() -> Iterator[None]: """Ensure SECRET_KEY-dependent logic sees an empty config value by default.""" from configs import dify_config diff --git a/api/tests/unit_tests/events/test_update_provider_when_message_created.py b/api/tests/unit_tests/events/test_update_provider_when_message_created.py index 54ae6460fb6..a700783b31c 100644 --- a/api/tests/unit_tests/events/test_update_provider_when_message_created.py +++ b/api/tests/unit_tests/events/test_update_provider_when_message_created.py @@ -11,7 +11,8 @@ from sqlalchemy.orm import Session, sessionmaker from core.app.entities.app_invoke_entities import ChatAppGenerateEntity from core.entities.provider_entities import ProviderQuotaType, QuotaUnit from events.event_handlers import update_provider_when_message_created -from models import TenantCreditPool +from models import Message, TenantCreditPool +from models.enums import ProviderQuotaType as ModelProviderQuotaType from models.provider import ProviderType @@ -30,7 +31,7 @@ def test_message_created_trial_credit_accounting_does_not_raise_when_balance_is_ pool_id = str(uuid4()) pool = TenantCreditPool( tenant_id=tenant_id, - pool_type=ProviderQuotaType.TRIAL, + pool_type=ModelProviderQuotaType.TRIAL, quota_limit=10, quota_used=9, ) @@ -61,7 +62,7 @@ def test_message_created_trial_credit_accounting_does_not_raise_when_balance_is_ ), ), ) - message = SimpleNamespace(message_tokens=2, answer_tokens=1) + message = Message(message_tokens=2, answer_tokens=1) with ( patch.object(update_provider_when_message_created, "_execute_provider_updates"), @@ -102,7 +103,7 @@ def test_message_created_paid_credit_accounting_uses_paid_pool() -> None: ), ), ) - message = SimpleNamespace(message_tokens=2, answer_tokens=1) + message = Message(message_tokens=2, answer_tokens=1) with ( patch.object(update_provider_when_message_created, "_deduct_credit_pool_quota_capped") as mock_deduct, diff --git a/api/tests/unit_tests/pyrefly.toml b/api/tests/unit_tests/pyrefly.toml index ad591ee5530..7cbfb9503de 100644 --- a/api/tests/unit_tests/pyrefly.toml +++ b/api/tests/unit_tests/pyrefly.toml @@ -28,7 +28,6 @@ project-excludes = [ "configs/test_dify_config.py", "configs/test_env_consistency.py", "configs/test_nacos_http_client.py", - "conftest.py", "controllers/common/test_agent_app_parameters.py", "controllers/common/test_app_access.py", "controllers/common/test_errors.py", @@ -652,7 +651,6 @@ project-excludes = [ "events/event_handlers/test_delete_tool_parameters_cache_when_sync_draft_workflow.py", "events/test_app_event_signals.py", "events/test_events_package_compat.py", - "events/test_update_provider_when_message_created.py", "extensions/logstore/repositories/test_logstore_api_workflow_node_execution_repository.py", "extensions/logstore/test_sql_escape.py", "extensions/otel/conftest.py", @@ -863,7 +861,6 @@ project-excludes = [ "services/test_file_service.py", "services/test_human_input_delivery_test_service.py", "services/test_human_input_file_upload_service.py", - "services/test_human_input_service.py", "services/test_knowledge_retrieval_inner_service.py", "services/test_knowledge_service.py", "services/test_message_service.py", diff --git a/api/tests/unit_tests/services/test_human_input_service.py b/api/tests/unit_tests/services/test_human_input_service.py index 78e1b056d2f..6d3e8d94c94 100644 --- a/api/tests/unit_tests/services/test_human_input_service.py +++ b/api/tests/unit_tests/services/test_human_input_service.py @@ -4,6 +4,7 @@ from datetime import datetime, timedelta from unittest.mock import MagicMock import pytest +from pydantic import JsonValue from pytest_mock import MockerFixture from sqlalchemy.engine import Engine from sqlalchemy.orm import Session, sessionmaker @@ -61,7 +62,7 @@ def _make_app(mode: AppMode) -> App: @pytest.fixture -def sample_form_record(): +def sample_form_record() -> HumanInputFormRecord: return HumanInputFormRecord( form_id="form-id", workflow_run_id="workflow-run-id", @@ -95,7 +96,7 @@ def sample_form_record(): def test_enqueue_resume_dispatches_task_for_workflow( mocker: MockerFixture, sqlite_session_factory: sessionmaker[Session], -): +) -> None: service = HumanInputService(sqlite_session_factory) workflow_run = MagicMock() @@ -121,8 +122,10 @@ def test_enqueue_resume_dispatches_task_for_workflow( def test_ensure_form_active_respects_global_timeout( - monkeypatch, sample_form_record: HumanInputFormRecord, unbound_session_factory -): + monkeypatch: pytest.MonkeyPatch, + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: service = HumanInputService(unbound_session_factory) expired_record = dataclasses.replace( sample_form_record, @@ -138,7 +141,7 @@ def test_ensure_form_active_respects_global_timeout( def test_enqueue_resume_dispatches_task_for_advanced_chat( mocker: MockerFixture, sqlite_session_factory: sessionmaker[Session], -): +) -> None: service = HumanInputService(sqlite_session_factory) workflow_run = MagicMock() @@ -166,7 +169,7 @@ def test_enqueue_resume_dispatches_task_for_advanced_chat( def test_enqueue_resume_skips_unsupported_app_mode( mocker: MockerFixture, sqlite_session_factory: sessionmaker[Session], -): +) -> None: service = HumanInputService(sqlite_session_factory) workflow_run = MagicMock() @@ -190,8 +193,9 @@ def test_enqueue_resume_skips_unsupported_app_mode( def test_get_form_definition_by_token_for_console_uses_repository( - sample_form_record: HumanInputFormRecord, unbound_session_factory -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) console_record = dataclasses.replace(sample_form_record, recipient_type=RecipientType.CONSOLE) repo.get_by_token.return_value = console_record @@ -232,8 +236,10 @@ def _build_resumption_context_state(*, options: list[str], workflow_run_id: str) def test_resolve_form_inputs_uses_runtime_select_options( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: configured_input = SelectInputConfig( output_variable_name="decision", option_source=StringListSource( @@ -270,8 +276,10 @@ def test_resolve_form_inputs_uses_runtime_select_options( def test_submit_form_by_token_calls_repository_and_enqueue( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record repo.mark_submitted.return_value = sample_form_record @@ -298,8 +306,10 @@ def test_submit_form_by_token_calls_repository_and_enqueue( def test_submit_form_by_token_enqueues_agent_app_resume_for_conversation_form( - sample_form_record, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: # ENG-635: a conversation-owned (Agent v2 chat) form routes to the chat # resume, not the workflow resume. repo = MagicMock(spec=HumanInputFormSubmissionRepository) @@ -327,8 +337,10 @@ def test_submit_form_by_token_enqueues_agent_app_resume_for_conversation_form( def test_submit_form_by_token_skips_enqueue_for_delivery_test( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) test_record = dataclasses.replace( sample_form_record, @@ -351,8 +363,10 @@ def test_submit_form_by_token_skips_enqueue_for_delivery_test( def test_submit_form_by_token_passes_submission_user_id( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record repo.mark_submitted.return_value = sample_form_record @@ -373,7 +387,10 @@ def test_submit_form_by_token_passes_submission_user_id( enqueue_spy.assert_called_once_with(sample_form_record.workflow_run_id) -def test_submit_form_by_token_invalid_action(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_submit_form_by_token_invalid_action( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = dataclasses.replace(sample_form_record) service = HumanInputService(unbound_session_factory, form_repository=repo) @@ -390,7 +407,10 @@ def test_submit_form_by_token_invalid_action(sample_form_record: HumanInputFormR repo.mark_submitted.assert_not_called() -def test_submit_form_by_token_missing_inputs(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_submit_form_by_token_missing_inputs( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition_with_input = FormDefinition( @@ -465,12 +485,12 @@ def test_submit_form_by_token_missing_inputs(sample_form_record: HumanInputFormR ], ) def test_validate_human_input_submission_rejects_invalid_select_and_file_payloads( - sample_form_record, - unbound_session_factory, - input_definition, - submitted_value, - expected_message, -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + input_definition: dict[str, JsonValue], + submitted_value: JsonValue, + expected_message: str, +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition.model_validate( { @@ -489,14 +509,14 @@ def test_validate_human_input_submission_rejects_invalid_select_and_file_payload recipient_type=RecipientType.STANDALONE_WEB_APP, form_token="token", selected_action_id="submit", - form_data={input_definition["output_variable_name"]: submitted_value}, + form_data={definition.inputs[0].output_variable_name: submitted_value}, ) assert expected_message in str(exc_info.value) repo.mark_submitted.assert_not_called() -def test_form_properties(sample_form_record: HumanInputFormRecord): +def test_form_properties(sample_form_record: HumanInputFormRecord) -> None: form = Form(sample_form_record) assert form.id == "form-id" assert form.workflow_run_id == "workflow-run-id" @@ -510,20 +530,20 @@ def test_form_properties(sample_form_record: HumanInputFormRecord): assert isinstance(form.expiration_time, datetime) -def test_form_submitted_error_init(): +def test_form_submitted_error_init() -> None: error = FormSubmittedError(form_id="test-form") assert error.description == "This form has already been submitted by another user, form_id=test-form" assert error.code == 412 -def test_human_input_service_init_with_engine(sqlite_engine: Engine): +def test_human_input_service_init_with_engine(sqlite_engine: Engine) -> None: service = HumanInputService(session_factory=sqlite_engine) assert isinstance(service._session_factory, sessionmaker) assert service._session_factory.kw["bind"] is sqlite_engine -def test_get_form_by_token_none(unbound_session_factory): +def test_get_form_by_token_none(unbound_session_factory: sessionmaker[Session]) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = None @@ -531,7 +551,10 @@ def test_get_form_by_token_none(unbound_session_factory): assert service.get_form_by_token("invalid") is None -def test_get_form_definition_by_token_mismatch(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_get_form_definition_by_token_mismatch( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record @@ -540,7 +563,10 @@ def test_get_form_definition_by_token_mismatch(sample_form_record: HumanInputFor assert service.get_form_definition_by_token(RecipientType.CONSOLE, "token") is None -def test_get_form_definition_by_token_success(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_get_form_definition_by_token_success( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record @@ -551,8 +577,9 @@ def test_get_form_definition_by_token_success(sample_form_record: HumanInputForm def test_get_form_definition_by_token_for_console_mismatch( - sample_form_record: HumanInputFormRecord, unbound_session_factory -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record # is STANDALONE_WEB_APP @@ -560,7 +587,9 @@ def test_get_form_definition_by_token_for_console_mismatch( assert service.get_form_definition_by_token_for_console("token") is None -def test_submit_form_by_token_delivery_not_enabled(unbound_session_factory): +def test_submit_form_by_token_delivery_not_enabled( + unbound_session_factory: sessionmaker[Session], +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = None @@ -570,8 +599,10 @@ def test_submit_form_by_token_delivery_not_enabled(unbound_session_factory): def test_submit_form_by_token_no_workflow_run_id( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture -): + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, +) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) repo.get_by_token.return_value = sample_form_record @@ -586,7 +617,10 @@ def test_submit_form_by_token_no_workflow_run_id( enqueue_spy.assert_not_called() -def test_ensure_form_active_errors(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_ensure_form_active_errors( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: service = HumanInputService(unbound_session_factory) # Submitted @@ -607,7 +641,10 @@ def test_ensure_form_active_errors(sample_form_record: HumanInputFormRecord, unb service.ensure_form_active(Form(expired_time_record)) -def test_ensure_not_submitted_raises(sample_form_record: HumanInputFormRecord, unbound_session_factory): +def test_ensure_not_submitted_raises( + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: service = HumanInputService(unbound_session_factory) submitted_record = dataclasses.replace(sample_form_record, submitted_at=naive_utc_now()) @@ -615,7 +652,10 @@ def test_ensure_not_submitted_raises(sample_form_record: HumanInputFormRecord, u service._ensure_not_submitted(Form(submitted_record)) -def test_enqueue_resume_workflow_not_found(mocker: MockerFixture, unbound_session_factory): +def test_enqueue_resume_workflow_not_found( + mocker: MockerFixture, + unbound_session_factory: sessionmaker[Session], +) -> None: service = HumanInputService(unbound_session_factory) workflow_run_repo = MagicMock() @@ -631,10 +671,10 @@ def test_enqueue_resume_workflow_not_found(mocker: MockerFixture, unbound_sessio def test_enqueue_resume_app_not_found( - mocker, + mocker: MockerFixture, sqlite_session_factory: sessionmaker[Session], caplog: pytest.LogCaptureFixture, -): +) -> None: service = HumanInputService(sqlite_session_factory) workflow_run = MagicMock() @@ -660,8 +700,10 @@ def test_enqueue_resume_app_not_found( def test_is_globally_expired_zero_timeout( - monkeypatch: pytest.MonkeyPatch, sample_form_record: HumanInputFormRecord, unbound_session_factory -): + monkeypatch: pytest.MonkeyPatch, + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], +) -> None: service = HumanInputService(unbound_session_factory) monkeypatch.setattr(human_input_service_module.dify_config, "HUMAN_INPUT_GLOBAL_TIMEOUT_SECONDS", 0) @@ -669,7 +711,9 @@ def test_is_globally_expired_zero_timeout( def test_submit_form_by_token_normalizes_select_and_files( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, ) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition( @@ -751,7 +795,8 @@ def test_submit_form_by_token_normalizes_select_and_files( def test_submit_form_by_token_invalid_select_value( - sample_form_record: HumanInputFormRecord, unbound_session_factory + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], ) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition( @@ -779,7 +824,8 @@ def test_submit_form_by_token_invalid_select_value( def test_submit_form_by_token_invalid_file_list_item( - sample_form_record: HumanInputFormRecord, unbound_session_factory + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], ) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition( @@ -805,7 +851,9 @@ def test_submit_form_by_token_invalid_file_list_item( def test_submit_form_by_token_rejects_cross_tenant_file( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, ) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition( @@ -837,7 +885,9 @@ def test_submit_form_by_token_rejects_cross_tenant_file( def test_submit_form_by_token_rejects_cross_tenant_file_list( - sample_form_record: HumanInputFormRecord, unbound_session_factory, mocker: MockerFixture + sample_form_record: HumanInputFormRecord, + unbound_session_factory: sessionmaker[Session], + mocker: MockerFixture, ) -> None: repo = MagicMock(spec=HumanInputFormSubmissionRepository) definition = FormDefinition(