From ee00f00a2e3a99721f0027ce0fbe7d4317283379 Mon Sep 17 00:00:00 2001 From: Anthony Truskinger Date: Mon, 11 Mar 2019 17:00:07 +1000 Subject: [PATCH] Fixes log naming bug Bug was introduced in 209745daa7702c8bbbccaf500330439b7bbddce5 for #220. I accidently added a space in the log file names :dizzy_face: I've also added more tests so that our log cleaning function gets tested now. --- src/Acoustics.Shared/Logging/Logging.cs | 25 +++++++---- .../Shared/LoggingTests/LoggingTests.cs | 42 +++++++++++++++++++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/Acoustics.Shared/Logging/Logging.cs b/src/Acoustics.Shared/Logging/Logging.cs index 4bb293b8d..13dad1910 100644 --- a/src/Acoustics.Shared/Logging/Logging.cs +++ b/src/Acoustics.Shared/Logging/Logging.cs @@ -15,7 +15,6 @@ namespace Acoustics.Shared.Logging using log4net.Appender; using log4net.Core; using log4net.Layout; - using log4net.Layout.Pattern; using log4net.Repository.Hierarchy; using log4net.Util; using static log4net.Appender.ManagedColoredConsoleAppender; @@ -24,10 +23,13 @@ public class Logging { public const string CleanLogger = "CleanLogger"; public const string LogFileOnly = "LogFileOnly"; + private const string LogPrefix = "log_"; private readonly Logger rootLogger; + // ReSharper disable once PrivateFieldCanBeConvertedToLocalVariable private readonly Logger cleanLogger; + // ReSharper disable once PrivateFieldCanBeConvertedToLocalVariable private readonly Logger noConsoleLogger; private readonly AppenderSkeleton standardConsoleAppender; @@ -92,7 +94,7 @@ internal Logging( string logFilePath = null; if (enableFileLogger) { - this.LogFileName = new PatternString("log_ %utcdate{yyyyMMddTHHmmssZ}.txt").Format(); + this.LogFileName = new PatternString(LogPrefix + "%utcdate{yyyyMMddTHHmmssZ}.txt").Format(); var fileAppender = new RollingFileAppender() { AppendToFile = false, @@ -256,13 +258,13 @@ private async Task CleanLogs(string logFilePath) { Contract.RequiresNotNull(logFilePath); const int threshold = 60; - const int target = 50; + int target = 50; void CleanFiles() { var logsPath = Path.GetDirectoryName(logFilePath) ?? throw new InvalidOperationException("Could not resolve logs directory path: " + logFilePath); - var files = Directory.GetFiles(logsPath, "log_*.txt"); + var files = Directory.GetFiles(logsPath, LogPrefix + "*.txt"); if (files.Length > threshold) { var sorted = new SortedDictionary>(); @@ -271,12 +273,21 @@ void CleanFiles() var name = Path.GetFileName(file); // assuming a format of log_20180717T130822Z.1.txt - var datePart = name.Substring(4, name.IndexOf(".", StringComparison.Ordinal) - 4); - var date = DateTime.ParseExact( + int prefixLength = LogPrefix.Length; + var datePart = name.Substring(prefixLength, name.IndexOf(".", StringComparison.Ordinal) - prefixLength); + var success = DateTime.TryParseExact( datePart, "yyyyMMddTHHmmssZ", CultureInfo.InvariantCulture, - DateTimeStyles.AssumeUniversal); + DateTimeStyles.AssumeUniversal, + out var date); + + if (!success) + { + // the default date value will ensure value is added into sorted list + // and it will be deleted first! + LoggedConsole.WriteWarnLine($"Log file with name `{name}` has invalid date format and is being cleaned"); + } if (sorted.ContainsKey(date)) { diff --git a/tests/Acoustics.Test/Shared/LoggingTests/LoggingTests.cs b/tests/Acoustics.Test/Shared/LoggingTests/LoggingTests.cs index 75c82ada5..c347b6e30 100644 --- a/tests/Acoustics.Test/Shared/LoggingTests/LoggingTests.cs +++ b/tests/Acoustics.Test/Shared/LoggingTests/LoggingTests.cs @@ -6,7 +6,10 @@ namespace Acoustics.Test.Shared.LoggingTests { using System; using System.IO; + using System.Linq; using System.Reflection; + using System.Threading.Tasks; + using Acoustics.Shared; using Acoustics.Shared.ConfigFile; using Acoustics.Shared.Logging; using Fasterflect; @@ -58,5 +61,44 @@ public void TestVerbosityModifier(string targetVerbosity, int expectedMessageCou Assert.AreEqual(expectedMessageCount, TestSetup.TestLogging.MemoryAppender.GetEvents().Length); } } + + [TestMethod] + public void TestLogFileIsCreated() + { + var logging = new Logging(false, Level.Info, quietConsole: false); + + var expectedPath = Path.Combine(LoggedConsole.LogFolder, logging.LogFileName); + Assert.That.PathExists(expectedPath); + + StringAssert.StartsWith(logging.LogFileName, "log_20"); + } + + [TestMethod] + public async Task TestLogFilesAreCleaned() + { + var logDirectory = LoggedConsole.LogFolder; + + // get count + var files = Directory.GetFiles(logDirectory); + var oldCount = files.Length; + + // it should allow up to 60 log files, before it cleans 10 + var delta = 60 - oldCount + 1; + + // "do" because we need to create a new log at least once for this test to trigger + // its cleaning logic + do + { + var log = new Logging(false, Level.Info, quietConsole: false); + delta--; + } while (delta > 0); + + // wait for delete async task to fire + await Task.Delay(100); + + // get count + files = Directory.GetFiles(logDirectory); + Assert.AreEqual(50, files.Length); + } } }