Skip to content

Commit 222b22c

Browse files
test: resolve PR #242 review comments
Address CodeQL (github-code-quality) and Copilot review comments on PR #242: CodeQL: unused imports / variables - test_queue_service_internals.py: drop redundant `tp = _task_param()` in test_output_cleanup_refuses_broad_prefix; drop unused `_no_op_worker` in test_start_service_runs_and_completes. - test_application_base_init.py: drop unused `import inspect` and unused `app` local in test_init_loads_app_config_when_url_set. - test_agent_speaking_capture.py: drop unused `datetime`/`MagicMock` imports. - test_agent_telemetry.py: drop unused `from datetime import UTC, datetime` (only the literal string ` UTC` is used, not the imports). - test_agent_telemetry_records.py: drop unused `import pytest`. - test_config.py (backend SAS blob): drop unused `pytest` import and unused `default_config` symbol from the import list (still referenced via `blob_config_module.default_config`). Copilot review: order-dependent MagicMock leak - test_async_helper_extra.py and test_helper_extra.py: replace `type(MagicMock()).__name__ = ...` mutation in `_wire_credential` with a dedicated dynamically-created stub class per credential type. Mutating `__name__` on the shared MagicMock class globally renamed that class for any later test that introspected `type(...)`, making the suite order-dependent/flaky. The new stubs scope each credential class name to its own type while preserving `hasattr(cred, 'account_key')` semantics for both account-key and AAD code paths. Verification - Backend unit tests: 585 passing (1 unrelated environmental failure against example.azconfig.io), 93.28% coverage. - Processor unit tests: 812 passing, 87.44% coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5b6f6fa commit 222b22c

8 files changed

Lines changed: 19 additions & 23 deletions

File tree

src/backend-api/src/tests/sas/storage/blob/test_async_helper_extra.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,16 @@ def _wire_credential(svc_cls, *, account_key=None, account_name="myacct",
8181
credential_cls_name="DefaultAzureCredential"):
8282
svc = _wire(svc_cls)
8383
svc.account_name = account_name
84+
# Use a dedicated stub class per credential type so that ``type(cred).__name__``
85+
# reflects the desired credential class without mutating the shared ``MagicMock``
86+
# class metadata (which would leak across tests and make order-dependent failures).
8487
if credential_cls_name == "AccountKey":
85-
cred = MagicMock()
88+
cred_cls = type("StorageSharedKeyCredential", (), {})
89+
cred = cred_cls()
8690
cred.account_key = account_key
87-
type(cred).__name__ = "StorageSharedKeyCredential"
8891
else:
89-
cred = MagicMock(spec=[])
90-
type(cred).__name__ = credential_cls_name
92+
cred_cls = type(credential_cls_name, (), {})
93+
cred = cred_cls()
9194
svc.credential = cred
9295
return svc
9396

src/backend-api/src/tests/sas/storage/blob/test_config.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
"""Tests for libs/sas/storage/blob/config.py."""
22

3-
import pytest
4-
53
from libs.sas.storage.blob import config as blob_config_module
64
from libs.sas.storage.blob.config import (
75
BlobHelperConfig,
86
create_config,
9-
default_config,
107
get_config,
118
set_config,
129
)

src/backend-api/src/tests/sas/storage/blob/test_helper_extra.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,21 @@ def test_delete_container_blobs_present_message_no_force(self, blob_service_mock
6666

6767
def _wire_credential(blob_service_mock, *, account_key=None, account_name="myacct",
6868
credential_cls_name="DefaultAzureCredential"):
69-
"""Make the helper's blob_service_client respond like an Azure SDK client."""
69+
"""Make the helper's blob_service_client respond like an Azure SDK client.
70+
71+
Uses a dedicated stub class per credential type so that ``type(cred).__name__``
72+
reflects the desired credential class without mutating the shared ``MagicMock``
73+
class metadata (which would leak across tests).
74+
"""
7075
h = _make_helper(blob_service_mock)
7176
h.blob_service_client.account_name = account_name
7277
if credential_cls_name == "AccountKey":
73-
cred = MagicMock()
78+
cred_cls = type("StorageSharedKeyCredential", (), {})
79+
cred = cred_cls()
7480
cred.account_key = account_key
75-
type(cred).__name__ = "StorageSharedKeyCredential"
7681
else:
77-
cred = MagicMock(spec=[])
78-
type(cred).__name__ = credential_cls_name
82+
cred_cls = type(credential_cls_name, (), {})
83+
cred = cred_cls()
7984
h.blob_service_client.credential = cred
8085
return h
8186

src/processor/src/tests/unit/libs/agent_framework/test_agent_speaking_capture.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
# Licensed under the MIT License.
33

44
import asyncio
5-
from datetime import datetime
65
from types import SimpleNamespace
7-
from unittest.mock import AsyncMock, MagicMock
6+
from unittest.mock import AsyncMock
87

98
from libs.agent_framework.agent_speaking_capture import AgentSpeakingCaptureMiddleware
109

src/processor/src/tests/unit/libs/base/test_application_base_init.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
from __future__ import annotations
77

8-
import inspect
98
from unittest.mock import MagicMock, patch
109

1110
import pytest
@@ -71,7 +70,7 @@ def test_init_loads_app_config_when_url_set(self, patches_chain, tmp_path):
7170
env_file = tmp_path / ".env"
7271
env_file.write_text("X=1")
7372
_App = _build_concrete()
74-
app = _App(env_file_path=str(env_file))
73+
_App(env_file_path=str(env_file))
7574
patches_chain["ac_helper"].assert_called_once()
7675
# The helper instance had its method invoked
7776
helper_instance = patches_chain["ac_helper"].return_value

src/processor/src/tests/unit/services/test_queue_service_internals.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,6 @@ def test_output_cleanup_no_account(self):
714714

715715
def test_output_cleanup_refuses_broad_prefix(self):
716716
s = _service()
717-
tp = _task_param()
718717
# Force output_file_folder to broad path matching "<pid>"
719718
tp = Analysis_TaskParam(
720719
process_id="p1",
@@ -784,9 +783,6 @@ def test_start_service_runs_and_completes(self):
784783
s._ensure_queues_exist = AsyncMock()
785784
s._control_watcher_loop = AsyncMock()
786785

787-
async def _no_op_worker(self_, worker_id):
788-
return None
789-
790786
# Patch worker loop to immediate return
791787
s._worker_loop = lambda wid: asyncio.sleep(0) # type: ignore[assignment]
792788
_run(s.start_service())

src/processor/src/tests/unit/utils/test_agent_telemetry.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
# Licensed under the MIT License.
33

44
import asyncio
5-
from datetime import UTC, datetime
65
from types import SimpleNamespace
76
from unittest.mock import AsyncMock, MagicMock, patch
87

src/processor/src/tests/unit/utils/test_agent_telemetry_records.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
import asyncio
99
from unittest.mock import AsyncMock, MagicMock, patch
1010

11-
import pytest
12-
1311
import utils.agent_telemetry as at
1412
from utils.agent_telemetry import ProcessStatus, TelemetryManager
1513

0 commit comments

Comments
 (0)