Skip to content

Add comprehensive test coverage for SendChatHistoryAsync parameter validation#151

Merged
pontemonti merged 4 commits intousers/johanb/RealTimeThreatProtectionfrom
copilot/sub-pr-148-again
Dec 31, 2025
Merged

Add comprehensive test coverage for SendChatHistoryAsync parameter validation#151
pontemonti merged 4 commits intousers/johanb/RealTimeThreatProtectionfrom
copilot/sub-pr-148-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 31, 2025

The original PR #148 had insufficient test coverage for SendChatHistoryAsync - tests only validated null parameters but not the Activity property validation logic or exception handling behavior.

Changes

  • Added Activity property validation tests: 3 new tests covering missing Conversation, Id, and Text properties with distinct mock scenarios
  • Each test validates a specific error path:
    • MissingConversationId: Activity has valid Id/Text but null Conversation
    • MissingMessageId: Activity has valid Conversation/Text but null Id
    • MissingUserMessage: Activity has valid Conversation/Id but null Text
var activityMock = new Mock<IActivity>();
activityMock.Setup(a => a.Id).Returns((string)null!); // Missing message ID
activityMock.Setup(a => a.Text).Returns("Hello");
activityMock.Setup(a => a.Conversation).Returns(conversationAccount);

All 8 tests pass (4 existing + 3 new + 1 unrelated).

Testing Limitations

HTTP behavior testing (success/error responses, timeouts, network errors) requires integration tests - the method uses RuntimeUtility.GetDefaultHttpClient() which creates HttpClient internally, making it impractical to mock in unit tests.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits December 31, 2025 05:27
- Added tests for missing Activity properties (conversation ID, message ID, user message)
- All tests validate expected exception handling
- Tests verify that InvalidOperationException is thrown with appropriate error messages
- Tests cover both method overloads (with and without ToolOptions)
- Existing null parameter validation tests remain unchanged

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
- SendChatHistoryAsync_MissingConversationId now properly mocks a null Conversation property
- SendChatHistoryAsync_MissingMessageId now mocks null Id with valid Conversation
- SendChatHistoryAsync_MissingUserMessage now mocks null Text with valid Conversation
- Each test validates a distinct error path through the method
- All 8 tests pass successfully

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
- Remove excessive blank lines between methods
- Use explicit cast syntax (Type)null! instead of null as Type!
- Maintain consistent null handling across all test methods
- All 8 tests continue to pass

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Copilot AI changed the title [WIP] Update API implementation for sending chat history Add comprehensive test coverage for SendChatHistoryAsync parameter validation Dec 31, 2025
Copilot AI requested a review from pontemonti December 31, 2025 05:36
@pontemonti pontemonti marked this pull request as ready for review December 31, 2025 05:51
@pontemonti pontemonti requested a review from a team as a code owner December 31, 2025 05:52
@pontemonti pontemonti merged commit ca2beef into users/johanb/RealTimeThreatProtection Dec 31, 2025
1 check passed
@pontemonti pontemonti deleted the copilot/sub-pr-148-again branch December 31, 2025 05:52
pontemonti added a commit that referenced this pull request Jan 6, 2026
…148)

* Initial implementation of API to send chat history to MCP platform.

* Address PR feedback to clarify documentation.

* Address PR feedback about documentation

* Address PR feedback related to references to instance fields

* Address PR feedback related to documentation

* Address PR feedback about required vs optional properties

* Update src/Tooling/Core/Services/McpToolServerConfigurationService.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Core/Utils/Utility.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Models/ChatHistoryMessageTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Use short-form Encoding.UTF8 instead of fully qualified name (#153)

* Initial plan

* Use Encoding.UTF8 instead of fully qualified name

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Document exception handling behavior in SendChatHistoryAsync XML comments (#152)

* Initial plan

* Update XML documentation for SendChatHistoryAsync methods to clarify exception handling

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Refactor HttpClient to use IHttpClientFactory to prevent socket exhaustion (#150)

* Initial plan

* Refactor HttpClient usage to use IHttpClientFactory

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Use RuntimeUtility.GetDefaultHttpClient with IHttpClientFactory

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Remove unnecessary using statement

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add comprehensive test coverage for SendChatHistoryAsync parameter validation (#151)

* Initial plan

* Add comprehensive tests for SendChatHistoryAsync method

- Added tests for missing Activity properties (conversation ID, message ID, user message)
- All tests validate expected exception handling
- Tests verify that InvalidOperationException is thrown with appropriate error messages
- Tests cover both method overloads (with and without ToolOptions)
- Existing null parameter validation tests remain unchanged

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix tests to properly mock distinct Activity property scenarios

- SendChatHistoryAsync_MissingConversationId now properly mocks a null Conversation property
- SendChatHistoryAsync_MissingMessageId now mocks null Id with valid Conversation
- SendChatHistoryAsync_MissingUserMessage now mocks null Text with valid Conversation
- Each test validates a distinct error path through the method
- All 8 tests pass successfully

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Address code review feedback: improve code style

- Remove excessive blank lines between methods
- Use explicit cast syntax (Type)null! instead of null as Type!
- Maintain consistent null handling across all test methods
- All 8 tests continue to pass

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tooling/Extensions/AgentFramework/Agent365AgentFrameworkSdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix missing IHttpClientFactory parameter in test constructor calls (#154)

* Initial plan

* Fix test failures by adding missing IHttpClientFactory parameter to McpToolServerConfigurationService constructor calls

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Remove exception throw for unsuccessful HTTP responses in SendChatHistoryAsync (#156)

* Initial plan

* Remove exception throw for failed HTTP requests in SendChatHistoryAsync

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add happy path test coverage for SendChatHistoryAsync HTTP POST (#157)

* Initial plan

* Add comprehensive happy path tests for SendChatHistoryAsync method

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix flaky test behavior by using fixed timestamps instead of DateTimeOffset.UtcNow

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tooling/Extensions/AzureAIFoundry/Agent365AzureAIFoundrySdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Extensions/SemanticKernel/Agent365SemanticKernelSdkUserAgentConfiguration.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient (#162)

* Initial plan

* Make IHttpClientFactory required in GetDefaultHttpClient

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Add test coverage for non-success HTTP response scenarios in SendChatHistoryAsync (#161)

* Initial plan

* Add test coverage for non-success HTTP response scenarios (400, 401, 403, 404, 500)

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Refactor HTTP error status tests to use parameterized theory approach

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Tooling/Core/Models/ChatMessageRequest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix HttpResponseMessage disposal in SendChatHistoryAsync (#163)

* Initial plan

* Wrap HttpResponseMessage in using statement for proper disposal

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Fix resource leak from undisposed HttpResponseMessage in test mocks (#164)

* Initial plan

* Fix HttpResponseMessage disposal issues by using lambda factory pattern

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix HttpResponseMessage disposal race condition in async mock test (#165)

* Initial plan

* Fix HttpResponseMessage disposal race condition in test

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Redesign SendChatHistoryAsync to return a result object.

* Add CancellationToken argument

---------

Co-authored-by: Johan Broberg <johanb@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants