fix(agents-api): Fix all pytest tests

creatorrr · creatorrr · commit 60a6da918dfb · 2025-06-24T15:58:20.000+05:30
Signed-off-by: Diwank Singh Tomer &lt;diwank.singh@gmail.com&gt;
diff --git a/agents-api/.pytest-runtimes b/agents-api/.pytest-runtimes
diff --git a/agents-api/AGENTS.md b/agents-api/AGENTS.md
@@ -85,3 +85,8 @@ Key Uses
 - All Ward imports removed, migration complete
 - Run tests: `poe test` or `poe test -k "pattern"` for specific tests
 - Stop on first failure: `poe test -x`
+
+## Type Checking
+- AIDEV-NOTE: autogen/openapi_model.py is handwritten, not auto-generated
+- Type checking errors from openapi_model.py are intentional (dynamic type property patches)
+- Use `ty check` for extremely fast type checking (pytype replacement)
diff --git a/agents-api/agents_api/autogen/AGENTS.md b/agents-api/agents_api/autogen/AGENTS.md
@@ -4,6 +4,7 @@ This folder contains auto-generated code in `agents-api` from TypeSpec and OpenA
 
 Key Points
 - Do NOT edit files here manually; they are overwritten on regeneration.
+- EXCEPTION: `openapi_model.py` is handwritten and should be edited manually (not auto-generated).
 - Regenerate via `bash scripts/generate_openapi_code.sh` from the project root.
 - Source-of-truth TypeSpec definitions reside in the `typespec/` directory.
 - Ensure version compatibility between TypeSpec plugin and codegen scripts.
diff --git a/agents-api/agents_api/autogen/openapi_model.py b/agents-api/agents_api/autogen/openapi_model.py
@@ -1,4 +1,5 @@
 # ruff: noqa: F401, F403, F405
+# AIDEV-NOTE: This file is handwritten, not auto-generated like other files in autogen/
 import ast
 from typing import Annotated, Any, Generic, Literal, Self, TypeVar, get_args
 from uuid import UUID
@@ -103,20 +104,20 @@ def type_property(self: BaseModel) -> str:
         else "api_call"
         if self.api_call
         else None
-    )
+    )  # type: ignore[invalid-return-type]
 
 
 # Patch original Tool class to add 'type' property
-TaskTool.type = computed_field(property(type_property))
+TaskTool.type = computed_field(property(type_property))  # type: ignore[invalid-attribute-access]
 
 # Patch original Tool class to add 'type' property
-Tool.type = computed_field(property(type_property))
+Tool.type = computed_field(property(type_property))  # type: ignore[invalid-attribute-access]
 
 # Patch original UpdateToolRequest class to add 'type' property
-UpdateToolRequest.type = computed_field(property(type_property))
+UpdateToolRequest.type = computed_field(property(type_property))  # type: ignore[invalid-attribute-access]
 
 # Patch original PatchToolRequest class to add 'type' property
-PatchToolRequest.type = computed_field(property(type_property))
+PatchToolRequest.type = computed_field(property(type_property))  # type: ignore[invalid-attribute-access]
 
 
 # Patch Task Workflow Steps
diff --git a/agents-api/agents_api/workflows/task_execution/helpers.py b/agents-api/agents_api/workflows/task_execution/helpers.py
@@ -46,10 +46,11 @@ def validate_execution_input(execution_input: ExecutionInput) -> TaskSpecDef:
     Raises:
         ApplicationError: If task is None
     """
-    if execution_input.task is None:
+    task = execution_input.task
+    if task is None:
         msg = "Execution input task cannot be None"
         raise ApplicationError(msg)
-    return execution_input.task
+    return task
 
 
 async def base_evaluate_activity(
diff --git a/agents-api/tests/conftest.py b/agents-api/tests/conftest.py
@@ -3,6 +3,7 @@
 Migrated from Ward fixtures.py
 """
 
+import contextlib
 import os
 import random
 import string
@@ -615,11 +616,37 @@ def disable_s3_cache():
 
 
 @pytest.fixture
