Skip to content

Commit 60ea96a

Browse files
halter73halter73
authored
Add XunitLoggerProvider (#81)
* Add XunitLoggerProvider * Avoid logging to stale ITestOutputHelper in fixture disposal * Use LoggedTest base class everywhere an ILoggerFactory is used
1 parent 729e688 commit 60ea96a

File tree

11 files changed

+178
-86
lines changed

11 files changed

+178
-86
lines changed

‎tests/ModelContextProtocol.TestSseServer/Program.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ namespace ModelContextProtocol.TestSseServer;
1010

1111
public class Program
1212
{
13-
private static ILoggerFactory CreateLoggerFactory()
13+
private static ILoggerFactory CreateLoggerFactory() => LoggerFactory.Create(ConfigureSerilog);
14+
15+
public static void ConfigureSerilog(ILoggingBuilder loggingBuilder)
1416
{
15-
// Use serilog
1617
Log.Logger = new LoggerConfiguration()
1718
.MinimumLevel.Verbose() // Capture all log levels
1819
.WriteTo.File(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "logs", "TestServer_.log"),
@@ -21,15 +22,12 @@ private static ILoggerFactory CreateLoggerFactory()
2122
.CreateLogger();
2223

2324
var logsPath = Path.Combine(AppContext.BaseDirectory, "testserver.log");
24-
return LoggerFactory.Create(builder =>
25-
{
26-
builder.AddSerilog();
27-
});
25+
loggingBuilder.AddSerilog();
2826
}
2927

3028
public static Task Main(string[] args) => MainAsync(args);
3129

32-
public static async Task MainAsync(string[] args, CancellationToken cancellationToken = default)
30+
public static async Task MainAsync(string[] args, ILoggerFactory? loggerFactory = null, CancellationToken cancellationToken = default)
3331
{
3432
Console.WriteLine("Starting server...");
3533

@@ -385,7 +383,7 @@ static CreateMessageRequestParams CreateRequestSamplingParams(string context, st
385383
},
386384
};
387385

388-
using var loggerFactory = CreateLoggerFactory();
386+
loggerFactory ??= CreateLoggerFactory();
389387
server = McpServerFactory.Create(new HttpListenerSseServerTransport("TestServer", 3001, loggerFactory), options, loggerFactory);
390388

391389
Console.WriteLine("Server initialized.");

‎tests/ModelContextProtocol.Tests/ClientIntegrationTestFixture.cs‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55

66
namespace ModelContextProtocol.Tests;
77

8-
public class ClientIntegrationTestFixture : IDisposable
8+
public class ClientIntegrationTestFixture
99
{
10-
public ILoggerFactory LoggerFactory { get; }
10+
private ILoggerFactory? _loggerFactory;
11+
1112
public McpClientOptions DefaultOptions { get; }
1213
public McpServerConfig EverythingServerConfig { get; }
1314
public McpServerConfig TestServerConfig { get; }
@@ -16,10 +17,6 @@ public class ClientIntegrationTestFixture : IDisposable
1617

1718
public ClientIntegrationTestFixture()
1819
{
19-
LoggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder =>
20-
builder.AddConsole()
21-
.SetMinimumLevel(LogLevel.Debug));
22-
2320
DefaultOptions = new()
2421
{
2522
ClientInfo = new() { Name = "IntegrationTestClient", Version = "1.0.0" },
@@ -56,17 +53,16 @@ public ClientIntegrationTestFixture()
5653
}
5754
}
5855

56+
public void Initialize(ILoggerFactory loggerFactory)
57+
{
58+
_loggerFactory = loggerFactory;
59+
}
60+
5961
public Task<IMcpClient> CreateClientAsync(string clientId, McpClientOptions? clientOptions = null) =>
6062
McpClientFactory.CreateAsync(clientId switch
6163
{
6264
"everything" => EverythingServerConfig,
6365
"test_server" => TestServerConfig,
6466
_ => throw new ArgumentException($"Unknown client ID: {clientId}")
65-
}, clientOptions ?? DefaultOptions, loggerFactory: LoggerFactory);
66-
67-
public void Dispose()
68-
{
69-
LoggerFactory?.Dispose();
70-
GC.SuppressFinalize(this);
71-
}
67+
}, clientOptions ?? DefaultOptions, loggerFactory: _loggerFactory);
7268
}

