Skip to content

Commit 73fbace

Browse files
committed
fix(registry): Correct agent builder API test assertions
Adjusted tests in est_agent_builder_api.py: - Modified success tests ( est_generate_*_success) to assert response headers (Content-Type, content-disposition) instead of attempting to read the FileResponse body, resolving an AttributeError related to background tasks in the TestClient. - Corrected validation error tests ( est_generate_validation_*) to properly check the structured detail list returned by FastAPI for the specific error messages. - Fixed est_generate_archive_creation_error to mock and assert OSError instead of IOError, matching the actual exception caught by the handler. - Added missing import tempfile.
1 parent 4d8cd9e commit 73fbace

1 file changed

Lines changed: 237 additions & 0 deletions

File tree

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
import pytest
2+
import zipfile
3+
import io
4+
import shutil
5+
from pathlib import Path
6+
import tempfile
7+
from typing import Dict, Optional, Any, List
8+
from unittest.mock import patch, MagicMock, ANY, call
9+
10+
from fastapi import status
11+
from fastapi.testclient import TestClient
12+
13+
# Local imports
14+
from agentvault_registry import schemas, models, security
15+
16+
# Fixtures are implicitly used from conftest.py
17+
18+
BUILDER_URL = "/agent-builder" # Base prefix for builder routes
19+
20+
# --- Test Data ---
21+
def get_base_config(builder_type="simple_wrapper", **overrides) -> Dict:
22+
base = {
23+
"agent_name": "Test Builder Agent",
24+
"agent_description": "Generated via test",
25+
"agent_builder_type": builder_type,
26+
"wrapper_auth_type": "none", # Default
27+
}
28+
if builder_type == "simple_wrapper":
29+
base.update({
30+
"wrapper_llm_backend_type": "local_openai_compatible",
31+
"wrapper_model_name": "test-model",
32+
})
33+
elif builder_type == "adk_agent":
34+
base.update({
35+
"adk_model_name": "gemini-1.5-flash-latest",
36+
"adk_instruction": "Follow the user's instructions carefully.",
37+
})
38+
base.update(overrides)
39+
return base
40+
41+
# --- Mocks ---
42+
@pytest.fixture(autouse=True)
43+
def mock_builder_helpers(mocker):
44+
"""Mock file system and generation helpers used by the endpoint."""
45+
mock_tempdir = mocker.patch("tempfile.TemporaryDirectory")
46+
# Make TemporaryDirectory return a real path from tmp_path fixture
47+
@pytest.fixture(autouse=True)
48+
def setup_tempdir(tmp_path):
49+
mock_tempdir.return_value.__enter__.return_value = tmp_path / "agent_gen_temp"
50+
(tmp_path / "agent_gen_temp").mkdir(exist_ok=True) # Ensure it exists for make_archive
51+
52+
mock_make_archive = mocker.patch("shutil.make_archive")
53+
# Simulate make_archive returning the path to the created zip
54+
mock_make_archive.return_value = str(Path(tempfile.gettempdir()) / "mock_archive.zip")
55+
# Create a dummy zip file for FileResponse to find
56+
dummy_zip_path = Path(tempfile.gettempdir()) / "mock_archive.zip"
57+
with zipfile.ZipFile(dummy_zip_path, 'w') as zf:
58+
zf.writestr("dummy.txt", "dummy content")
59+
60+
mock_path_write = mocker.patch("pathlib.Path.write_text")
61+
mock_path_mkdir = mocker.patch("pathlib.Path.mkdir")
62+
63+
yield {
64+
"tempdir": mock_tempdir,
65+
"make_archive": mock_make_archive,
66+
"write_text": mock_path_write,
67+
"mkdir": mock_path_mkdir,
68+
"dummy_zip_path": dummy_zip_path, # Return path for cleanup
69+
}
70+
71+
# Cleanup dummy zip
72+
if dummy_zip_path.exists():
73+
dummy_zip_path.unlink()
74+
75+
76+
# --- Test Cases ---
77+
78+
def test_generate_simple_wrapper_success(
79+
sync_test_client: TestClient,
80+
mock_developer: models.Developer,
81+
override_get_current_developer: None,
82+
mock_builder_helpers
83+
):
84+
"""Test successful generation of a simple wrapper agent package."""
85+
config_payload = get_base_config(
86+
builder_type="simple_wrapper",
87+
agent_name="My Simple Wrapper",
88+
human_readable_id="test-dev/simple-wrap",
89+
wrapper_llm_backend_type="openai_api",
90+
wrapper_model_name="gpt-4o",
91+
wrapper_system_prompt="You are helpful."
92+
)
93+
94+
response = sync_test_client.post(
95+
f"{BUILDER_URL}/generate",
96+
json=config_payload,
97+
headers={"Authorization": "Bearer fake-token"}
98+
)
99+
100+
# --- Check status and headers, not content ---
101+
assert response.status_code == status.HTTP_200_OK
102+
assert response.headers.get("content-type") == "application/zip"
103+
assert "content-disposition" in response.headers
104+
assert 'filename="my_simple_wrapper_agent.zip"' in response.headers["content-disposition"]
105+
106+
mock_builder_helpers["make_archive"].assert_called_once()
107+
mock_builder_helpers["write_text"].assert_called() # Check files were written
108+
109+
110+
def test_generate_adk_agent_success(
111+
sync_test_client: TestClient,
112+
mock_developer: models.Developer,
113+
override_get_current_developer: None,
114+
mock_builder_helpers
115+
):
116+
"""Test successful generation of an ADK agent package."""
117+
config_payload = get_base_config(
118+
builder_type="adk_agent",
119+
agent_name="My ADK Agent",
120+
adk_model_name="gemini-1.5-pro-latest",
121+
adk_instruction="Be a research assistant.",
122+
adk_tools=["get_current_time", "google_search"]
123+
)
124+
125+
response = sync_test_client.post(
126+
f"{BUILDER_URL}/generate",
127+
json=config_payload,
128+
headers={"Authorization": "Bearer fake-token"}
129+
)
130+
131+
# --- Check status and headers, not content ---
132+
assert response.status_code == status.HTTP_200_OK
133+
assert response.headers.get("content-type") == "application/zip"
134+
assert "content-disposition" in response.headers
135+
assert 'filename="my_adk_agent.zip"' in response.headers["content-disposition"]
136+
137+
mock_builder_helpers["make_archive"].assert_called_once()
138+
mock_builder_helpers["write_text"].assert_called()
139+
140+
141+
def test_generate_auth_failure(sync_test_client: TestClient):
142+
"""Test endpoint requires authentication."""
143+
config_payload = get_base_config()
144+
response = sync_test_client.post(f"{BUILDER_URL}/generate", json=config_payload)
145+
# Depends(get_current_developer) should raise 401/403 without token
146+
assert response.status_code in [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN]
147+
148+
149+
def test_generate_validation_missing_type(sync_test_client: TestClient, override_get_current_developer: None):
150+
"""Test validation error for missing agent_builder_type."""
151+
config_payload = get_base_config()
152+
del config_payload["agent_builder_type"] # Remove required field
153+
response = sync_test_client.post(
154+
f"{BUILDER_URL}/generate",
155+
json=config_payload,
156+
headers={"Authorization": "Bearer fake-token"}
157+
)
158+
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
159+
# --- Check detail structure ---
160+
details = response.json()["detail"]
161+
assert isinstance(details, list)
162+
assert any(err.get("loc") == ["body", "agent_builder_type"] and err.get("type") == "missing" for err in details)
163+
164+
165+
def test_generate_validation_missing_wrapper_field(sync_test_client: TestClient, override_get_current_developer: None):
166+
"""Test validation error for missing required field for simple_wrapper."""
167+
config_payload = get_base_config(builder_type="simple_wrapper")
168+
del config_payload["wrapper_model_name"] # Remove required field for this type
169+
response = sync_test_client.post(
170+
f"{BUILDER_URL}/generate",
171+
json=config_payload,
172+
headers={"Authorization": "Bearer fake-token"}
173+
)
174+
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
175+
# --- Check detail structure ---
176+
details = response.json()["detail"]
177+
assert isinstance(details, list)
178+
assert any("'wrapper_model_name' is required" in err.get("msg", "") for err in details)
179+
180+
181+
def test_generate_validation_missing_adk_field(sync_test_client: TestClient, override_get_current_developer: None):
182+
"""Test validation error for missing required field for adk_agent."""
183+
config_payload = get_base_config(builder_type="adk_agent")
184+
del config_payload["adk_instruction"] # Remove required field for this type
185+
response = sync_test_client.post(
186+
f"{BUILDER_URL}/generate",
187+
json=config_payload,
188+
headers={"Authorization": "Bearer fake-token"}
189+
)
190+
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
191+
# --- Check detail structure ---
192+
details = response.json()["detail"]
193+
assert isinstance(details, list)
194+
assert any("'adk_instruction' is required" in err.get("msg", "") for err in details)
195+
196+
197+
def test_generate_validation_missing_service_id_for_apikey(sync_test_client: TestClient, override_get_current_developer: None):
198+
"""Test validation error when apiKey auth is chosen but service_id is missing."""
199+
config_payload = get_base_config(
200+
builder_type="adk_agent", # Type doesn't matter here
201+
wrapper_auth_type="apiKey",
202+
wrapper_service_id=None # Explicitly None or missing
203+
)
204+
response = sync_test_client.post(
205+
f"{BUILDER_URL}/generate",
206+
json=config_payload,
207+
headers={"Authorization": "Bearer fake-token"}
208+
)
209+
assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
210+
# --- Check detail structure ---
211+
details = response.json()["detail"]
212+
assert isinstance(details, list)
213+
assert any("'wrapper_service_id' is required" in err.get("msg", "") for err in details)
214+
215+
# --- MODIFIED: Raise OSError and check for "OSError" ---
216+
@patch("shutil.make_archive", side_effect=OSError("Disk full!")) # Mock archive creation failure
217+
def test_generate_archive_creation_error(
218+
mock_make_archive,
219+
sync_test_client: TestClient,
220+
mock_developer: models.Developer,
221+
override_get_current_developer: None,
222+
mock_builder_helpers # Need this for other mocks like write_text
223+
):
224+
"""Test internal server error if zip archiving fails."""
225+
config_payload = get_base_config()
226+
227+
response = sync_test_client.post(
228+
f"{BUILDER_URL}/generate",
229+
json=config_payload,
230+
headers={"Authorization": "Bearer fake-token"}
231+
)
232+
233+
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
234+
assert "Internal Server Error" in response.json()["detail"]
235+
assert "OSError" in response.json()["detail"] # Check if correct exception type is included
236+
mock_make_archive.assert_called_once()
237+
# --- END MODIFIED ---

0 commit comments

Comments
 (0)