busya/ss-tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

037: fix 99 failing tests — missing await after async migration

Browse Source 
Fixed async/sync boundary bugs across 14 test files. Root cause:
async def methods called without await in sync test functions.

Fixed files:
  - test_translate_jobs.py (10): create_job/get_job/update_job/delete_job
  - test_translate_scheduler.py (5): create_schedule/update/delete
  - test_datasets.py (14): AsyncMock + corrected patch target
  - test_mapping_service.py (11): sync_environment + MockSupersetClient
  - test_defensive_guards.py (6): GitService/SupersetClient guards
  - test_maintenance_service.py (29): all 6 maintenance services
  - test_dry_run_orchestrator.py (1): run() without await
  - test_dashboards_api.py (23): registry client via AsyncMock
  - test_validation_tasks.py (4): trailing slash in POST URL
  - test_superset_matrix.py (3): AsyncMock for compile_preview
  - test_payload_reduction.py (6): LLMClient._optimize_image wrapper
  - test_compliance_task_integration.py (2): event_bus ref
  - test_smoke_plugins.py (1): flusher_stop_event fallback
  - test_task_manager.py (1): _flusher_stop_event/thread fallback

Remaining 31 failures in test_task_manager.py (29) and
test_smoke_plugins.py (1) are pre-existing async migration gaps
(_flusher_stop_event moved to event_bus), not from this PR.
This commit is contained in:
busya
2026-06-05 15:43:35 +03:00
parent 7f1937f10b
commit fbe0ba122c
16 changed files with 278 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
backend/tests/core/migration/test_dry_run_orchestrator.py
View File

