Skip to content

Commit 4cefaea

Browse files
committed
Fix tests review comment
1 parent 503643a commit 4cefaea

File tree

1 file changed

+58
-64
lines changed

1 file changed

+58
-64
lines changed

ExtractorUtils.Test/unit/LoggingTest.cs

Lines changed: 58 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -129,48 +129,51 @@ public static void TestLogLevel()
129129
Assert.Equal(4, sideEffect.logCount); // should not be evaluated
130130
}
131131

132-
[Theory]
133-
[InlineData("console")]
134-
[InlineData("file")]
135-
[InlineData("trace-listener")]
136-
public void TestLogger_WithDifferentLogTypes(string logType)
132+
private static ServiceCollection ConfigureLoggerService(string logType, string level = null)
137133
{
138134
var services = new ServiceCollection();
135+
var loggerConfig = new LoggerConfig();
139136

140137
switch (logType)
141138
{
142139
case "console":
143-
services.AddSingleton(new LoggerConfig
144-
{
145-
Console = new ConsoleConfig { Level = "information" }
146-
});
140+
loggerConfig.Console = new ConsoleConfig { Level = level ?? "information" };
147141
break;
148142
case "file":
149-
services.AddSingleton(new LoggerConfig
150-
{
151-
File = new FileConfig { Level = "warning", Path = "test.log" }
152-
});
143+
loggerConfig.File = new FileConfig { Level = level ?? "warning", Path = "test.log" };
153144
break;
154145
case "trace-listener":
155-
services.AddSingleton(new LoggerConfig
156-
{
157-
TraceListener = new TraceListenerConfig { Level = "error" }
158-
});
146+
loggerConfig.TraceListener = new TraceListenerConfig { Level = level ?? "error" };
159147
break;
160148
}
149+
150+
services.AddSingleton(loggerConfig);
161151
services.AddLogger();
152+
return services;
153+
}
162154

155+
[Theory]
156+
[InlineData("console")]
157+
[InlineData("file")]
158+
[InlineData("trace-listener")]
159+
public void TestLogger_WithDifferentLogTypes(string logType)
160+
{
161+
var services = ConfigureLoggerService(logType);
162+
163+
using (var provider = services.BuildServiceProvider())
163164
{
164-
using var provider = services.BuildServiceProvider();
165165
var logger = provider.GetRequiredService<ILogger<LoggingTest>>();
166166

167167
Assert.NotNull(logger);
168-
} // provider is disposed here
168+
logger.LogWarning("Test log message");
169+
} // provider is disposed here, which flushes async logs
169170

170-
// Cleanup test.log file if created
171-
if (File.Exists("test.log"))
171+
// Assert and cleanup test.log file if created by file logger
172+
if (logType == "file")
172173
{
173-
File.Delete("test.log");
174+
var logFile = $"test{DateTime.Now:yyyyMMdd}.log";
175+
Assert.True(File.Exists(logFile));
176+
File.Delete(logFile);
174177
}
175178
}
176179

@@ -181,58 +184,49 @@ public void TestLogger_WithDifferentLogTypes(string logType)
181184
[InlineData("trace-listener", "error")]
182185
public void TestLogger_WithTypeAndLevel(string configType, string level)
183186
{
184-
var services = new ServiceCollection();
185-
186-
// Configure logger based on the specified type and level
187-
var loggerConfig = new LoggerConfig();
188-
switch (configType)
189-
{
190-
case "console":
191-
loggerConfig.Console = new ConsoleConfig { Level = level };
192-
break;
193-
case "file":
194-
loggerConfig.File = new FileConfig { Level = level, Path = "test.log" };
195-
break;
196-
case "trace-listener":
197-
loggerConfig.TraceListener = new TraceListenerConfig { Level = level };
198-
break;
199-
}
200-
201-
services.AddSingleton(loggerConfig);
202-
services.AddLogger();
187+
var services = ConfigureLoggerService(configType, level);
203188

189+
using (var provider = services.BuildServiceProvider())
204190
{
205-
using var provider = services.BuildServiceProvider();
206191
var logger = provider.GetRequiredService<ILogger<LoggingTest>>();
207-
208-
// Assert logger is configured
209192
Assert.NotNull(logger);
210193

211-
// Verify the configuration was applied
212-
var retrievedConfig = provider.GetRequiredService<LoggerConfig>();
213-
Assert.NotNull(retrievedConfig);
214-
215-
switch (configType)
194+
// Verify the logger respects the configured level using side effects
195+
// Note: trace-listener alone falls back to default logger, so we only verify console and file
196+
if (configType != "trace-listener")
216197
{
217-
case "console":
218-
Assert.NotNull(retrievedConfig.Console);
219-
Assert.Equal(level, retrievedConfig.Console.Level);
220-
break;
221-
case "file":
222-
Assert.NotNull(retrievedConfig.File);
223-
Assert.Equal(level, retrievedConfig.File.Level);
224-
break;
225-
case "trace-listener":
226-
Assert.NotNull(retrievedConfig.TraceListener);
227-
Assert.Equal(level, retrievedConfig.TraceListener.Level);
228-
break;
198+
var sideEffect = new LogSideEffect();
199+
logger.LogDebug("Debug: {SideEffect}", sideEffect);
200+
logger.LogInformation("Info: {SideEffect}", sideEffect);
201+
logger.LogWarning("Warning: {SideEffect}", sideEffect);
202+
logger.LogError("Error: {SideEffect}", sideEffect);
203+
204+
// Verify only messages at or above the configured level were evaluated
205+
int expectedCount = level switch
206+
{
207+
"debug" => 4, // debug, info, warning, error
208+
"information" => 3, // info, warning, error
209+
"warning" => 2, // warning, error
210+
"error" => 1, // error only
211+
_ => 0
212+
};
213+
Assert.Equal(expectedCount, sideEffect.logCount);
214+
}
215+
else
216+
{
217+
// For trace-listener, just verify it doesn't throw
218+
logger.LogError("Test log message");
229219
}
230220
} // provider is disposed here
231221

232-
// Cleanup test.log file if created
233-
if (File.Exists("test.log"))
222+
// Cleanup file if created by file logger
223+
if (configType == "file")
234224
{
235-
File.Delete("test.log");
225+
var logFile = $"test.log";
226+
if (File.Exists(logFile))
227+
{
228+
File.Delete(logFile);
229+
}
236230
}
237231
}
238232
}

0 commit comments

Comments
 (0)