-def s3_client():
+async def s3_client(localstack_container):
     """S3 client fixture that works with TestClient's event loop."""
-    # The TestClient's lifespan will create the S3 client
-    # The disable_s3_cache fixture ensures we don't have event loop issues
-    yield
+    from contextlib import AsyncExitStack
+
+    from aiobotocore.session import get_session
+
+    # AIDEV-NOTE: Fixed S3 client fixture with proper LocalStack integration
+    # to resolve NoSuchKey errors in file route tests
+
+    # Create async S3 client using LocalStack
+    session = get_session()
+
+    async with AsyncExitStack() as stack:
+        client = await stack.enter_async_context(
+            session.create_client(
+                "s3",
+                aws_access_key_id=localstack_container.env["AWS_ACCESS_KEY_ID"],
+                aws_secret_access_key=localstack_container.env["AWS_SECRET_ACCESS_KEY"],
+                endpoint_url=localstack_container.get_url(),
+                region_name="us-east-1",
+            )
+        )
+
+        # Ensure default bucket exists
+        try:
+            await client.head_bucket(Bucket="default")
+        except Exception:
+            with contextlib.suppress(Exception):
+                await client.create_bucket(Bucket="default")  # Bucket might already exist
+
+        yield client
 
 
 @pytest.fixture
diff --git a/agents-api/tests/sample_tasks/test_find_selector.py b/agents-api/tests/sample_tasks/test_find_selector.py
@@ -48,7 +48,7 @@ async def test_workflow_sample_find_selector_start_with_bad_input_should_fail(
     ):
         task_def = sample_file.read()
 
