Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 29, 2026

Addresses unresolved feedback from #1191: the test class was overriding LoggerFactory to use LogLevel.Trace for all tests, but Debug should remain the default since Trace is too verbose for general test output.

Changes

  • Remove class-level LoggerFactory override from constructor (now uses base class default of Debug)
  • Update SendMessageAsync_Should_Log_At_Trace_Level and ReadMessagesAsync_Should_Log_Received_At_Trace_Level to create local trace-level logger factories for their specific needs
// Each trace-level test now creates its own logger infrastructure
var mockLoggerProvider = new MockLoggerProvider();
using var traceLoggerFactory = LoggerFactory.Create(builder =>
{
    builder.AddProvider(XunitLoggerProvider);
    builder.AddProvider(mockLoggerProvider);
    builder.SetMinimumLevel(LogLevel.Trace);
});
Original prompt

Address this comment that wasn't handled before the PR was merged: @modelcontextprotocol/csharp-sdk/pull/1191


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

…g level

Address feedback from PR #1191: The class-level LoggerFactory override was removed
from the constructor (now uses base class default of Debug). The trace-level
logging tests now create their own local MockLoggerProvider and trace-level
LoggerFactory to properly test Trace-level logging without affecting other tests.

Co-authored-by: stephentoub <[email protected]>
Copilot AI changed the title [WIP] Address comment from merged PR 1191 Fix StdioServerTransportTests to use Debug logging level instead of Trace Jan 29, 2026
Copilot AI requested a review from stephentoub January 29, 2026 20:15
@stephentoub stephentoub marked this pull request as ready for review January 29, 2026 20:20
@stephentoub stephentoub requested a review from halter73 January 29, 2026 20:20
@stephentoub stephentoub merged commit 482e311 into main Jan 29, 2026
10 of 17 checks passed
@stephentoub stephentoub deleted the copilot/address-comment-from-pr-1191 branch January 29, 2026 22:20
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.

3 participants