From 311d37de4750d856f3e2292fdf8bacc1bcc62709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Tue, 6 Apr 2021 10:52:32 +0200 Subject: [PATCH 1/2] Added tests for LoggerMessage.Define with skipEnabledCheck --- .../tests/Common/LoggerMessageTest.cs | 41 +++++++++++++++++++ .../tests/DI.Common/Common/src/TestLogger.cs | 5 --- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs index 49e2a17c54fd83..c372e1ea73f21c 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs @@ -193,6 +193,36 @@ public void LogMessages(Delegate messageDelegate, int argumentCount) Assert.Equal(expectedToString, actualLogValues.ToString()); } + [Theory] + [MemberData(nameof(LogMessagesDataSkipEnabledCheck))] + public void LogMessagesSkipEnabledCheck(Delegate messageDelegate, int argumentCount) + { + // Arrange + var testSink = new TestSink(); + var testLogger = new TestLogger("testlogger", testSink, l => l == LogLevel.None); + var exception = new Exception("TestException"); + var parameterNames = Enumerable.Range(0, argumentCount).Select(i => "P" + i).ToArray(); + var parameters = new List(); + parameters.Add(testLogger); + parameters.AddRange(parameterNames); + parameters.Add(exception); + + var expectedFormat = "Log " + string.Join(" ", parameterNames.Select(p => "{" + p + "}")); + var expectedToString = "Log " + string.Join(" ", parameterNames); + var expectedValues = parameterNames.Select(p => new KeyValuePair(p, p)).ToList(); + expectedValues.Add(new KeyValuePair("{OriginalFormat}", expectedFormat)); + + // Act + messageDelegate.DynamicInvoke(parameters.ToArray()); + + // Assert + Assert.Single(testSink.Writes); + var write = testSink.Writes.First(); + var actualLogValues = Assert.IsAssignableFrom>>(write.State); + AssertLogValues(expectedValues, actualLogValues.ToList()); + Assert.Equal(expectedToString, actualLogValues.ToString()); + } + [Fact] public void LogMessage_WithNullParameter_DoesNotMutateArgument() { @@ -392,6 +422,17 @@ public void DefineAndDefineScope_ThrowsException_WhenFormatString_IsNull(Delegat new object[] { LoggerMessage.Define(LogLevel.Error, 6, "Log {P0} {P1} {P2} {P3} {P4} {P5}"), 6 }, }; + public static IEnumerable LogMessagesDataSkipEnabledCheck => new[] + { + new object[] { LoggerMessage.Define(LogLevel.Error, 0, "Log ", skipEnabledCheck: true), 0 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 1, "Log {P0}", skipEnabledCheck: true), 1 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 2, "Log {P0} {P1}", skipEnabledCheck: true), 2 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 3, "Log {P0} {P1} {P2}", skipEnabledCheck: true), 3 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 4, "Log {P0} {P1} {P2} {P3}", skipEnabledCheck: true), 4 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 5, "Log {P0} {P1} {P2} {P3} {P4}", skipEnabledCheck: true), 5 }, + new object[] { LoggerMessage.Define(LogLevel.Error, 6, "Log {P0} {P1} {P2} {P3} {P4} {P5}", skipEnabledCheck: true), 6 }, + }; + private delegate Delegate Define(LogLevel logLevel, EventId eventId, string formatString); private delegate Delegate DefineScope(string formatString); private static object[] DefineInvalidParameters => new object[] { LogLevel.Error, new EventId(0), null }; diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs b/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs index bf625b178ccc65..1d6504ee663a42 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs @@ -41,11 +41,6 @@ public IDisposable BeginScope(TState state) public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { - if (!IsEnabled(logLevel)) - { - return; - } - _sink.Write(new WriteContext() { LogLevel = logLevel, From d1b4e93652016f93eceafcadbe1ac1f0b0ae3de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Foidl?= Date: Tue, 6 Apr 2021 19:38:50 +0200 Subject: [PATCH 2/2] Use call-counting IsEnabled to assert the skipEnabledCheck --- .../tests/Common/LoggerMessageTest.cs | 4 +++- .../tests/DI.Common/Common/src/TestLogger.cs | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs index c372e1ea73f21c..d71b27f093505e 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerMessageTest.cs @@ -191,6 +191,7 @@ public void LogMessages(Delegate messageDelegate, int argumentCount) var actualLogValues = Assert.IsAssignableFrom>>(write.State); AssertLogValues(expectedValues, actualLogValues.ToList()); Assert.Equal(expectedToString, actualLogValues.ToString()); + Assert.Equal(2, testLogger.IsEnabledCallCount); } [Theory] @@ -199,7 +200,7 @@ public void LogMessagesSkipEnabledCheck(Delegate messageDelegate, int argumentCo { // Arrange var testSink = new TestSink(); - var testLogger = new TestLogger("testlogger", testSink, l => l == LogLevel.None); + var testLogger = new TestLogger("testlogger", testSink, enabled: true); var exception = new Exception("TestException"); var parameterNames = Enumerable.Range(0, argumentCount).Select(i => "P" + i).ToArray(); var parameters = new List(); @@ -221,6 +222,7 @@ public void LogMessagesSkipEnabledCheck(Delegate messageDelegate, int argumentCo var actualLogValues = Assert.IsAssignableFrom>>(write.State); AssertLogValues(expectedValues, actualLogValues.ToList()); Assert.Equal(expectedToString, actualLogValues.ToString()); + Assert.Equal(1, testLogger.IsEnabledCallCount); } [Fact] diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs b/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs index 1d6504ee663a42..4774e963d80403 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/DI.Common/Common/src/TestLogger.cs @@ -25,6 +25,7 @@ public TestLogger(string name, ITestSink sink, Func filter) } public string Name { get; set; } + public int IsEnabledCallCount { get; private set; } public IDisposable BeginScope(TState state) { @@ -41,6 +42,11 @@ public IDisposable BeginScope(TState state) public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { + if (!IsEnabled(logLevel)) + { + return; + } + _sink.Write(new WriteContext() { LogLevel = logLevel, @@ -55,6 +61,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except public bool IsEnabled(LogLevel logLevel) { + IsEnabledCallCount++; return logLevel != LogLevel.None && _filter(logLevel); }