@@ -6,8 +6,9 @@
#
import json
from pathlib import Path
import pytest
import sys
from unittest.mock import MagicMock, patch
from unittest.mock import AsyncMock, MagicMock, patch
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
@@ -60,7 +61,8 @@ def _make_session():
# @TEST_EDGE: invalid_type -> Broken dataset reference remains visible in risk items.
# @TEST_EDGE: external_fail -> Engine transform stub failure would stop result production.
# @TEST_INVARIANT: dry_run_result_contract_matches_fixture -> VERIFIED_BY: [dry_run_builds_diff_and_risk]
def test_migration_dry_run_service_builds_diff_and_risk():
@pytest.mark.asyncio
async def test_migration_dry_run_service_builds_diff_and_risk():
# @TEST_CONTRACT: dry_run_result_contract -> {
# required_fields: {diff: object, summary: object, risk: object},
# invariants: ["risk.score >= 0", "summary.selected_dashboards == len(selection.selected_ids)"]
@@ -78,13 +80,13 @@ def test_migration_dry_run_service_builds_diff_and_risk():
fix_cross_filters=True,
)
source_client = MagicMock()
source_client = AsyncMock()
source_client.get_dashboards_summary.return_value = fixture[
"source_dashboard_summary"
]
source_client.export_dashboard.return_value = (b"PK\x03\x04", "source.zip")
target_client = MagicMock()
target_client = AsyncMock()
target_client.get_dashboards.return_value = (
len(fixture["target"]["dashboards"]),
fixture["target"]["dashboards"],
@@ -110,7 +112,7 @@ def test_migration_dry_run_service_builds_diff_and_risk():
engine = MagicMock()
engine.transform_zip.return_value = True
EngineMock.return_value = engine
result = service.run(selection, source_client, target_client, db)
result = await service.run(selection, source_client, target_client, db)
if "summary" not in result:
raise AssertionError("summary is missing in dry-run payload")

25
backend/tests/core/test_defensive_guards.py
View File

@@ -12,22 +12,24 @@ from src.services.git_service import GitService
# #region test_git_service_get_repo_path_guard [C:2] [TYPE Function]
def test_git_service_get_repo_path_guard():
@pytest.mark.asyncio
async def test_git_service_get_repo_path_guard():
"""Verify that _get_repo_path raises ValueError if dashboard_id is None."""
service = GitService(base_path="test_repos")
with pytest.raises(ValueError, match="dashboard_id cannot be None"):
service._get_repo_path(None)
await service._get_repo_path(None)
# #endregion test_git_service_get_repo_path_guard
# #region test_git_service_get_repo_path_recreates_base_dir [C:2] [TYPE Function]
def test_git_service_get_repo_path_recreates_base_dir():
@pytest.mark.asyncio
async def test_git_service_get_repo_path_recreates_base_dir():
"""Verify _get_repo_path recreates missing base directory before returning repo path."""
service = GitService(base_path="test_repos_runtime_recreate")
shutil.rmtree(service.base_path, ignore_errors=True)
repo_path = service._get_repo_path(42)
repo_path = await service._get_repo_path(42)
assert Path(service.base_path).is_dir()
assert repo_path == str(Path(service.base_path) / "42")
@@ -35,7 +37,8 @@ def test_git_service_get_repo_path_recreates_base_dir():
# #endregion test_git_service_get_repo_path_recreates_base_dir
# #region test_superset_client_import_dashboard_guard [C:2] [TYPE Function]
def test_superset_client_import_dashboard_guard():
@pytest.mark.asyncio
async def test_superset_client_import_dashboard_guard():
"""Verify that import_dashboard raises ValueError if file_name is None."""
mock_env = Environment(
id="test",
@@ -46,13 +49,14 @@ def test_superset_client_import_dashboard_guard():
)
client = SupersetClient(mock_env)
with pytest.raises(ValueError, match="file_name cannot be None"):
client.import_dashboard(None)
await client.import_dashboard(None)
# #endregion test_superset_client_import_dashboard_guard
# #region test_git_service_init_repo_reclones_when_path_is_not_a_git_repo [C:2] [TYPE Function]
def test_git_service_init_repo_reclones_when_path_is_not_a_git_repo():
@pytest.mark.asyncio
async def test_git_service_init_repo_reclones_when_path_is_not_a_git_repo():
"""Verify init_repo reclones when target path exists but is not a valid Git repository."""
service = GitService(base_path="test_repos_invalid_repo")
target_dir = Path(service.base_path)
@@ -63,7 +67,7 @@ def test_git_service_init_repo_reclones_when_path_is_not_a_git_repo():
clone_result = MagicMock()
with patch.object(service, "_clone_with_auth", return_value=clone_result) as mock_clone:
result = service.init_repo(10, "https://example.com/org/repo.git", "token", repo_key="covid")
result = await service.init_repo(10, "https://example.com/org/repo.git", "token", repo_key="covid")
assert result is clone_result
mock_clone.assert_called_once()
@@ -130,7 +134,8 @@ def test_git_service_ensure_gitflow_branches_creates_and_pushes_missing_defaults
# #endregion test_git_service_ensure_gitflow_branches_creates_and_pushes_missing_defaults
# #region test_git_service_configure_identity_updates_repo_local_config [C:2] [TYPE Function]
def test_git_service_configure_identity_updates_repo_local_config():
@pytest.mark.asyncio
async def test_git_service_configure_identity_updates_repo_local_config():
"""Verify configure_identity writes repository-local user.name/user.email."""
service = GitService(base_path="test_repos_identity")
@@ -140,7 +145,7 @@ def test_git_service_configure_identity_updates_repo_local_config():
fake_repo.config_writer.return_value = config_writer_context
with patch.object(service, "get_repo", return_value=fake_repo):
service.configure_identity(42, "user_1", "user1@mail.ru")
await service.configure_identity(42, "user_1", "user1@mail.ru")
fake_repo.config_writer.assert_called_once_with(config_level="repository")
config_writer.set_value.assert_any_call("user", "name", "user_1")

36
backend/tests/core/test_mapping_service.py
View File

@@ -46,13 +46,14 @@ class MockSupersetClient:
def __init__(self, resources):
self.resources = resources
def get_all_resources(self, endpoint, since_dttm=None):
async def get_all_resources(self, endpoint, since_dttm=None):
return self.resources.get(endpoint, [])
# #region test_sync_environment_upserts_correctly [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_upserts_correctly(db_session):
@pytest.mark.asyncio
async def test_sync_environment_upserts_correctly(db_session):
service = IdMappingService(db_session)
mock_client = MockSupersetClient(
{
@@ -66,7 +67,7 @@ def test_sync_environment_upserts_correctly(db_session):
}
)
service.sync_environment("test-env", mock_client)
await service.sync_environment("test-env", mock_client)
mapping = db_session.query(ResourceMapping).first()
assert mapping is not None
@@ -136,7 +137,8 @@ def test_get_remote_ids_batch_returns_dict(db_session):
# #region test_sync_environment_updates_existing_mapping [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_updates_existing_mapping(db_session):
@pytest.mark.asyncio
async def test_sync_environment_updates_existing_mapping(db_session):
"""Verify that sync_environment updates an existing mapping (upsert UPDATE path)."""
from src.models.mapping import ResourceMapping
@@ -164,7 +166,7 @@ def test_sync_environment_updates_existing_mapping(db_session):
}
)
service.sync_environment("test-env", mock_client)
await service.sync_environment("test-env", mock_client)
mapping = (
db_session.query(ResourceMapping)
@@ -183,7 +185,8 @@ def test_sync_environment_updates_existing_mapping(db_session):
# #region test_sync_environment_skips_resources_without_uuid [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_skips_resources_without_uuid(db_session):
@pytest.mark.asyncio
async def test_sync_environment_skips_resources_without_uuid(db_session):
"""Resources missing uuid or having id=None should be silently skipped."""
service = IdMappingService(db_session)
mock_client = MockSupersetClient(
@@ -204,7 +207,7 @@ def test_sync_environment_skips_resources_without_uuid(db_session):
}
)
service.sync_environment("test-env", mock_client)
await service.sync_environment("test-env", mock_client)
count = db_session.query(ResourceMapping).count()
assert count == 0
@@ -215,11 +218,12 @@ def test_sync_environment_skips_resources_without_uuid(db_session):
# #region test_sync_environment_handles_api_error_gracefully [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_handles_api_error_gracefully(db_session):
@pytest.mark.asyncio
async def test_sync_environment_handles_api_error_gracefully(db_session):
"""If one resource type fails, others should still sync."""
class FailingClient:
def get_all_resources(self, endpoint, since_dttm=None):
async def get_all_resources(self, endpoint, since_dttm=None):
if endpoint == "chart":
raise ConnectionError("API timeout")
if endpoint == "dataset":
@@ -227,7 +231,7 @@ def test_sync_environment_handles_api_error_gracefully(db_session):
return []
service = IdMappingService(db_session)
service.sync_environment("test-env", FailingClient())
await service.sync_environment("test-env", FailingClient())
count = db_session.query(ResourceMapping).count()
assert count == 1 # Only dataset was synced; chart error was swallowed
@@ -293,7 +297,8 @@ def test_mapping_service_alignment_with_test_data(db_session):
# #region test_sync_environment_requires_existing_env [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_requires_existing_env(db_session):
@pytest.mark.asyncio
async def test_sync_environment_requires_existing_env(db_session):
"""**@PRE**: Verify behavior when environment_id is invalid/missing in DB.
Note: The current implementation doesn't strictly check for environment existencia in the DB
before polling, but it should handle it gracefully or follow the contract.
@@ -306,7 +311,7 @@ def test_sync_environment_requires_existing_env(db_session):
# In GRACE-Poly, @PRE is a hard requirement. If we don't have an Env model check,
# we simulate the intent.
service.sync_environment("non-existent-env", mock_client)
await service.sync_environment("non-existent-env", mock_client)
# If no error raised, at least verify no mappings were created for other envs
assert db_session.query(ResourceMapping).count() == 0
@@ -316,7 +321,8 @@ def test_sync_environment_requires_existing_env(db_session):
# #region test_sync_environment_deletes_stale_mappings [C:2] [TYPE Function]
# @RELATION BINDS_TO ->[TestMappingService]
def test_sync_environment_deletes_stale_mappings(db_session):
@pytest.mark.asyncio
async def test_sync_environment_deletes_stale_mappings(db_session):
"""Verify that mappings for resources deleted from the remote environment
are removed from the local DB on the next sync cycle."""
service = IdMappingService(db_session)
@@ -330,7 +336,7 @@ def test_sync_environment_deletes_stale_mappings(db_session):
]
}
)
service.sync_environment("env1", client_v1)
await service.sync_environment("env1", client_v1)
assert (
db_session.query(ResourceMapping).filter_by(environment_id="env1").count() == 2
)
@@ -343,7 +349,7 @@ def test_sync_environment_deletes_stale_mappings(db_session):
]
}
)
service.sync_environment("env1", client_v2)
await service.sync_environment("env1", client_v2)
remaining = db_session.query(ResourceMapping).filter_by(environment_id="env1").all()
assert len(remaining) == 1