‎tests/ModelContextProtocol.Tests/ClientIntegrationTests.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,25 @@
66
using System.Text.Json;
77
using ModelContextProtocol.Configuration;
88
using ModelContextProtocol.Protocol.Transport;
9+
using ModelContextProtocol.Tests.Utils;
910
using Xunit.Sdk;
1011
using System.Text.Encodings.Web;
1112
using System.Text.Json.Serialization.Metadata;
1213
using System.Text.Json.Serialization;
1314

1415
namespace ModelContextProtocol.Tests;
1516

16-
public class ClientIntegrationTests : IClassFixture<ClientIntegrationTestFixture>
17+
public class ClientIntegrationTests : LoggedTest, IClassFixture<ClientIntegrationTestFixture>
1718
{
1819
private static readonly string? s_openAIKey = Environment.GetEnvironmentVariable("AI:OpenAI:ApiKey")!;
1920

2021
private readonly ClientIntegrationTestFixture _fixture;
2122

22-
public ClientIntegrationTests(ClientIntegrationTestFixture fixture)
23+
public ClientIntegrationTests(ClientIntegrationTestFixture fixture, ITestOutputHelper testOutputHelper)
24+
: base(testOutputHelper)
2325
{
2426
_fixture = fixture;
27+
_fixture.Initialize(LoggerFactory);
2528
}
2629

2730
public static IEnumerable<object[]> GetClients() =>
@@ -474,7 +477,7 @@ public async Task CallTool_Stdio_MemoryServer()
474477
await using var client = await McpClientFactory.CreateAsync(
475478
serverConfig,
476479
clientOptions,
477-
loggerFactory: _fixture.LoggerFactory,
480+
loggerFactory: LoggerFactory,
478481
cancellationToken: TestContext.Current.CancellationToken);
479482

480483
// act

‎tests/ModelContextProtocol.Tests/Server/McpServerFactoryTests.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
using ModelContextProtocol.Protocol.Transport;
22
using ModelContextProtocol.Protocol.Types;
33
using ModelContextProtocol.Server;
4-
using Microsoft.Extensions.Logging.Abstractions;
4+
using ModelContextProtocol.Tests.Utils;
55
using Moq;
66

77
namespace ModelContextProtocol.Tests.Server;
88

9-
public class McpServerFactoryTests
9+
public class McpServerFactoryTests : LoggedTest
1010
{
1111
private readonly Mock<IServerTransport> _serverTransport;
1212
private readonly McpServerOptions _options;
13-
private readonly IServiceProvider _serviceProvider;
1413

15-
public McpServerFactoryTests()
14+
public McpServerFactoryTests(ITestOutputHelper testOutputHelper)
15+
: base(testOutputHelper)
1616
{
1717
_serverTransport = new Mock<IServerTransport>();
1818
_options = new McpServerOptions
@@ -21,14 +21,13 @@ public McpServerFactoryTests()
2121
ProtocolVersion = "1.0",
2222
InitializationTimeout = TimeSpan.FromSeconds(30)
2323
};
24-
_serviceProvider = new Mock<IServiceProvider>().Object;
2524
}
2625

2726
[Fact]
2827
public async Task Create_Should_Initialize_With_Valid_Parameters()
2928
{
3029
// Arrange & Act
31-
await using IMcpServer server = McpServerFactory.Create(_serverTransport.Object, _options, NullLoggerFactory.Instance);
30+
await using IMcpServer server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory);
3231

3332
// Assert
3433
Assert.NotNull(server);
@@ -38,13 +37,13 @@ public async Task Create_Should_Initialize_With_Valid_Parameters()
3837
public void Constructor_Throws_For_Null_ServerTransport()
3938
{
4039
// Arrange, Act & Assert
41-
Assert.Throws<ArgumentNullException>("serverTransport", () => McpServerFactory.Create(null!, _options, NullLoggerFactory.Instance));
40+
Assert.Throws<ArgumentNullException>("serverTransport", () => McpServerFactory.Create(null!, _options, LoggerFactory));
4241
}
4342

4443
[Fact]
4544
public void Constructor_Throws_For_Null_Options()
4645
{
4746
// Arrange, Act & Assert
48-
Assert.Throws<ArgumentNullException>("serverOptions", () => McpServerFactory.Create(_serverTransport.Object, null!, NullLoggerFactory.Instance));
47+
Assert.Throws<ArgumentNullException>("serverOptions", () => McpServerFactory.Create(_serverTransport.Object, null!, LoggerFactory));
4948
}
5049
}

