chore: Dev merge to Main#242
Open
Shreyas-Microsoft wants to merge 29 commits into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only shows Coverage badge + TOTAL row + test summary in PR comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Generate a text-format summary with only the TOTAL line from coverage.xml. Use pytest-coverage-path instead of pytest-xml-coverage-path so the Coverage Report dropdown shows only TOTAL row without per-file listing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix test_returns_false_for_none to call is_valid_uuid directly - Fix inaccurate comment about non-existent env file path - Make test_get_service_status_not_initialized deterministic with mock - Exclude test files from processor coverage (tool.coverage.run.omit) - Regenerate uv.lock with pytest-asyncio Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use pytest-xml-coverage-path with coverage-path-prefix so the action correctly resolves XML paths (relative to subdir) against PR diff paths. Combined with report-only-changed-files: true, when no source files are in the diff this shows: badge + Coverage Report dropdown with TOTAL row + 'No files were changed' note + test summary table. Matches the format used in Multi-Agent-Custom-Automation-Engine PR #961. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Processor: 75% -> 87.44% (+199 tests across 5 files) - queue_service.py: 35% -> 86% - groupchat_orchestrator.py: 25% -> ~70%+ (helpers fully covered) - mcp_mermaid.py: 52% -> 99% - application_base.py: 0% -> 100% - main.py: 0% -> 94% Backend-api: 82.27% -> 93.28% (+125 tests across 7 files) - SKLogicBase.py: 0% -> 93% - blob/helper.py: 68% -> 99% - blob/async_helper.py: 72% -> 97% - blob/config.py: 70% -> 100% - shared_config.py: 67% -> 100% - services/interfaces.py: 74% -> 100% - file_repository.py / process_repository.py: 67% -> 100% CI: add --cov-fail-under=82 to both backend and processor jobs in .github/workflows/test.yml so the pipeline fails when coverage drops below 82%. Backend pyproject.toml: add [tool.coverage.run] omit list for parity with processor so local pytest runs (which use pytest.ini's --cov=src) do not inflate coverage with test-file lines. No production code modified, no coverage exclusions added beyond the test directory, all 1077 prior + new tests pass on both services. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve unresolved Copilot review threads on PR #211, scoped strictly to test files and CI workflow config (no production source changes per reviewer-imposed scope). Test fixes: - src/processor/src/tests/unit/libs/agent_framework/test_agent_speaking_capture.py Remove dead-code branch from `_run` helper (`...if False else asyncio.run`) and simplify to a single `asyncio.run(coro)` call. - src/processor/src/tests/unit/utils/test_credential_util.py Replace `cred.__class__.__name__ = "AzureCliCredential"` mutation on a MagicMock (which leaks the mutated class name globally and can affect other tests) with an instance of a small dummy class named `AzureCliCredential` defined inside the test. - src/backend-api/src/tests/base/test_kernel_agent.py Expand the rationale comment around the `SKBaseModel` test-only shim, drop any pre-cached `libs.base.kernel_agent` module from sys.modules so the shim is picked up on first import, and document that fixing the empty `libs/base/SKBase.py` source file is intentionally out of scope for this PR (the file and `kernel_agent.py` are orphan modules with no production importers, so the missing symbol does not surface at runtime today). Workflow alignment (.github/workflows/test.yml): - actions/checkout@v5 -> @v4 (matches every other workflow in the repo) - actions/setup-python@v6 -> @v5 (matches pylint.yml, validate-bicep-params.yml) False positives (replied on the threads, not changed): - Coverage-XML filename rewrite for both jobs is correct as-is. coverage.py emits filenames relative to the `--cov=PATH` source root, not the cwd: backend XML uses paths like `libs/base/kernel_agent.py` (relative to src/app), processor XML uses `libs/__init__.py` (relative to src), so prefixing with `src/backend-api/src/app/` and `src/processor/src/` respectively produces correct repo-root paths with no duplication. Verification: - backend-api: 588 tests passing, 93.28% coverage (gate 82%). - processor: 812 tests passing, 87.44% coverage (gate 82%). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- test_migration_processor_extras.py: replace MagicMock() + `del obj.dict` with a small dummy class exposing only `model_dump` so the v2 branch is exercised without depending on Mock's deletion semantics. - test_sk_logic_base.py: capture the original `libs.base.SKBase.SKBaseModel` before patching with our extra-allow stub and restore it in `teardown_module` so the patch no longer leaks across test modules. Both fixes are confined to test files. Coverage remains above the 82% gate (backend 93.28%, processor 87.44%). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the missing second blank line after `teardown_module` so the next module-level `import` statement satisfies E305 (expected 2 blank lines after class or function definition). Caught by the repo's flake8 job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lib, mermaid, uuid) - urllib3: 2.6.3 → 2.7.0 (High - header forwarding & decompression-bomb bypass) - python-multipart: 0.0.26/0.0.22 → 0.0.27 (High - DoS via unbounded headers) - authlib: 1.7.0 → 1.7.2 (Medium - OIDC open redirect) - mermaid: 11.13.0/11.14.0 → 11.15.0 (Medium - CSS/HTML injection) - uuid: 11.0.x → 11.1.1+ (Medium - buffer bounds check) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
test: Add unit tests for backend-api
The previous commit accidentally regenerated infra/main.json with bicep 0.42.1 because the local CLI was out of date. Re-built with bicep 0.43.8.12551 to match the version on main while keeping the cosmos enableAnalyticalStorage fix intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…torage fix: remove enableAnalyticalStorage on Cosmos DB account creation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands test coverage across both the backend API and the processor component, and updates CI to run each component’s test suite with coverage reporting (including async-test support).
Changes:
- Added extensive new unit tests for the processor (utils, services, orchestrators, migration processor, and agent framework helpers).
- Added broad backend API test coverage (routers, services, repositories, models, Azure helpers, and bootstrap code), plus updated dev dependencies/lockfile for async testing.
- Enhanced GitHub Actions workflow to run backend + processor tests separately with coverage thresholds and XML path normalization.
Reviewed changes
Copilot reviewed 65 out of 73 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/test.yml |
Adds processor test job, expands workflow triggers, updates dependency installs and coverage settings. |
infra/main.json |
Updates generated template hash and reorders dependsOn entries. |
infra/modules/cosmosDb.bicep |
Removes enableAnalyticalStorage from Cosmos DB account module params. |
src/backend-api/pyproject.toml |
Adds pytest-asyncio to dev dependencies and sets coverage omit rules. |
src/backend-api/uv.lock |
Updates lockfile to include pytest-asyncio. |
src/backend-api/src/tests/application/test_application.py |
Tests backend Application bootstrap/service registration/router inclusion. |
src/backend-api/src/tests/application/test_application_context_extra.py |
Adds deeper AppContext lifecycle and scope tests (backend). |
src/backend-api/src/tests/azure/test_app_configuration_helper.py |
Tests App Configuration helper initialization and env population. |
src/backend-api/src/tests/base/test_kernel_agent.py |
Adds tests for kernel_agent behaviors using test-time stubs/mocks. |
src/backend-api/src/tests/base/test_sk_logic_base.py |
Adds tests for SKLogicBase with module-load shims and abstract branch coverage. |
src/backend-api/src/tests/base/test_typed_fastapi.py |
Tests TypedFastAPI/AppContext protocol helpers. |
src/backend-api/src/tests/model/test_entities.py |
Tests entity/model defaults and serialization behavior. |
src/backend-api/src/tests/model/test_router_models.py |
Tests Pydantic router models and base64 queue message helpers. |
src/backend-api/src/tests/repositories/test_process_status_repository.py |
Tests repository helpers and process/agent status rendering utilities. |
src/backend-api/src/tests/repositories/test_repositories_extra.py |
Adds extra repository initialization/update coverage for file/process repos. |
src/backend-api/src/tests/routers/test_http_probes.py |
Tests probe endpoints (/, /health, /startup). |
src/backend-api/src/tests/routers/test_router_debug.py |
Tests debug config endpoint behavior. |
src/backend-api/src/tests/routers/test_router_files.py |
Tests file upload route behavior, including error paths and filename sanitization. |
src/backend-api/src/tests/sas/storage/blob/test_async_helper_extra.py |
Adds coverage for SAS URL generation and inner failure branches (async blob helper). |
src/backend-api/src/tests/sas/storage/blob/test_config.py |
Tests blob helper config defaults/env overrides/module helpers. |
src/backend-api/src/tests/sas/storage/test_shared_config.py |
Tests shared storage config defaults/env overrides/module-level helpers. |
src/backend-api/src/tests/services/test_auth.py |
Tests auth helpers and tenant extraction behavior. |
src/backend-api/src/tests/services/test_implementations.py |
Tests concrete service implementations (logger/data/http client). |
src/backend-api/src/tests/services/test_input_validation.py |
Tests UUID validation helper. |
src/backend-api/src/tests/services/test_interfaces.py |
Adds coverage for ABC interface method bodies and abstract instantiation. |
src/backend-api/src/tests/services/test_process_services.py |
Tests ProcessService behaviors with async context manager mocks. |
src/backend-api/src/tests/test_app_init.py |
Tests app/__init__.py sys.path bootstrap behavior. |
src/backend-api/src/tests/test_main.py |
Tests main.get_app() singleton caching. |
src/processor/pyproject.toml |
Adds coverage omit rules for processor tests. |
src/processor/src/tests/unit/libs/agent_framework/test_agent_builder.py |
Tests AgentBuilder fluent API and agent creation wiring. |
src/processor/src/tests/unit/libs/agent_framework/test_agent_framework_helper.py |
Tests AgentFrameworkHelper initialization and client creation behavior. |
src/processor/src/tests/unit/libs/agent_framework/test_agent_framework_settings.py |
Tests service discovery and env-file loading behavior. |
src/processor/src/tests/unit/libs/agent_framework/test_agent_speaking_capture.py |
Tests middleware capture/filters/callback behavior. |
src/processor/src/tests/unit/libs/agent_framework/test_azure_openai_response_retry_extras.py |
Adds extra coverage for retry/trim utility internals. |
src/processor/src/tests/unit/libs/agent_framework/test_azure_openai_response_retry_utils.py |
Updates expectations/comments for retry/trim behavior. |
src/processor/src/tests/unit/libs/agent_framework/test_cosmos_checkpoint_storage.py |
Tests Cosmos checkpoint models/storage wrapper. |
src/processor/src/tests/unit/libs/agent_framework/test_mem0_async_memory.py |
Tests lazy initialization and default config of Mem0 async memory manager. |
src/processor/src/tests/unit/libs/agent_framework/test_middlewares_extras.py |
Adds tests for additional middleware behaviors. |
src/processor/src/tests/unit/libs/application/test_application_context_extras.py |
Adds extra AppContext DI/lifetime/shutdown coverage (processor). |
src/processor/src/tests/unit/libs/base/test_application_base_init.py |
Tests ApplicationBase init behavior with patched external deps. |
src/processor/src/tests/unit/libs/base/test_orchestrator_base.py |
Tests OrchestratorBase helpers, client caching, and telemetry hooks. |
src/processor/src/tests/unit/libs/reporting/test_migration_report_generator.py |
Tests migration report collector/generator classification and report output. |
src/processor/src/tests/unit/services/test_queue_service_helpers.py |
Tests queue service helpers, message parsing, and control/cleanup behavior. |
src/processor/src/tests/unit/steps/analysis/test_analysis_executor.py |
Adjusts monkeypatching of text2art to be tolerant of missing import. |
src/processor/src/tests/unit/steps/documentation/test_documentation_orchestrator_execute.py |
Tests DocumentationOrchestrator agent prep and forwarding hooks. |
src/processor/src/tests/unit/steps/test_migration_processor_extras.py |
Tests exception formatting and memory store creation behavior. |
src/processor/src/tests/unit/steps/test_migration_processor_run.py |
Tests MigrationProcessor event-stream handling, telemetry updates, and termination paths. |
src/processor/src/tests/unit/steps/test_step_orchestrator_agent_infos.py |
Tests agent info preparation and forwarding hooks for orchestrators. |
src/processor/src/tests/unit/test_main.py |
Tests processor main entry wiring without invoking full init. |
src/processor/src/tests/unit/test_main_service.py |
Tests QueueMigrationServiceApp control API/service lifecycle behaviors. |
src/processor/src/tests/unit/utils/test_console_util.py |
Tests console formatting/styling helpers. |
src/processor/src/tests/unit/utils/test_credential_util.py |
Tests credential selection and bearer token provider helpers. |
src/processor/src/tests/unit/utils/test_logging_utils.py |
Tests logging helper behaviors and error detail formatting. |
src/processor/src/tests/unit/utils/test_prompt_util.py |
Tests template rendering utilities (string/file, loops, substitutions). |
src/processor/src/tests/unit/utils/test_security_policy_evidence.py |
Tests blob evidence collection logic, skipping rules, and signal detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+80
to
+91
| def _wire_credential(svc_cls, *, account_key=None, account_name="myacct", | ||
| credential_cls_name="DefaultAzureCredential"): | ||
| svc = _wire(svc_cls) | ||
| svc.account_name = account_name | ||
| if credential_cls_name == "AccountKey": | ||
| cred = MagicMock() | ||
| cred.account_key = account_key | ||
| type(cred).__name__ = "StorageSharedKeyCredential" | ||
| else: | ||
| cred = MagicMock(spec=[]) | ||
| type(cred).__name__ = credential_cls_name | ||
| svc.credential = cred |
… lockfile The package-lock.json declared name 'processor' but package.json had no matching metadata, causing npm to rewrite the lockfile on every install. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ct.toml Use == instead of >= for urllib3 and authlib overrides to keep lockfile regeneration deterministic and consistent with the rest of the project. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: upgrade vulnerable dependencies (urllib3, python-multipart, auth…
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>
4 tasks
test: resolve PR #242 review comments (CodeQL + Copilot)
| # Licensed under the MIT License. | ||
|
|
||
| import asyncio | ||
| from unittest.mock import AsyncMock, MagicMock, patch |
| import asyncio | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| import pytest |
|
|
||
| import pytest | ||
|
|
||
| from libs.agent_framework import cosmos_checkpoint_storage as ccs |
Comment on lines
+20
to
+25
| from libs.agent_framework.groupchat_orchestrator import ( | ||
| AgentResponse, | ||
| AgentResponseStream, | ||
| GroupChatOrchestrator, | ||
| OrchestrationResult, | ||
| ) |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json |
| import json | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest |
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| from unittest.mock import MagicMock, patch |
| import asyncio | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest |
| from types import SimpleNamespace | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| import pytest |
| @@ -0,0 +1,108 @@ | |||
| """Tests for libs/sas/storage/shared_config.py.""" | |||
|
|
|||
| import pytest | |||
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 79 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- src/frontend/package-lock.json: Language not supported
- src/processor/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/test.yml:29
- Same as the push trigger:
pull_request.pathsshould includesrc/backend-api/uv.lockandsrc/processor/uv.lockso dependency-only changes still execute the test workflow.
- 'src/backend-api/**/*.py'
- 'src/backend-api/pyproject.toml'
- 'src/backend-api/pytest.ini'
- 'src/processor/**/*.py'
- 'src/processor/pyproject.toml'
.github/workflows/test.yml:132
- Same issue as the backend job:
pip install -e .will likely fail forsrc/processorbecause itspyproject.tomlalso lacks[build-system]and there’s nosetup.py. Useuv sync --frozen(as in the processor Dockerfile) or add a build backend to make editable installs valid.
run: |
python -m pip install --upgrade pip
cd src/processor
pip install -e .
pip install pytest pytest-cov pytest-asyncio
Comment on lines
9
to
+13
| - 'src/backend-api/**/*.py' | ||
| - 'src/backend-api/pyproject.toml' | ||
| - 'src/backend-api/pytest.ini' | ||
| - 'src/processor/**/*.py' | ||
| - 'src/processor/pyproject.toml' |
Comment on lines
51
to
+55
| run: | | ||
| python -m pip install --upgrade pip | ||
| cd src/backend-api | ||
| pip install -e . | ||
| pip install pytest pytest-cov | ||
| pip install pytest pytest-cov pytest-asyncio |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request expands and improves the testing and CI infrastructure for both the backend API and the new processor component. It introduces new tests, updates dependencies, and enhances the GitHub Actions workflow to provide better coverage reporting and support for asynchronous tests.
CI/CD Workflow Improvements:
src/processor/**/*.pyandsrc/processor/pyproject.tomlto the paths that trigger the test workflow, ensuring processor code changes are properly tested. [1] [2]processor_testsjob in.github/workflows/test.ymlto run tests and coverage for the processor, including logic to skip tests if no test files are found.actions/checkoutandactions/setup-pythonto v4 and v5 respectively for improved stability and compatibility.pytest-asyncioto support async tests, and ensured it's included in both backend and processor test environments. [1] [2]--cov-fail-under=82) and logic to prefix coverage XML filenames with repo-root paths for more accurate reporting in both backend and processor jobs. [1] [2]Testing Enhancements:
test_application.py: Verifies application bootstrap, service registration, and router inclusion.test_application_context_extra.py: Thoroughly tests service lifetimes, async/sync service registration, scoping, and shutdown logic in the application context.test_app_configuration_helper.py: Tests Azure App Configuration helper initialization, environment variable population, and client delegation.Backend and Infra Configuration:
pytest-asyncioand configured coverage to omit test files from reports.infra/main.jsonand reordered dependencies for private DNS zones for clarity. [1] [2]enableAnalyticalStorage: truefrom Cosmos DB Bicep module to align with new requirements or defaults.These changes collectively improve code quality, test coverage, and reliability of the CI pipeline for both backend and processor components.
Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information