-
Notifications
You must be signed in to change notification settings - Fork 42
Integration testing framework #211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces an integration testing framework for Microsoft Agents applications. It provides infrastructure for setting up and testing agent applications with support for multiple hosting environments (initially aiohttp), sample applications, and client interactions.
Key changes:
- Integration testing framework with decorator-based test setup supporting service URLs, sample applications, or raw aiohttp apps
- Client libraries for sending messages to agents (
AgentClient) and receiving responses (ResponseClient) - Environment abstraction for hosting agents with an initial aiohttp implementation
- Sample application structure with a quickstart example demonstrating basic agent functionality
Reviewed Changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 28 comments.
Show a summary per file
| File | Description |
|---|---|
| dev/integration/src/tests/test_quickstart.py | Adds integration test for quickstart sample functionality |
| dev/integration/src/samples/quickstart_sample.py | Implements a basic agent sample with greeting and echo handlers |
| dev/integration/src/samples/init.py | Exports QuickstartSample class |
| dev/integration/src/environments/aiohttp_environment.py | Provides aiohttp-based hosting environment for agents |
| dev/integration/src/environments/init.py | Exports AiohttpEnvironment class |
| dev/integration/src/core/sample.py | Defines abstract base class for sample applications |
| dev/integration/src/core/integration_fixtures.py | Provides pytest fixtures for integration testing |
| dev/integration/src/core/integration.py | Implements decorator for integration test setup with multiple strategies |
| dev/integration/src/core/environment.py | Defines abstract base class for hosting environments |
| dev/integration/src/core/client/response_client.py | Implements client for receiving and managing agent responses |
| dev/integration/src/core/client/bot_response.py | Provides alternative bot response server implementation |
| dev/integration/src/core/client/auto_client.py | Contains commented-out auto-client functionality |
| dev/integration/src/core/client/agent_client.py | Implements client for sending activities to agents |
| dev/integration/src/core/client/init.py | Exports client classes |
| dev/integration/src/core/application_runner.py | Provides base class for running applications in separate threads |
| dev/integration/src/core/init.py | Exports core integration testing components |
| dev/integration/requirements.txt | Adds aioresponses dependency |
| dev/integration/env.TEMPLATE | Template for environment configuration |
Comments suppressed due to low confidence (1)
dev/integration/src/environments/aiohttp_environment.py:2
- Import of 'E' is not used.
from tkinter import E
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,74 @@ | |||
|
|
|||
| from tkinter import E | |||
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import of 'E' from 'tkinter'. This appears to be a leftover import that should be removed as tkinter is not used in this file.
| from tkinter import E |
| self._prev_stdout = None | ||
| self._service_endpoint = service_endpoint | ||
| self._activities_list = [] | ||
| self._activities_list_lock = [] |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lock should be a Lock() instance, not an empty list. This will cause the with statement on line 53 to fail since lists don't support the context manager protocol. Change to self._activities_list_lock = Lock().
| self._activities_list_lock = [] | |
| self._activities_list_lock = Lock() |
| async def test_quickstart_functionality(self, agent_client, response_client): | ||
| await agent_client.send("hi") | ||
| await asyncio.sleep(2) | ||
| response = (await response_client.pop())[0] |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ResponseClient class does not have a pop() method defined. This will cause an AttributeError at runtime.
| response = (await response_client.pop())[0] | |
| response = (await response_client.get())[0] |
| target_cls.setup_method = setup_method | ||
| target_cls.teardown_method = teardown_method | ||
|
|
||
| target_cls = Integration._with_response_server(target_cls, host_response) |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference to Integration class should be _Integration as that's the actual class name defined on line 16.
| def setup_method(self): | ||
| self._environment = environment_cls(sample_cls.get_config()) | ||
| await self._environment.__aenter__() | ||
|
|
||
| self._sample = sample_cls(self._environment) | ||
| await self._sample.__aenter__() | ||
|
|
||
| def teardown_method(self): |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing async keyword for setup_method. This function uses await so it must be defined as async def setup_method(self):.
| def setup_method(self): | |
| self._environment = environment_cls(sample_cls.get_config()) | |
| await self._environment.__aenter__() | |
| self._sample = sample_cls(self._environment) | |
| await self._sample.__aenter__() | |
| def teardown_method(self): | |
| async def setup_method(self): | |
| self._environment = environment_cls(sample_cls.get_config()) | |
| await self._environment.__aenter__() | |
| self._sample = sample_cls(self._environment) | |
| await self._sample.__aenter__() | |
| async def teardown_method(self): |
| import os | ||
| import json | ||
| import asyncio | ||
| from typing import Any, Optional, cast |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Any' is not used.
| from typing import Any, Optional, cast | |
| from typing import Optional, cast |
| import asyncio | ||
| import json | ||
| import sys | ||
| import threading |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'threading' is not used.
| import threading |
|
|
||
| from aiohttp import web, ClientSession | ||
| from aiohttp.web import Request, Response | ||
| import aiohttp_security |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'aiohttp_security' is not used.
| import aiohttp_security |
| @@ -0,0 +1,151 @@ | |||
| from typing import Optional, TypeVar, Union, Callable, Any | |||
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Union' is not used.
| from typing import Optional, TypeVar, Union, Callable, Any | |
| from typing import Optional, TypeVar, Callable, Any |
| @@ -0,0 +1,38 @@ | |||
| from typing import TypeVar, Any, AsyncGenerator, Callable | |||
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Any' is not used.
Import of 'TypeVar' is not used.
| from typing import TypeVar, Any, AsyncGenerator, Callable | |
| from typing import AsyncGenerator, Callable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 24 out of 29 changed files in this pull request and generated 17 comments.
Comments suppressed due to low confidence (5)
dev/integration/src/core/integration.py:72
- Variable setup_method is not used.
async def setup_method(self):
dev/integration/src/core/integration.py:79
- Variable teardown_method is not used.
async def teardown_method(self):
dev/integration/src/environments/aiohttp_environment.py:2
- Import of 'E' is not used.
from tkinter import E
dev/integration/src/tests/core/client/test_agent_client.py:10
- Keyword argument 'base_url' is not a supported parameter name of AgentClient.init.
return AgentClient(base_url="")
dev/integration/src/tests/core/client/test_agent_client.py:16
- Keyword argument 'base_url' is not a supported parameter name of AgentClient.init.
client = AgentClient(base_url="https://example.com")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,19 @@ | |||
| from re import A | |||
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import 'A' from the 're' module. This import should be removed as it serves no purpose.
| from re import A |
| async def setup_method(self): | ||
| self._environment = environment_cls(sample_cls.get_config()) | ||
| await self._environment.__aenter__() | ||
|
|
||
| self._sample = sample_cls(self._environment) | ||
| await self._sample.__aenter__() |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing assignment of setup_method and teardown_method to target_cls. The decorated methods should be assigned like target_cls.setup_method = setup_method and target_cls.teardown_method = teardown_method before calling _with_response_client, similar to the from_service_url method.
| def decorator(target_cls: T) -> T: | ||
|
|
||
| async def setup_method(self): | ||
| self._environment = environment_cls(sample_cls.get_config()) |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sample_cls.get_config() is an async method but is being called without await. This should be await sample_cls.get_config() to properly retrieve the configuration.
| self._environment = environment_cls(sample_cls.get_config()) | |
| self._environment = environment_cls(await sample_cls.get_config()) |
| authorization=self.authorization, | ||
| **self.agents_sdk_config |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference to undefined attribute self.agents_sdk_config. This attribute is not defined in the init_env method or anywhere in the class. Either define this attribute or remove the unpacking.
| authorization=self.authorization, | |
| **self.agents_sdk_config | |
| authorization=self.authorization |
| async def __aenter__(self) -> None: | ||
|
|
||
| if self._thread: | ||
| raise RuntimeError("Server is already running") | ||
|
|
||
| self._thread = Thread(target=self._start_server, daemon=True) | ||
| self._thread.start() | ||
|
|
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The __aenter__ method should return self (or the ApplicationRunner instance) to support the context manager protocol properly. Change return type to 'ApplicationRunner' and add return self at the end.
| async def __aenter__(self) -> None: | |
| if self._thread: | |
| raise RuntimeError("Server is already running") | |
| self._thread = Thread(target=self._start_server, daemon=True) | |
| self._thread.start() | |
| async def __aenter__(self) -> "ApplicationRunner": | |
| if self._thread: | |
| raise RuntimeError("Server is already running") | |
| self._thread = Thread(target=self._start_server, daemon=True) | |
| self._thread.start() | |
| return self |
|
|
||
| import aiohttp.web | ||
|
|
||
| from .application_runner import ApplicationRunner |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ApplicationRunner' is not used.
| from .application_runner import ApplicationRunner |
|
|
||
| from .application_runner import ApplicationRunner | ||
| from .environment import Environment | ||
| from .client import AgentClient, ResponseClient |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'AgentClient' is not used.
| from .client import AgentClient, ResponseClient | |
| from .client import ResponseClient |
| Sample | ||
| ) | ||
|
|
||
| from ._common import SimpleRunner, OtherSimpleRunner |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'OtherSimpleRunner' is not used.
| from ._common import SimpleRunner, OtherSimpleRunner | |
| from ._common import SimpleRunner |
| @@ -0,0 +1,7 @@ | |||
| import pytest | |||
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.
| import pytest |
| from src.core import integration | ||
|
|
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'integration' is not used.
| from src.core import integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 24 out of 29 changed files in this pull request and generated 17 comments.
Comments suppressed due to low confidence (1)
dev/integration/src/environments/aiohttp_environment.py:2
- Import of 'E' is not used.
from tkinter import E
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,74 @@ | |||
|
|
|||
| from tkinter import E | |||
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import E from the tkinter module should be removed. This import is not used anywhere in the code and tkinter is unrelated to the aiohttp environment implementation.
| from tkinter import E |
| app = {} | ||
| runner = SimpleRunner(app) | ||
| async with runner as r: | ||
| sleep(0.1) |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using synchronous sleep() in an async test function blocks the event loop. Replace with await asyncio.sleep(0.1) to properly yield control in the async context.
| app = {} | ||
| runner = SimpleRunner(app) | ||
| async with runner as r: | ||
| sleep(0.1) |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using synchronous sleep() in an async test function blocks the event loop. Replace with await asyncio.sleep(0.1) to properly yield control in the async context.
| app = {} | ||
| runner = SimpleRunner(app) | ||
| async with runner as r: | ||
| sleep(0.1) |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using synchronous sleep() in an async test function blocks the event loop. Replace with await asyncio.sleep(0.1) to properly yield control in the async context.
| async def setup_method(self): | ||
| self._environment = environment_cls(sample_cls.get_config()) | ||
| await self._environment.__aenter__() | ||
|
|
||
| self._sample = sample_cls(self._environment) | ||
| await self._sample.__aenter__() |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sample_cls.get_config() is an async classmethod (as seen in sample.py), but it's being called without await. This should be await sample_cls.get_config() to properly retrieve the configuration.
| import sys | ||
| from io import StringIO | ||
| from typing import Optional | ||
| from threading import Lock |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Lock' is not used.
|
|
||
| @pytest.fixture | ||
| def agent_client(self) -> AgentClient: | ||
| return AgentClient(base_url="") |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword argument 'base_url' is not a supported parameter name of AgentClient.init.
| async def context_manager(): | ||
| with aioresponses() as mocked: | ||
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | ||
| client = AgentClient(base_url="https://example.com") |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword argument 'base_url' is not a supported parameter name of AgentClient.init.
| return ResponseClient(base_url="") | ||
|
|
||
| @pytest.fixture | ||
| def service_url(self): | ||
| with aioresponses() as mocked: | ||
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | ||
| client = ResponseClient(base_url="https://example.com") |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword argument 'base_url' is not a supported parameter name of ResponseClient.init.
| return ResponseClient(base_url="") | |
| @pytest.fixture | |
| def service_url(self): | |
| with aioresponses() as mocked: | |
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | |
| client = ResponseClient(base_url="https://example.com") | |
| return ResponseClient("") | |
| @pytest.fixture | |
| def service_url(self): | |
| with aioresponses() as mocked: | |
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | |
| client = ResponseClient("https://example.com") |
| return ResponseClient(base_url="") | ||
|
|
||
| @pytest.fixture | ||
| def service_url(self): | ||
| with aioresponses() as mocked: | ||
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | ||
| client = ResponseClient(base_url="https://example.com") |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword argument 'base_url' is not a supported parameter name of ResponseClient.init.
| return ResponseClient(base_url="") | |
| @pytest.fixture | |
| def service_url(self): | |
| with aioresponses() as mocked: | |
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | |
| client = ResponseClient(base_url="https://example.com") | |
| return ResponseClient("") | |
| @pytest.fixture | |
| def service_url(self): | |
| with aioresponses() as mocked: | |
| mocked.get("https://example.com/service-url", payload={"serviceUrl": "https://service.example.com"}) | |
| client = ResponseClient("https://example.com") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 25 out of 30 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
dev/integration/src/environments/aiohttp_environment.py:2
- Import of 'E' is not used.
from tkinter import E
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._service_url = service_url | ||
|
|
||
| async def teardown_method(self): | ||
| self._service_url = service_url |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The teardown_method is redundantly reassigning the service_url that was already set in setup_method. This assignment serves no purpose in cleanup and should be removed.
| self._service_url = service_url | |
| pass |
| yield response_client | ||
|
|
||
| @pytest.fixture | ||
| def create_response_client(self) -> Callable[None, ResponseClient]: |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type annotation Callable[None, ResponseClient] is incorrect. For a callable with no arguments, use Callable[[], ResponseClient] instead. None is not a valid argument specification.
| def create_response_client(self) -> Callable[None, ResponseClient]: | |
| def create_response_client(self) -> Callable[[], ResponseClient]: |
| app = {} | ||
| runner = OtherSimpleRunner(app) | ||
| async with runner as r: | ||
| sleep(0.1) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using synchronous sleep() in an async test blocks the event loop. Replace with await asyncio.sleep(0.1) to properly yield control in async context.
| app = {} | ||
| runner = SimpleRunner(app) | ||
| async with runner: | ||
| sleep(0.1) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using synchronous sleep() in an async test blocks the event loop. Replace with await asyncio.sleep(0.1) to properly yield control in async context.
| async def _handle_conversation(self, request: Request) -> Response: | ||
| try: | ||
| body = await request.text() | ||
| activity = Activity.model_validate(body) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The body from request.text() is a string, but Activity.model_validate() expects a dict. This should be Activity.model_validate_json(body) or parse the JSON first with json.loads(body) then validate.
| return cast(Activity, activity_or_text) | ||
|
|
||
| async def send_activity(self, activity_or_text: Activity | str, timeout: Optional[float] = None) -> str: | ||
| timeout = timeout or self._default_timeout |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable timeout is not used.
| return content | ||
|
|
||
| async def send_expect_replies(self, activity_or_text: Activity | str, timeout: Optional[float] = None) -> list[Activity]: | ||
| timeout = timeout or self._default_timeout |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable timeout is not used.
| body = await request.text() | ||
| activity = Activity.model_validate(body) | ||
|
|
||
| conversation_id = activity.conversation.id if activity.conversation else None |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable conversation_id is not used.
| conversation_id = activity.conversation.id if activity.conversation else None |
| @@ -0,0 +1,65 @@ | |||
| import json | |||
| from contextlib import contextmanager | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'contextmanager' is not used.
| from contextlib import contextmanager |
| import re | ||
|
|
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 're' is not used.
| import re |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 25 out of 30 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (2)
dev/integration/src/core/aiohttp/aiohttp_environment.py:2
- Import of 'E' is not used.
from tkinter import E
dev/integration/src/core/aiohttp/aiohttp_environment.py:3
- Import of 'run_app' is not used.
from aiohttp.web import Request, Response, Application, run_app
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,115 @@ | |||
| from typing import Optional | |||
| from typing import Optional | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate import statement for typing.Optional. Remove the duplicate on line 2.
| from typing import Optional |
| APP["agent_app"] = self.agent_application | ||
| APP["adapter"] = self.adapter | ||
|
|
||
| return AiohttpRunner(APP) No newline at end of file |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import for AiohttpRunner. The class is used but not imported. Add from .aiohttp_runner import AiohttpRunner to the imports.
|
|
||
| async def __aenter__(self): | ||
| if self._server_thread: | ||
| raise RuntimeError("ResponseClient is already running.") |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message references 'ResponseClient' but this is the AiohttpRunner class. The message should be 'AiohttpRunner is already running.' or 'Server is already running.'
| raise RuntimeError("ResponseClient is already running.") | |
| raise RuntimeError("AiohttpRunner is already running.") |
|
|
||
| async def _stop_server(self): | ||
| if not self._server_thread: | ||
| raise RuntimeError("ResponseClient is not running.") |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message references 'ResponseClient' but this is the AiohttpRunner class. The message should be 'AiohttpRunner is not running.' or 'Server is not running.'
|
|
||
| async def __aexit__(self, exc_type, exc, tb): | ||
| if not self._server_thread: | ||
| raise RuntimeError("ResponseClient is not running.") |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message references 'ResponseClient' but this is the AiohttpRunner class. The message should be 'AiohttpRunner is not running.' or 'Server is not running.'
| @@ -0,0 +1,59 @@ | |||
|
|
|||
| from tkinter import E | |||
| from aiohttp.web import Request, Response, Application, run_app | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import run_app. This import is not used in the file and should be removed.
| from aiohttp.web import Request, Response, Application, run_app | |
| from aiohttp.web import Request, Response, Application |
| @@ -0,0 +1,50 @@ | |||
| import pytest | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.
| import pytest |
| @@ -0,0 +1,50 @@ | |||
| import pytest | |||
| from typing import Optional, TypeVar, Union, Callable, Any | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Any' is not used.
Import of 'Union' is not used.
| from typing import Optional, TypeVar, Union, Callable, Any | |
| from typing import Optional, TypeVar, Callable |
|
|
||
| import aiohttp.web | ||
|
|
||
| from .application_runner import ApplicationRunner |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ApplicationRunner' is not used.
| from .application_runner import ApplicationRunner |
|
|
||
| from .application_runner import ApplicationRunner | ||
| from .environment import Environment | ||
| from .client import AgentClient, ResponseClient |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'AgentClient' is not used.
Import of 'ResponseClient' is not used.
| from .client import AgentClient, ResponseClient |
| await asyncio.sleep(0.1) # Simulate processing delay | ||
| return Response(status=200, text="Activity received") | ||
| except Exception as e: | ||
| return Response(status=500, text=str(e)) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, the detailed error message must not be returned to the external user. Instead, we should log the error server-side for developers/administrators and send a generic error message in the HTTP response.
The best way to do this in the provided snippet is:
- Use the standard
loggingmodule to log the exception, including a stack trace, in the exception handler. - Return a generic message like "An internal server error occurred" in the Response, with status 500.
- No changes are required except within the
_handle_conversationmethod and adding aloggingimport at the top if not already present.
-
Copy modified line R5 -
Copy modified lines R82-R83
| @@ -2,6 +2,7 @@ | ||
|
|
||
| import sys | ||
| from io import StringIO | ||
| import logging | ||
| from typing import Optional | ||
| from threading import Lock, Thread, Event | ||
| import asyncio | ||
| @@ -78,7 +79,8 @@ | ||
| await asyncio.sleep(0.1) # Simulate processing delay | ||
| return Response(status=200, text="Activity received") | ||
| except Exception as e: | ||
| return Response(status=500, text=str(e)) | ||
| logging.exception("Unhandled exception in _handle_conversation") | ||
| return Response(status=500, text="An internal server error occurred.") | ||
|
|
||
| async def _handle_streamed_activity( | ||
| self, activity: Activity, *args, **kwargs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 25 out of 30 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (2)
dev/integration/src/core/aiohttp/aiohttp_environment.py:2
- Import of 'E' is not used.
from aiohttp.web import Request, Response, Application, run_app
dev/integration/src/core/aiohttp/aiohttp_environment.py:3
- Import of 'run_app' is not used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._server_thread = Thread( | ||
| target=lambda: asyncio.run(self._start_server()), daemon=True | ||
| ) | ||
| self._server_thread.start() |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message refers to 'ResponseClient' but this is the AiohttpRunner class. Change the message to 'AiohttpRunner is already running.' or 'Server is already running.'
|
|
||
| self._shutdown_event.clear() |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-raising the exception without modification is unnecessary. Either remove the try-except block entirely or handle the exception meaningfully (e.g., logging, cleanup, or error transformation).
| from ..core import integration | ||
|
|
||
|
|
||
| @integration(service_url="http://localhost:3978/api/messages") |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @integration decorator is called with 'service_url' as a parameter, but the IntegrationFixtures class expects 'messaging_endpoint' to be set. Based on the code in integration.py line 125, only 'messaging_endpoint' parameter is handled, not 'service_url'. This will likely cause the decorator to raise a ValueError.
| @integration(service_url="http://localhost:3978/api/messages") | |
| @integration(messaging_endpoint="http://localhost:3978/api/messages") |
| activity = self._to_activity(activity_or_text) | ||
| activity.delivery_mode = DeliveryModes.expect_replies | ||
|
|
||
| content = await self.send_request(activity) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable timeout is not used.
| with aioresponses() as mocked: | ||
|
|
||
| def callback(url, **kwargs): | ||
| a = requests.post( |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable a is not used.
| a = requests.post( | |
| requests.post( |
| from typing import ( | ||
| Optional, | ||
| TypeVar, | ||
| Union, | ||
| Callable, | ||
| Any, | ||
| AsyncGenerator, | ||
| ) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Union' is not used.
|
|
||
| import aiohttp.web | ||
|
|
||
| from .application_runner import ApplicationRunner |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ApplicationRunner' is not used.
| from .application_runner import ApplicationRunner |
| assert activities[0].type == "message" | ||
| assert activities[0].text == "Hello, World!" |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'assert' statement contains an expression which may have side effects.
| assert activities[0].type == "message" | |
| assert activities[0].text == "Hello, World!" | |
| activity = activities[0] | |
| assert activity.type == "message" | |
| assert activity.text == "Hello, World!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 27 out of 33 changed files in this pull request and generated 16 comments.
Comments suppressed due to low confidence (8)
dev/integration/src/core/client/agent_client.py:1
- Import of 'os' is not used.
import os
dev/integration/src/core/aiohttp/aiohttp_environment.py:2
- Import of 'run_app' is not used.
from aiohttp.web import Request, Response, Application, run_app
dev/integration/src/core/integration.py:10
- Import of 'Union' is not used.
from typing import (
Optional,
TypeVar,
Union,
Callable,
Any,
AsyncGenerator,
)
dev/integration/src/core/client/response_client.py:6
- Import of 'Thread' is not used.
Import of 'Event' is not used.
from threading import Lock, Thread, Event
dev/integration/src/tests/test_framework/core/client/test_agent_client.py:2
- Import of 'contextmanager' is not used.
from contextlib import contextmanager
dev/integration/src/tests/test_framework/core/client/test_agent_client.py:3
- Import of 're' is not used.
import re
dev/integration/src/tests/test_framework/core/test_integration_from_service_url.py:4
- Import of 'copy' is not used.
dev/integration/src/tests/test_quickstart.py:2 - Import of 'asyncio' is not used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,12 @@ | |||
| import pytest | |||
| import asyncio | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'asyncio' module is imported but not used in this file. Remove this unused import.
| import pytest | ||
| import asyncio | ||
| import requests | ||
| from copy import copy |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'copy' function is imported but not used in this file. Remove this unused import.
dev/integration/env.TEMPLATE
Outdated
| aioresponses | ||
| microsoft-agents-activity | ||
| microsoft-agents-hosting-core No newline at end of file |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file appears to be misnamed or incorrectly formatted. Based on the naming pattern 'env.TEMPLATE' and comparison with 'dev/integration/src/tests/env.TEMPLATE', this should likely contain environment variable templates (e.g., KEY=value format) rather than package names. The content looks like it belongs in a requirements file instead.
| aioresponses | |
| microsoft-agents-activity | |
| microsoft-agents-hosting-core | |
| # Example environment variable template | |
| # Copy this file to .env and fill in the values as needed | |
| API_KEY= | |
| DATABASE_URL= | |
| DEBUG=false |
| def create_runner(self) -> ApplicationRunner: | ||
| """Create an application runner for the environment.""" |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The abstract method signature for 'create_runner' in the base class returns 'ApplicationRunner' without parameters, but the concrete implementation in 'AiohttpEnvironment' requires 'host' and 'port' parameters. This creates an inconsistent interface. Consider adding these parameters to the base class signature or using a configuration object.
| def create_runner(self) -> ApplicationRunner: | |
| """Create an application runner for the environment.""" | |
| def create_runner(self, host: str, port: int) -> ApplicationRunner: | |
| """Create an application runner for the environment. | |
| Args: | |
| host (str): The host address to bind the runner. | |
| port (int): The port number to bind the runner. | |
| """ |
| @@ -0,0 +1,130 @@ | |||
| import os | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'os' module is imported but not used anywhere in this file. Remove this unused import.
| import os |
| with aioresponses() as mocked: | ||
|
|
||
| def callback(url, **kwargs): | ||
| a = requests.post( |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable a is not used.
| **self.config | ||
| ) | ||
|
|
||
| def create_runner(self, host: str, port: int) -> ApplicationRunner: |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires 3 positional arguments, whereas overridden Environment.create_runner requires 1.
| @@ -0,0 +1,58 @@ | |||
| from tkinter import E | |||
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'E' is not used.
| from tkinter import E |
|
|
||
| import aiohttp.web | ||
|
|
||
| from .application_runner import ApplicationRunner |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ApplicationRunner' is not used.
| from .application_runner import ApplicationRunner |
| assert activities[0].type == "message" | ||
| assert activities[0].text == "Hello, World!" | ||
|
|
||
| assert (await response_client.pop()) == [] |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'assert' statement contains an expression which may have side effects.
| assert (await response_client.pop()) == [] | |
| popped_activities = await response_client.pop() | |
| assert popped_activities == [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 27 out of 33 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
dev/integration/src/core/aiohttp/aiohttp_environment.py:1
- Import of 'E' is not used.
from tkinter import E
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import pytest | ||
| import asyncio | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate import of AiohttpEnvironment. Remove the duplicate occurrence from the import statement.
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment | |
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment |
|
|
||
| async def _stop_server(self): | ||
| if not self._server_thread: | ||
| raise RuntimeError("ResponseClient is not running.") |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message refers to 'ResponseClient' but this is the AiohttpRunner class. The message should say 'AiohttpRunner is not running.' or 'Server is not running.'
| conversation_id = ( | ||
| activity.conversation.id if activity.conversation else None | ||
| ) |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable conversation_id is not used.
| conversation_id = ( | |
| activity.conversation.id if activity.conversation else None | |
| ) | |
| # Removed unused variable 'conversation_id' |
| from typing import ( | ||
| Optional, | ||
| TypeVar, | ||
| Union, | ||
| Callable, | ||
| Any, | ||
| AsyncGenerator, | ||
| ) |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Union' is not used.
| import aiohttp.web | ||
| from dotenv import load_dotenv | ||
|
|
||
| from .application_runner import ApplicationRunner |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ApplicationRunner' is not used.
| from .application_runner import ApplicationRunner |
| @@ -0,0 +1,22 @@ | |||
| import pytest | |||
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.
| import pytest |
| import pytest | ||
| import asyncio | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'integration' is not used.
Import of 'IntegrationFixtures' is not used.
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment | |
| from src.core import AiohttpEnvironment, AiohttpEnvironment |
| import pytest | ||
| import asyncio | ||
| import requests | ||
| from copy import copy |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'copy' is not used.
| @@ -0,0 +1,12 @@ | |||
| import pytest | |||
| import asyncio | |||
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'asyncio' is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 26 out of 31 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
dev/integration/src/core/aiohttp/aiohttp_environment.py:1
- Import of 'E' is not used.
from tkinter import E
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| json=activity.model_dump(by_alias=True, exclude_unset=True, exclude_none=True, mode="json"), | ||
| ) as response: | ||
| content = await response.text() | ||
| breakpoint() |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A breakpoint() statement is left in the production code. This should be removed before merging as it will halt execution in production environments.
| breakpoint() |
| import pytest | ||
| import asyncio | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment, AgentClient |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AiohttpEnvironment is imported twice. Remove the duplicate import.
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment, AgentClient | |
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AgentClient |
| timeout = timeout or self._default_timeout | ||
| activity = self._to_activity(activity_or_text) | ||
| activity.delivery_mode = DeliveryModes.expect_replies | ||
| activity.service_url = activity.service_url or "http://localhost" # temporary fix |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded fallback URL 'http://localhost' with a comment indicating this is a 'temporary fix'. Consider making this configurable or documenting why this default is necessary. If this is truly temporary, create a TODO or track it for proper resolution.
| async def send_activity( | ||
| self, activity_or_text: Activity | str, sleep: float = 0, timeout: Optional[float] = None | ||
| ) -> str: | ||
| timeout = timeout or self._default_timeout |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable timeout is not used.
| print(f"Server running at http://{host}:{port}/api/messages") | ||
| while True: | ||
|
|
||
| res = await client.send_expect_replies("Hello, Agent!") |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable res is not used.
| res = await client.send_expect_replies("Hello, Agent!") | |
| await client.send_expect_replies("Hello, Agent!") |
| **self.config | ||
| ) | ||
|
|
||
| def create_runner(self, host: str, port: int) -> ApplicationRunner: |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method requires 3 positional arguments, whereas overridden Environment.create_runner requires 1.
| def create_runner(self, host: str, port: int) -> ApplicationRunner: | |
| def create_runner(self, runner_config: dict) -> ApplicationRunner: | |
| host = runner_config.get("host", "127.0.0.1") | |
| port = runner_config.get("port", 8080) |
| @@ -0,0 +1,48 @@ | |||
| import os | |||
| import pytest | |||
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.
| import pytest |
| import pytest | ||
| import asyncio | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment, AgentClient |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'integration' is not used.
Import of 'IntegrationFixtures' is not used.
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment, AiohttpEnvironment, AgentClient | |
| from src.core import AiohttpEnvironment, AiohttpEnvironment, AgentClient |
| import pytest | ||
| import asyncio | ||
| import requests | ||
| from copy import copy |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'copy' is not used.
| from copy import copy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 29 out of 36 changed files in this pull request and generated 14 comments.
Comments suppressed due to low confidence (1)
dev/integration/src/tests/manual_test.py:2
- Import of 'pytest' is not used.
import pytest
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| async def __aenter__(self): | ||
| if self._server_thread: | ||
| raise RuntimeError("ResponseClient is already running.") |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message mentions 'ResponseClient' but this is the AiohttpRunner class. The message should say 'AiohttpRunner is already running.' or 'Server is already running.'
| raise RuntimeError("ResponseClient is already running.") | |
| raise RuntimeError("Server is already running.") |
| @@ -0,0 +1,41 @@ | |||
| import os | |||
| import pytest | |||
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import. The pytest module is imported but never used in this file.
| import pytest |
| @integration() | ||
| class TestFoundation(IntegrationFixtures): | ||
|
|
||
| def load_activity(self, activity_name) -> Activity: |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import for return type annotation. The method uses Activity as a return type but it's not imported in this file. Add Activity to the imports from microsoft_agents.activity.
| assert result is not None | ||
| last = result[-1] | ||
| assert last.type == ActivityTypes.message | ||
| assert last.text.lower() == "you said: {activity.text}".lower() |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String formatting issue. The assertion uses a literal string {activity.text} instead of an f-string. Change to f\"you said: {activity.text}\" to properly interpolate the activity.text variable.
| assert last.text.lower() == "you said: {activity.text}".lower() | |
| assert last.text.lower() == f"you said: {activity.text}".lower() |
|
|
||
| def load_activity(channel: str, name: str) -> Activity: | ||
|
|
||
| with open("./dev/integration/src/tests/integration/foundational/activities/{}/{}.json".format(channel, name), "r") as f: |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Use f-string for better readability. Replace .format(channel, name) with an f-string: f\"./dev/integration/src/tests/integration/foundational/activities/{channel}/{name}.json\".
| with open("./dev/integration/src/tests/integration/foundational/activities/{}/{}.json".format(channel, name), "r") as f: | |
| with open(f"./dev/integration/src/tests/integration/foundational/activities/{channel}/{name}.json", "r") as f: |
| import asyncio | ||
|
|
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'asyncio' is not used.
| import asyncio |
| ActivityTypes, | ||
| ) | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'AiohttpEnvironment' is not used.
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment | |
| from src.core import integration, IntegrationFixtures |
| ) | ||
|
|
||
| from src.core import integration, IntegrationFixtures, AiohttpEnvironment | ||
| from src.samples import QuickstartSample |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'QuickstartSample' is not used.
| from src.samples import QuickstartSample |
| from microsoft_agents.activity import ( | ||
| Activity, | ||
| ActivityTypes, | ||
| DeliveryModes, | ||
| ConversationAccount, | ||
| ChannelAccount, | ||
| ) |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'ActivityTypes' is not used.
| assert activities[0].type == "message" | ||
| assert activities[0].text == "Hello, World!" | ||
|
|
||
| assert (await response_client.pop()) == [] |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'assert' statement contains an expression which may have side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 30 out of 38 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
dev/integration/src/core/aiohttp/aiohttp_environment.py:1
- Import of 'E' is not used.
from tkinter import E
dev/integration/src/tests/manual_test.py:2
- Import of 'pytest' is not used.
import pytest
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from microsoft_agents.activity import Activity | ||
|
|
||
| def load_activity(channel: str, name: str) -> Activity: | ||
|
|
||
| with open("./dev/integration/src/tests/integration/foundational/activities/{}/{}.json".format(channel, name), "r") as f: |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded relative path may fail depending on execution context. Consider using pathlib.Path(__file__).parent to construct a path relative to the current file, or use os.path.join() with a base directory constant to make the path more robust.
| from microsoft_agents.activity import Activity | |
| def load_activity(channel: str, name: str) -> Activity: | |
| with open("./dev/integration/src/tests/integration/foundational/activities/{}/{}.json".format(channel, name), "r") as f: | |
| from pathlib import Path | |
| from microsoft_agents.activity import Activity | |
| def load_activity(channel: str, name: str) -> Activity: | |
| activity_path = Path(__file__).parent / "activities" / channel / f"{name}.json" | |
| with open(activity_path, "r") as f: |
This pull request introduces a new integration testing framework for agent-based applications, adding core abstractions, client utilities, and environment setup. It establishes a modular structure for integration tests, including agent and response clients, application runners, and environment definitions. The changes also add requirements for new dependencies and set up import/export patterns for the new modules.
Key changes include:
Core abstractions and environment setup:
Environmentabstract base class to define the structure and initialization of integration test environments, supporting agent applications, storage, adapters, and connection management. (dev/integration/src/core/environment.py)ApplicationRunnerabstract base class for managing the lifecycle (start/stop) of agent applications in integration tests, with support for asynchronous context management. (dev/integration/src/core/application_runner.py)Client utilities:
AgentClientfor sending authenticated activities to agent endpoints, including support for access token acquisition and handling both simple and expect-replies message flows. (dev/integration/src/core/client/agent_client.py)ResponseClientto simulate a server receiving activities, supporting activity validation and response logic, with a stub for streamed activity handling. (dev/integration/src/core/client/response_client.py)BotResponsefor simulating bot endpoints, including activity storage, streaming support, and a simple aiohttp-based web server. (dev/integration/src/core/client/bot_response.py)Integration framework and utilities:
integrationdecorator/factory, providing flexible integration test setup using service URLs, sample/environment classes, or aiohttp apps, and supporting response server hosting. (dev/integration/src/core/integration.py)core/__init__.pyandcore/client/__init__.pyfor consistent import patterns. (dev/integration/src/core/__init__.py,dev/integration/src/core/client/__init__.py) [1] [2]Dependency and environment configuration:
aioresponses,microsoft-agents-activity,microsoft-agents-hosting-core) to the integration environment and requirements. (dev/integration/env.TEMPLATE,dev/integration/requirements.txt) [1] [2]These changes lay the groundwork for robust, modular, and reusable integration tests for agent-based systems.