fix(tests): correct two independent test-api defects found while auditing T-55
1. get_task_executor mocking: same dependency_overrides fix as the previous
commit, applied to this file's remaining patch('src.main.get_task_executor')
call sites (test_valid_api_key_allows_access, test_create_task_with_valid_data,
test_trigger_task_endpoint, test_stats_endpoint_returns_metrics,
test_executions_endpoint_returns_history, test_executions_filter_by_task_name).
2. test_create_task_missing_fields_returns_400 asserted a status this endpoint
cannot return. `task: TaskCreate` in src/main.py is a plain Pydantic request
body with no custom validation for missing fields — FastAPI's own
dependency-resolution layer rejects the request before create_task's body
runs, and that layer always answers 422, not 400. There is no code path in
this repo, at any point in its git history, that produces 400 for this
request. Renamed to test_create_task_missing_fields_returns_422 and the
assertion updated to match; source unchanged.
Kept together in one commit because both live in the same small file and were
found in the same pass, rather than risk a manual hunk split on tightly
interleaved diff context.
This commit is contained in:
+123
-119
@@ -57,14 +57,13 @@ class TestAuthenticationEndpoints:
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict):
|
||||
def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
|
||||
"""Test that valid API key allows access."""
|
||||
with patch('src.main.get_task_executor') as mock_executor:
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []
|
||||
override_task_executor.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []
|
||||
|
||||
response = client.get("/tasks", headers=auth_headers)
|
||||
# May fail with 500 due to DB, but should not be 401/403
|
||||
assert response.status_code not in [401, 403]
|
||||
response = client.get("/tasks", headers=auth_headers)
|
||||
# May fail with 500 due to DB, but should not be 401/403
|
||||
assert response.status_code not in [401, 403]
|
||||
|
||||
|
||||
@pytest.mark.api
|
||||
@@ -72,8 +71,16 @@ class TestAuthenticationEndpoints:
|
||||
class TestTaskEndpoints:
|
||||
"""Tests for task management endpoints."""
|
||||
|
||||
def test_create_task_missing_fields_returns_400(self, client: TestClient, auth_headers: dict):
|
||||
"""Test that creating task without required fields returns 400."""
|
||||
def test_create_task_missing_fields_returns_422(self, client: TestClient, auth_headers: dict):
|
||||
"""Test that creating task without required fields returns 422.
|
||||
|
||||
`task: TaskCreate` (src/main.py) is a plain Pydantic request body with
|
||||
no custom validation — FastAPI's own dependency-resolution layer
|
||||
rejects a request missing required fields before create_task's body
|
||||
ever runs, and that layer always answers 422, never 400. There is no
|
||||
code path in this repo that could produce 400 here; renamed rather
|
||||
than asserting a status this endpoint cannot return.
|
||||
"""
|
||||
incomplete_task = {
|
||||
"task_name": "test",
|
||||
# Missing service, executor, priority
|
||||
@@ -83,63 +90,61 @@ class TestTaskEndpoints:
|
||||
headers=auth_headers,
|
||||
json=incomplete_task
|
||||
)
|
||||
assert response.status_code == 400
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict):
|
||||
def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict, override_task_executor: MagicMock):
|
||||
"""Test creating a task with valid data."""
|
||||
with patch('src.main.get_task_executor') as mock_executor:
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
|
||||
# Setup mock to return task data
|
||||
mock_cursor.fetchone.return_value = {**sample_task_data, "id": 1}
|
||||
# Setup mock to return task data
|
||||
mock_cursor.fetchone.return_value = {**sample_task_data, "id": 1}
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
response = client.post(
|
||||
"/tasks",
|
||||
headers=auth_headers,
|
||||
json=sample_task_data
|
||||
)
|
||||
|
||||
# Verify the call was made
|
||||
assert mock_cursor.execute.called
|
||||
# Check that config was JSON-encoded
|
||||
call_args = mock_cursor.execute.call_args
|
||||
assert 'config' in call_args[0][1]
|
||||
|
||||
def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
|
||||
"""Test manually triggering a task."""
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
|
||||
# Mock task retrieval
|
||||
mock_cursor.fetchone.return_value = {
|
||||
"task_name": "test_task",
|
||||
"enabled": True,
|
||||
"priority": 50,
|
||||
"executor": "example_executor"
|
||||
}
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
with patch('asyncio.create_task'):
|
||||
response = client.post(
|
||||
"/tasks",
|
||||
headers=auth_headers,
|
||||
json=sample_task_data
|
||||
"/tasks/test_task/trigger",
|
||||
headers=auth_headers
|
||||
)
|
||||
|
||||
# Verify the call was made
|
||||
assert mock_cursor.execute.called
|
||||
# Check that config was JSON-encoded
|
||||
call_args = mock_cursor.execute.call_args
|
||||
assert 'config' in call_args[0][1]
|
||||
|
||||
def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict):
|
||||
"""Test manually triggering a task."""
|
||||
with patch('src.main.get_task_executor') as mock_executor:
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
|
||||
# Mock task retrieval
|
||||
mock_cursor.fetchone.return_value = {
|
||||
"task_name": "test_task",
|
||||
"enabled": True,
|
||||
"priority": 50,
|
||||
"executor": "example_executor"
|
||||
}
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
with patch('asyncio.create_task'):
|
||||
response = client.post(
|
||||
"/tasks/test_task/trigger",
|
||||
headers=auth_headers
|
||||
)
|
||||
|
||||
# Should return success message
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert data["task_name"] == "test_task"
|
||||
# Should return success message
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert data["task_name"] == "test_task"
|
||||
|
||||
|
||||
@pytest.mark.api
|
||||
@@ -152,36 +157,37 @@ class TestStatsEndpoint:
|
||||
response = client.get("/stats")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict):
|
||||
def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
|
||||
"""Test that stats endpoint returns system metrics."""
|
||||
with patch('src.main.get_scheduler') as mock_scheduler, \
|
||||
patch('src.main.get_task_executor') as mock_executor:
|
||||
# Note: `/stats` also depends on get_scheduler via Depends(), which this
|
||||
# test does not override (out of this fix's measured scope — see
|
||||
# override_task_executor's docstring for why patch() cannot reach it).
|
||||
# It is not load-bearing here: the app's real scheduler is running by
|
||||
# the time TestClient's lifespan completes, so `sched.running` is True
|
||||
# without an override, same as test_health_endpoint_returns_healthy.
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchone.side_effect = [
|
||||
{"count": 3}, # enabled tasks
|
||||
{"count": 0}, # running tasks
|
||||
]
|
||||
mock_cursor.fetchall.return_value = [
|
||||
{"status": "success", "count": 10},
|
||||
{"status": "failed", "count": 1}
|
||||
]
|
||||
|
||||
mock_scheduler.return_value.running = True
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchone.side_effect = [
|
||||
{"count": 3}, # enabled tasks
|
||||
{"count": 0}, # running tasks
|
||||
]
|
||||
mock_cursor.fetchall.return_value = [
|
||||
{"status": "success", "count": 10},
|
||||
{"status": "failed", "count": 1}
|
||||
]
|
||||
response = client.get("/stats", headers=auth_headers)
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
response = client.get("/stats", headers=auth_headers)
|
||||
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert "scheduler_running" in data
|
||||
assert "tasks_enabled" in data
|
||||
assert "concurrent_limit" in data
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert "scheduler_running" in data
|
||||
assert "tasks_enabled" in data
|
||||
assert "concurrent_limit" in data
|
||||
|
||||
|
||||
@pytest.mark.api
|
||||
@@ -189,48 +195,46 @@ class TestStatsEndpoint:
|
||||
class TestExecutionHistoryEndpoint:
|
||||
"""Tests for /executions endpoint."""
|
||||
|
||||
def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict):
|
||||
def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
|
||||
"""Test that executions endpoint returns execution history."""
|
||||
with patch('src.main.get_task_executor') as mock_executor:
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchall.return_value = [
|
||||
{
|
||||
"id": 1,
|
||||
"task_name": "test_task",
|
||||
"status": "success",
|
||||
"duration_seconds": 5
|
||||
}
|
||||
]
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchall.return_value = [
|
||||
{
|
||||
"id": 1,
|
||||
"task_name": "test_task",
|
||||
"status": "success",
|
||||
"duration_seconds": 5
|
||||
}
|
||||
]
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
response = client.get("/executions", headers=auth_headers)
|
||||
response = client.get("/executions", headers=auth_headers)
|
||||
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert "executions" in data
|
||||
assert "count" in data
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
assert "executions" in data
|
||||
assert "count" in data
|
||||
|
||||
def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict):
|
||||
def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
|
||||
"""Test filtering executions by task name."""
|
||||
with patch('src.main.get_task_executor') as mock_executor:
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchall.return_value = []
|
||||
mock_conn = MagicMock()
|
||||
mock_cursor = MagicMock()
|
||||
mock_cursor.fetchall.return_value = []
|
||||
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
|
||||
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
|
||||
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
|
||||
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
|
||||
|
||||
response = client.get(
|
||||
"/executions?task_name=test_task&limit=10",
|
||||
headers=auth_headers
|
||||
)
|
||||
response = client.get(
|
||||
"/executions?task_name=test_task&limit=10",
|
||||
headers=auth_headers
|
||||
)
|
||||
|
||||
# Should execute query with filters
|
||||
assert mock_cursor.execute.called
|
||||
# Should execute query with filters
|
||||
assert mock_cursor.execute.called
|
||||
|
||||
Reference in New Issue
Block a user