test: 7 agents — plugins ~158 tests, extractor 98-100%, dashboard routes 99-100%, git services 94-100%, services 94-100%, dataset_review 100%. Fix test_api_key_routes.py sys.modules pollution

backend/tests/core/test_mapping_service.py

@@ -8,6 +8,7 @@ from datetime import UTC, datetime
 from pathlib import Path
 import pytest
 import sys
+from unittest.mock import MagicMock, patch
 
 from sqlalchemy import create_engine, event
 from sqlalchemy.orm import sessionmaker
@@ -357,4 +358,219 @@ async def test_sync_environment_deletes_stale_mappings(db_session):
 
 
 # #endregion test_sync_environment_deletes_stale_mappings
+
+
+# #region test_start_scheduler_basic [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+@patch("src.core.mapping_service.seed_trace_id")
+@patch("src.core.mapping_service.BackgroundScheduler")
+def test_start_scheduler_basic(mock_scheduler_cls, mock_seed, db_session):
+    """start_scheduler adds job and starts scheduler."""
+    mock_scheduler = MagicMock()
+    mock_scheduler_cls.return_value = mock_scheduler
+    mock_scheduler.running = False
+
+    service = IdMappingService(db_session)
+    mock_factory = MagicMock(return_value=MagicMock())
+    service.start_scheduler("0 */5 * * *", ["env1", "env2"], mock_factory)
+
+    mock_scheduler.add_job.assert_called_once()
+    mock_scheduler.start.assert_called_once()
+
+
+# #endregion test_start_scheduler_basic
+
+
+# #region test_start_scheduler_update_existing [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+@patch("src.core.mapping_service.seed_trace_id")
+@patch("src.core.mapping_service.BackgroundScheduler")
+def test_start_scheduler_update_existing(mock_scheduler_cls, mock_seed, db_session):
+    """start_scheduler with existing job → updates, doesn't re-start scheduler."""
+    mock_scheduler = MagicMock()
+    mock_scheduler_cls.return_value = mock_scheduler
+    mock_scheduler.running = False  # Not running initially
+
+    service = IdMappingService(db_session)
+    mock_factory = MagicMock(return_value=MagicMock())
+    service.start_scheduler("0 */5 * * *", ["env1"], mock_factory)
+
+    # First call should start the scheduler (was not running)
+    mock_scheduler.start.assert_called_once()
+
+    # Second call with existing job — scheduler already running
+    mock_scheduler.running = True
+    service.start_scheduler("0 */10 * * *", ["env1"], mock_factory)
+
+    mock_scheduler.remove_job.assert_called_once()  # called because _sync_job exists
+    assert mock_scheduler.add_job.call_count == 2
+
+
+# #endregion test_start_scheduler_update_existing
+
+
+# #region test_get_remote_id_value_error [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+def test_get_remote_id_value_error(db_session):
+    """get_remote_id with non-integer remote_integer_id → None."""
+    service = IdMappingService(db_session)
+    mapping = ResourceMapping(
+        environment_id="test-env",
+        resource_type=ResourceType.DATASET,
+        uuid="uuid-bad",
+        remote_integer_id="not-a-number",
+    )
+    db_session.add(mapping)
+    db_session.commit()
+
+    result = service.get_remote_id("test-env", ResourceType.DATASET, "uuid-bad")
+    assert result is None
+
+
+# #endregion test_get_remote_id_value_error
+
+
+# #region test_get_remote_ids_batch_value_error [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+def test_get_remote_ids_batch_value_error(db_session):
+    """get_remote_ids_batch with non-integer remote_integer_id → skipped."""
+    service = IdMappingService(db_session)
+    m1 = ResourceMapping(
+        environment_id="test-env",
+        resource_type=ResourceType.DASHBOARD,
+        uuid="uuid-ok",
+        remote_integer_id="42",
+    )
+    m2 = ResourceMapping(
+        environment_id="test-env",
+        resource_type=ResourceType.DASHBOARD,
+        uuid="uuid-bad",
+        remote_integer_id="not-a-number",
+    )
+    db_session.add_all([m1, m2])
+    db_session.commit()
+
+    result = service.get_remote_ids_batch(
+        "test-env", ResourceType.DASHBOARD, ["uuid-ok", "uuid-bad"]
+    )
+    assert result["uuid-ok"] == 42
+    assert "uuid-bad" not in result
+
+
+# #endregion test_get_remote_ids_batch_value_error
+
+
+# #region test_sync_environment_incremental [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+@pytest.mark.asyncio
+async def test_sync_environment_incremental(db_session):
+    """incremental=True uses max_date filter for since_dttm."""
+    service = IdMappingService(db_session)
+
+    # Add an existing mapping with a last_synced_at value
+    from datetime import timedelta
+    old_mapping = ResourceMapping(
+        environment_id="test-env",
+        resource_type=ResourceType.CHART,
+        uuid="existing-uuid",
+        remote_integer_id="1",
+        resource_name="Old Chart",
+        last_synced_at=datetime.now(UTC) - timedelta(days=1),
+    )
+    db_session.add(old_mapping)
+    db_session.commit()
+
+    mock_client = MockSupersetClient(
+        {
+            "chart": [
+                {
+                    "id": 42,
+                    "uuid": "existing-uuid",
+                    "slice_name": "Updated Chart",
+                },
+                {
+                    "id": 99,
+                    "uuid": "new-uuid",
+                    "slice_name": "New Chart",
+                },
+            ]
+        }
+    )
+
+    await service.sync_environment("test-env", mock_client, incremental=True)
+
+    # Both existing and new should be synced
+    mappings = db_session.query(ResourceMapping).filter_by(environment_id="test-env").all()
+    assert len(mappings) == 2
+
+    # Existing mapping should be updated
+    updated = db_session.query(ResourceMapping).filter_by(uuid="existing-uuid").first()
+    assert updated.remote_integer_id == "42"
+    assert updated.resource_name == "Updated Chart"
+
+    # New mapping created
+    new_mapping = db_session.query(ResourceMapping).filter_by(uuid="new-uuid").first()
+    assert new_mapping is not None
+    assert new_mapping.remote_integer_id == "99"
+
+
+# #endregion test_sync_environment_incremental
+
+
+# #region test_sync_environment_no_new_data_incremental [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+@pytest.mark.asyncio
+async def test_sync_environment_no_new_data_incremental(db_session):
+    """incremental=True with no existing last_synced_at → since_dttm is None."""
+    service = IdMappingService(db_session)
+
+    mock_client = MockSupersetClient(
+        {
+            "chart": [
+                {"id": 1, "uuid": "aaa", "slice_name": "Chart A"},
+            ],
+            "dataset": [],
+            "dashboard": [],
+        }
+    )
+    await service.sync_environment("test-env", mock_client, incremental=True)
+
+    count = db_session.query(ResourceMapping).count()
+    assert count == 1
+    mapping = db_session.query(ResourceMapping).first()
+    assert mapping.uuid == "aaa"
+
+
+# #endregion test_sync_environment_no_new_data_incremental
+
+
+# #region test_sync_environment_critical_failure [C:2] [TYPE Function]
+# @RELATION  BINDS_TO ->[TestMappingService]
+@pytest.mark.asyncio
+async def test_sync_environment_critical_failure(db_session):
+    """Exception outside resource loop → rollback and re-raise."""
+    service = IdMappingService(db_session)
+
+    service = IdMappingService(db_session)
+    # Simulate a commit failure — raise before actual commit
+    def broken_commit():
+        raise Exception("commit failure")
+    db_session.commit = broken_commit
+
+    mock_client = MockSupersetClient(
+        {
+            "chart": [
+                {"id": 1, "uuid": "uuid-1", "slice_name": "Chart A"},
+            ]
+        }
+    )
+
+    with pytest.raises(Exception, match="commit failure"):
+        await service.sync_environment("test-env", mock_client)
+
+    # DB should have rolled back (rollback clears uncommitted changes)
+    assert db_session.query(ResourceMapping).count() == 0
+
+
+# #endregion test_sync_environment_critical_failure
 # #endregion TestMappingService