‎tests/ModelContextProtocol.Tests/Server/McpServerTests.cs‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,18 @@
1111

1212
namespace ModelContextProtocol.Tests.Server;
1313

14-
public class McpServerTests
14+
public class McpServerTests : LoggedTest
1515
{
1616
private readonly Mock<IServerTransport> _serverTransport;
17-
private readonly Mock<ILoggerFactory> _loggerFactory;
1817
private readonly Mock<ILogger> _logger;
1918
private readonly McpServerOptions _options;
2019
private readonly IServiceProvider _serviceProvider;
2120

22-
public McpServerTests()
21+
public McpServerTests(ITestOutputHelper testOutputHelper)
22+
: base(testOutputHelper)
2323
{
2424
_serverTransport = new Mock<IServerTransport>();
25-
_loggerFactory = new Mock<ILoggerFactory>();
2625
_logger = new Mock<ILogger>();
27-
_loggerFactory.Setup(f => f.CreateLogger(It.IsAny<string>())).Returns(_logger.Object);
2826
_options = CreateOptions();
2927
_serviceProvider = new Mock<IServiceProvider>().Object;
3028
}
@@ -44,7 +42,7 @@ private static McpServerOptions CreateOptions(ServerCapabilities? capabilities =
4442
public async Task Constructor_Should_Initialize_With_Valid_Parameters()
4543
{
4644
// Arrange & Act
47-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
45+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
4846

4947
// Assert
5048
Assert.NotNull(server);
@@ -54,14 +52,14 @@ public async Task Constructor_Should_Initialize_With_Valid_Parameters()
5452
public void Constructor_Throws_For_Null_Transport()
5553
{
5654
// Arrange, Act & Assert
57-
Assert.Throws<ArgumentNullException>(() => McpServerFactory.Create(null!, _options, _loggerFactory.Object, _serviceProvider));
55+
Assert.Throws<ArgumentNullException>(() => McpServerFactory.Create(null!, _options, LoggerFactory, _serviceProvider));
5856
}
5957

6058
[Fact]
6159
public void Constructor_Throws_For_Null_Options()
6260
{
6361
// Arrange, Act & Assert
64-
Assert.Throws<ArgumentNullException>(() => McpServerFactory.Create(_serverTransport.Object, null!, _loggerFactory.Object, _serviceProvider));
62+
Assert.Throws<ArgumentNullException>(() => McpServerFactory.Create(_serverTransport.Object, null!, LoggerFactory, _serviceProvider));
6563
}
6664

6765
[Fact]
@@ -78,7 +76,7 @@ public async Task Constructor_Does_Not_Throw_For_Null_Logger()
7876
public async Task Constructor_Does_Not_Throw_For_Null_ServiceProvider()
7977
{
8078
// Arrange & Act
81-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, null);
79+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, null);
8280

8381
// Assert
8482
Assert.NotNull(server);
@@ -88,7 +86,7 @@ public async Task Constructor_Does_Not_Throw_For_Null_ServiceProvider()
8886
public async Task StartAsync_Should_Throw_InvalidOperationException_If_Already_Initializing()
8987
{
9088
// Arrange
91-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
89+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
9290
var task = server.StartAsync(TestContext.Current.CancellationToken);
9391

9492
// Act & Assert
@@ -101,7 +99,7 @@ public async Task StartAsync_Should_Throw_InvalidOperationException_If_Already_I
10199
public async Task StartAsync_Should_Do_Nothing_If_Already_Initialized()
102100
{
103101
// Arrange
104-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
102+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
105103
SetInitialized(server, true);
106104

107105
await server.StartAsync(TestContext.Current.CancellationToken);
@@ -114,7 +112,7 @@ public async Task StartAsync_Should_Do_Nothing_If_Already_Initialized()
114112
public async Task StartAsync_ShouldStartListening()
115113
{
116114
// Arrange
117-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
115+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
118116

119117
// Act
120118
await server.StartAsync(TestContext.Current.CancellationToken);
@@ -127,7 +125,7 @@ public async Task StartAsync_ShouldStartListening()
127125
public async Task StartAsync_Sets_Initialized_After_Transport_Responses_Initialized_Notification()
128126
{
129127
await using var transport = new TestServerTransport();
130-
await using var server = McpServerFactory.Create(transport, _options, _loggerFactory.Object, _serviceProvider);
128+
await using var server = McpServerFactory.Create(transport, _options, LoggerFactory, _serviceProvider);
131129

132130
await server.StartAsync(TestContext.Current.CancellationToken);
133131

@@ -147,7 +145,7 @@ await transport.SendMessageAsync(new JsonRpcNotification
147145
public async Task RequestSamplingAsync_Should_Throw_McpServerException_If_Client_Does_Not_Support_Sampling()
148146
{
149147
// Arrange
150-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
148+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
151149
SetClientCapabilities(server, new ClientCapabilities());
152150

153151
var action = () => server.RequestSamplingAsync(new CreateMessageRequestParams { Messages = [] }, CancellationToken.None);
@@ -161,7 +159,7 @@ public async Task RequestSamplingAsync_Should_SendRequest()
161159
{
162160
// Arrange
163161
await using var transport = new TestServerTransport();
164-
await using var server = McpServerFactory.Create(transport, _options, _loggerFactory.Object, _serviceProvider);
162+
await using var server = McpServerFactory.Create(transport, _options, LoggerFactory, _serviceProvider);
165163
SetClientCapabilities(server, new ClientCapabilities { Sampling = new SamplingCapability() });
166164

167165
await server.StartAsync(TestContext.Current.CancellationToken);
@@ -179,7 +177,7 @@ public async Task RequestSamplingAsync_Should_SendRequest()
179177
public async Task RequestRootsAsync_Should_Throw_McpServerException_If_Client_Does_Not_Support_Roots()
180178
{
181179
// Arrange
182-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
180+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
183181
SetClientCapabilities(server, new ClientCapabilities());
184182

185183
// Act & Assert
@@ -191,7 +189,7 @@ public async Task RequestRootsAsync_Should_SendRequest()
191189
{
192190
// Arrange
193191
await using var transport = new TestServerTransport();
194-
await using var server = McpServerFactory.Create(transport, _options, _loggerFactory.Object, _serviceProvider);
192+
await using var server = McpServerFactory.Create(transport, _options, LoggerFactory, _serviceProvider);
195193
SetClientCapabilities(server, new ClientCapabilities { Roots = new RootsCapability() });
196194
await server.StartAsync(TestContext.Current.CancellationToken);
197195

@@ -208,7 +206,7 @@ public async Task RequestRootsAsync_Should_SendRequest()
208206
[Fact]
209207
public async Task Throws_Exception_If_Not_Connected()
210208
{
211-
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, _loggerFactory.Object, _serviceProvider);
209+
await using var server = McpServerFactory.Create(_serverTransport.Object, _options, LoggerFactory, _serviceProvider);
212210
SetClientCapabilities(server, new ClientCapabilities { Roots = new RootsCapability() });
213211
_serverTransport.SetupGet(t => t.IsConnected).Returns(false);
214212

@@ -555,7 +553,7 @@ private async Task Can_Handle_Requests(ServerCapabilities? serverCapabilities, s
555553
var options = CreateOptions(serverCapabilities);
556554
configureOptions?.Invoke(options);
557555

558-
await using var server = McpServerFactory.Create(transport, options, _loggerFactory.Object, _serviceProvider);
556+
await using var server = McpServerFactory.Create(transport, options, LoggerFactory, _serviceProvider);
559557

560558
await server.StartAsync();
561559

@@ -587,7 +585,7 @@ private async Task Throws_Exception_If_No_Handler_Assigned(ServerCapabilities se
587585
await using var transport = new TestServerTransport();
588586
var options = CreateOptions(serverCapabilities);
589587

590-
Assert.Throws<McpServerException>(() => McpServerFactory.Create(transport, options, _loggerFactory.Object, _serviceProvider));
588+
Assert.Throws<McpServerException>(() => McpServerFactory.Create(transport, options, LoggerFactory, _serviceProvider));
591589
}
592590

593591
[Fact]

0 commit comments

Comments
 (0)