Skip to content

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

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

Add test coverage for non-success HTTP response scenarios in SendChatHistoryAsync#161
pontemonti merged 3 commits intousers/johanb/RealTimeThreatProtectionfrom
copilot/sub-pr-148-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 31, 2025

Addresses missing test coverage for error handling in McpToolServerConfigurationService.SendChatHistoryAsync. The implementation logs errors for non-successful HTTP responses but lacked tests verifying this behavior.

Changes

  • Added parameterized test covering HTTP error status codes: 400, 401, 403, 404, 500
  • Verifies error logging includes correct status code text and method completes without throwing
[Theory]
[InlineData(HttpStatusCode.BadRequest, "BadRequest", "Invalid request format")]
[InlineData(HttpStatusCode.Unauthorized, "Unauthorized", "Unauthorized access")]
[InlineData(HttpStatusCode.Forbidden, "Forbidden", "Access forbidden")]
[InlineData(HttpStatusCode.NotFound, "NotFound", "Endpoint not found")]
[InlineData(HttpStatusCode.InternalServerError, "InternalServerError", "Internal server error")]
public async Task SendChatHistoryAsync_NonSuccessStatusCode_LogsErrorAndCompletesWithoutException(
    HttpStatusCode statusCode, string expectedStatusText, string errorMessage)

Tests validate the error handling code at lines 138-142 that logs non-successful responses without propagating exceptions.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Dec 31, 2025
Copilot AI and others added 2 commits December 31, 2025 06:36
…403, 404, 500)

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Copilot AI changed the title [WIP] Update API implementation based on feedback for chat history Add test coverage for non-success HTTP response scenarios in SendChatHistoryAsync Dec 31, 2025
Copilot AI requested a review from pontemonti December 31, 2025 06:39
@pontemonti pontemonti marked this pull request as ready for review December 31, 2025 16:15
@pontemonti pontemonti requested a review from a team as a code owner December 31, 2025 16:15
@pontemonti pontemonti merged commit 393db58 into users/johanb/RealTimeThreatProtection Dec 31, 2025
1 check passed
@pontemonti pontemonti deleted the copilot/sub-pr-148-again branch December 31, 2025 16:16
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