Copilot commented on code in PR #7277:
URL: https://github.com/apache/ignite-3/pull/7277#discussion_r2640020254
##########
modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServiceCollectionExtensionsTests.cs:
##########
@@ -153,6 +153,55 @@ public void
TestRegisterScopesConfigurationFuncWithKeyKeyed([Values] ServiceLife
(s, key) => s.AddIgniteClientGroupKeyed(key, (_, _) =>
CreateGroupConfig(), lifetime));
}
+ [Test]
+ public void TestAutomaticLoggerFactorySetFromServices()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
Review Comment:
The ILoggerFactory created here should be disposed to prevent resource
leaks. ILoggerFactory implements IDisposable, and the logger factory should be
disposed after the test completes. Consider wrapping it in a using statement or
disposing it explicitly at the end of the test.
```suggestion
using var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
```
##########
modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServiceCollectionExtensionsTests.cs:
##########
@@ -153,6 +153,55 @@ public void
TestRegisterScopesConfigurationFuncWithKeyKeyed([Values] ServiceLife
(s, key) => s.AddIgniteClientGroupKeyed(key, (_, _) =>
CreateGroupConfig(), lifetime));
}
+ [Test]
+ public void TestAutomaticLoggerFactorySetFromServices()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
+ services.AddSingleton(diLoggerFactory);
+
+ var config = new IgniteClientGroupConfiguration
+ {
+ ClientConfiguration = new
IgniteClientConfiguration(_server.Endpoint)
+ };
+
+ services.AddIgniteClientGroup(config);
+
+ using var serviceProvider = services.BuildServiceProvider();
+ using var group =
serviceProvider.GetRequiredService<IgniteClientGroup>();
+
+ var actualLoggerFactory =
group.Configuration.ClientConfiguration.LoggerFactory;
+ Assert.AreSame(diLoggerFactory, actualLoggerFactory);
+ }
Review Comment:
The automatic LoggerFactory injection feature lacks test coverage for
Func-based registration methods. Consider adding tests for AddIgniteClientGroup
with Func<IServiceProvider, IgniteClientGroupConfiguration> to ensure the
automatic logger factory injection works correctly when configurations are
created dynamically from the service provider.
##########
modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServiceCollectionExtensionsTests.cs:
##########
@@ -153,6 +153,55 @@ public void
TestRegisterScopesConfigurationFuncWithKeyKeyed([Values] ServiceLife
(s, key) => s.AddIgniteClientGroupKeyed(key, (_, _) =>
CreateGroupConfig(), lifetime));
}
+ [Test]
+ public void TestAutomaticLoggerFactorySetFromServices()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
+ services.AddSingleton(diLoggerFactory);
+
+ var config = new IgniteClientGroupConfiguration
+ {
+ ClientConfiguration = new
IgniteClientConfiguration(_server.Endpoint)
+ };
+
+ services.AddIgniteClientGroup(config);
+
+ using var serviceProvider = services.BuildServiceProvider();
+ using var group =
serviceProvider.GetRequiredService<IgniteClientGroup>();
+
+ var actualLoggerFactory =
group.Configuration.ClientConfiguration.LoggerFactory;
+ Assert.AreSame(diLoggerFactory, actualLoggerFactory);
+ }
+
+ [Test]
+ public void TestCustomLoggerFactoryIsPreserved()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
+ services.AddSingleton(diLoggerFactory);
+
+ var customLoggerFactory =
TestUtils.GetConsoleLoggerFactory(LogLevel.Trace);
+ var config = new IgniteClientGroupConfiguration
+ {
+ ClientConfiguration = new
IgniteClientConfiguration(_server.Endpoint)
+ {
+ LoggerFactory = customLoggerFactory
+ }
+ };
+
+ services.AddIgniteClientGroup(config);
+
+ using var serviceProvider = services.BuildServiceProvider();
+ using var group =
serviceProvider.GetRequiredService<IgniteClientGroup>();
+
+ var actualLoggerFactory =
group.Configuration.ClientConfiguration.LoggerFactory;
+ Assert.AreSame(customLoggerFactory, actualLoggerFactory);
+ Assert.AreNotSame(diLoggerFactory, actualLoggerFactory);
+ }
Review Comment:
The automatic LoggerFactory injection feature lacks test coverage for keyed
service registration methods. Consider adding tests for
AddIgniteClientGroupKeyed to ensure the automatic logger factory injection
works correctly when using service keys.
##########
modules/platforms/dotnet/Apache.Ignite.Tests/IgniteServiceCollectionExtensionsTests.cs:
##########
@@ -153,6 +153,55 @@ public void
TestRegisterScopesConfigurationFuncWithKeyKeyed([Values] ServiceLife
(s, key) => s.AddIgniteClientGroupKeyed(key, (_, _) =>
CreateGroupConfig(), lifetime));
}
+ [Test]
+ public void TestAutomaticLoggerFactorySetFromServices()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
+ services.AddSingleton(diLoggerFactory);
+
+ var config = new IgniteClientGroupConfiguration
+ {
+ ClientConfiguration = new
IgniteClientConfiguration(_server.Endpoint)
+ };
+
+ services.AddIgniteClientGroup(config);
+
+ using var serviceProvider = services.BuildServiceProvider();
+ using var group =
serviceProvider.GetRequiredService<IgniteClientGroup>();
+
+ var actualLoggerFactory =
group.Configuration.ClientConfiguration.LoggerFactory;
+ Assert.AreSame(diLoggerFactory, actualLoggerFactory);
+ }
+
+ [Test]
+ public void TestCustomLoggerFactoryIsPreserved()
+ {
+ var services = new ServiceCollection();
+
+ var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
+ services.AddSingleton(diLoggerFactory);
+
+ var customLoggerFactory =
TestUtils.GetConsoleLoggerFactory(LogLevel.Trace);
Review Comment:
The ILoggerFactory created here should be disposed to prevent resource
leaks. ILoggerFactory implements IDisposable, and both logger factories should
be disposed after the test completes. Consider wrapping them in using
statements or disposing them explicitly at the end of the test.
```suggestion
using var customLoggerFactory =
TestUtils.GetConsoleLoggerFactory(LogLevel.Trace);
var services = new ServiceCollection();
var diLoggerFactory = LoggerFactory.Create(builder =>
builder.AddConsole());
services.AddSingleton(diLoggerFactory);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