-        async with patch_testing_temporal() as (_, temporal_client):
+        async with patch_testing_temporal():
             response = make_request(
                 method="POST",
                 url=f"/agents/{agent_id}/tasks/{task_id}",
@@ -85,7 +85,7 @@ async def test_workflow_sample_find_selector_start_with_correct_input(
     ):
         task_def = sample_file.read()
 
-        async with patch_testing_temporal() as (_, mock_temporal_client):
+        async with patch_testing_temporal():
             response = make_request(
                 method="POST",
                 url=f"/agents/{agent_id}/tasks/{task_id}",
diff --git a/agents-api/tests/test_chat_streaming.py b/agents-api/tests/test_chat_streaming.py
@@ -333,7 +333,7 @@ async def mock_render(*args, **kwargs):
         create_entries_mock.assert_called_once()
         call_args = create_entries_mock.call_args[1]
         assert call_args["developer_id"] == test_developer_id
-        assert call_args["session_id"] == test_session.id
+        assert call_args["session_id"] == session.id
         # Verify we're saving the user message
         assert len(call_args["data"]) == 1
         assert call_args["data"][0].role == "user"
diff --git a/agents-api/tests/test_docs_routes.py b/agents-api/tests/test_docs_routes.py
@@ -1,4 +1,3 @@
-
 from .utils import patch_testing_temporal
 
 
diff --git a/agents-api/tests/test_middleware.py b/agents-api/tests/test_middleware.py
@@ -8,7 +8,9 @@
 import asyncpg
 import pytest
 from agents_api.app import app
+from agents_api.clients.pg import create_db_pool
 from agents_api.env import free_tier_cost_limit
+from agents_api.queries.developers.create_developer import create_developer
 from fastapi import HTTPException, status
 from fastapi.testclient import TestClient
 from pydantic import BaseModel
diff --git a/agents-api/tests/test_usage_cost.py b/agents-api/tests/test_usage_cost.py
@@ -19,15 +19,26 @@ async def test_query_get_usage_cost_returns_zero_when_no_usage_records_exist(
     """Test that get_usage_cost returns zero cost when no usage records exist."""
     pool = await create_db_pool(dsn=pg_dsn)
 
-    # Calculate expected cost
+    # Create a new developer ID for this test to ensure clean state
+    clean_developer_id = uuid7()
+    await create_developer(
+        email=f"clean-test-{clean_developer_id}@example.com",
+        active=True,
+        tags=["test"],
+        settings={},
+        developer_id=clean_developer_id,
+        connection_pool=pool,
+    )
+
+    # Calculate expected cost - should be 0 for new developer
     expected_cost = Decimal("0")
 
     # Get the usage cost
-    cost_record = await get_usage_cost(developer_id=test_developer_id, connection_pool=pool)
+    cost_record = await get_usage_cost(developer_id=clean_developer_id, connection_pool=pool)
 
     # Verify the record
     assert cost_record is not None, "Should have a cost record"
-    assert cost_record["developer_id"] == test_developer_id
+    assert cost_record["developer_id"] == clean_developer_id
     assert "cost" in cost_record, "Should have a cost field"
     assert isinstance(cost_record["cost"], Decimal), "Cost should be a Decimal"
     assert cost_record["cost"] == expected_cost, (
@@ -43,17 +54,28 @@ async def test_query_get_usage_cost_returns_the_correct_cost_when_records_exist(
     """Test that get_usage_cost returns the correct cost for a developer with usage records."""
     pool = await create_db_pool(dsn=pg_dsn)
 
+    # Create a new developer ID for this test to ensure clean state
+    clean_developer_id = uuid7()
+    await create_developer(
+        email=f"clean-test-{clean_developer_id}@example.com",
+        active=True,
+        tags=["test"],
+        settings={},
+        developer_id=clean_developer_id,
+        connection_pool=pool,
+    )
+
     # Create some usage records for the developer
     record1 = await create_usage_record(
-        developer_id=test_developer_id,
+        developer_id=clean_developer_id,
         model="gpt-4o-mini",
         prompt_tokens=1000,
         completion_tokens=2000,
         connection_pool=pool,
     )
 
     record2 = await create_usage_record(
-        developer_id=test_developer_id,
+        developer_id=clean_developer_id,
         model="gpt-4o-mini",
         prompt_tokens=500,
         completion_tokens=1500,
@@ -71,11 +93,11 @@ async def test_query_get_usage_cost_returns_the_correct_cost_when_records_exist(
     await asyncio.sleep(0.1)
 
     # Get the usage cost
-    cost_record = await get_usage_cost(developer_id=test_developer_id, connection_pool=pool)
+    cost_record = await get_usage_cost(developer_id=clean_developer_id, connection_pool=pool)
 
     # Verify the record
     assert cost_record is not None, "Should have a cost record"
-    assert cost_record["developer_id"] == test_developer_id
+    assert cost_record["developer_id"] == clean_developer_id
     assert "cost" in cost_record, "Should have a cost field"
     assert isinstance(cost_record["cost"], Decimal), "Cost should be a Decimal"
     assert cost_record["cost"] == expected_cost, (
diff --git a/agents-api/tests/test_user_routes.py b/agents-api/tests/test_user_routes.py
@@ -122,6 +122,12 @@ def test_query_patch_user_with_project(make_request, test_user, test_project):
 
 def test_query_list_users(make_request):
     """query: list users"""
+    # First create a user to ensure there's at least one
+    data = {"name": "test user for list", "about": "test user about"}
+    create_response = make_request(method="POST", url="/users", json=data)
+    assert create_response.status_code == 201
+
+    # Now list users
     response = make_request(method="GET", url="/users")
     assert response.status_code == 200
     response = response.json()
diff --git a/agents-api/tests/utils.py b/agents-api/tests/utils.py
@@ -3,7 +3,7 @@
 import math
 import os
 import subprocess
-from contextlib import asynccontextmanager, contextmanager
+from contextlib import asynccontextmanager, contextmanager, suppress
 from typing import Any
 from unittest.mock import patch
 from uuid import UUID
@@ -138,10 +138,8 @@ async def patch_testing_temporal():
             # Cancel the worker task if it's still running
             if not worker_task.done():
                 worker_task.cancel()
-                try:
+                with suppress(asyncio.CancelledError):
                     await worker_task
-                except asyncio.CancelledError:
-                    pass  # Expected when task is cancelled
 
     # Reset log levels
     logger.setLevel(previous_log_level)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-`
`2`	`1`	`from .utils import patch_testing_temporal`
`3`	`2`
`4`	`3`