Merge pull request #120 from nowsprinting/fix/logger

Fix FileLogger doesn't work when Autopilot is stopped and re-run
DeNA · Nov 27, 2024 · 792ef5d · 792ef5d
2 parents faff502 + 6f004a2
commit 792ef5d
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 177 deletions.
diff --git a/Runtime/Autopilot.cs b/Runtime/Autopilot.cs
@@ -148,7 +148,6 @@ private void OnDestroy()
 
             _logger?.Log("Destroy Autopilot object");
             _dispatcher?.Dispose();
-            _settings.LoggerAsset?.Dispose();
         }
 
         /// <inheritdoc/>

diff --git a/Runtime/Loggers/AbstractLoggerAsset.cs b/Runtime/Loggers/AbstractLoggerAsset.cs
@@ -1,29 +1,26 @@
 // Copyright (c) 2023-2024 DeNA Co., Ltd.
 // This software is released under the MIT License.
 
-using System;
 using UnityEngine;
 
 namespace DeNA.Anjin.Loggers
 {
     /// <summary>
     /// Abstract logger settings used for autopilot.
     /// </summary>
-    public abstract class AbstractLoggerAsset : ScriptableObject, IDisposable
+    public abstract class AbstractLoggerAsset : ScriptableObject
     {
 #if UNITY_EDITOR
         /// <summary>
         /// Description about this agent instance.
         /// </summary>
-        [Multiline] public string description;
+        [Multiline]
+        public string description;
 #endif
 
         /// <summary>
         /// Logger implementation used for autopilot.
         /// </summary>
         public abstract ILogger Logger { get; }
-
-        /// <inheritdoc />
-        public abstract void Dispose();
     }
 }
diff --git a/Runtime/Loggers/CompositeLoggerAsset.cs b/Runtime/Loggers/CompositeLoggerAsset.cs
@@ -39,13 +39,7 @@ public override ILogger Logger
             }
         }
 
-        /// <inheritdoc />
-        public override void Dispose()
-        {
-            _handler?.Dispose();
-        }
-
-        private class CompositeLogHandler : ILogHandler, IDisposable
+        private class CompositeLogHandler : ILogHandler
         {
             private readonly List<AbstractLoggerAsset> _loggerAssets;
             private readonly AbstractLoggerAsset _owner;
@@ -73,15 +67,6 @@ public void LogException(Exception exception, Object context)
                     loggerAsset.Logger.LogException(exception, context);
                 }
             }
-
-            /// <inheritdoc />
-            public void Dispose()
-            {
-                foreach (var loggerAsset in _loggerAssets.Where(x => x != null && x != _owner))
-                {
-                    loggerAsset.Dispose();
-                }
-            }
         }
     }
 }
diff --git a/Runtime/Loggers/ConsoleLoggerAsset.cs b/Runtime/Loggers/ConsoleLoggerAsset.cs
@@ -1,7 +1,6 @@
 // Copyright (c) 2023-2024 DeNA Co., Ltd.
 // This software is released under the MIT License.
 
-using DeNA.Anjin.Attributes;
 using UnityEngine;
 
 namespace DeNA.Anjin.Loggers
@@ -34,22 +33,5 @@ public override ILogger Logger
                 return _logger;
             }
         }
-
-        /// <inheritdoc />
-        public override void Dispose()
-        {
-            // Nothing to dispose.
-        }
-
-        [InitializeOnLaunchAutopilot(InitializeOnLaunchAutopilotAttribute.InitializeLoggerOrder)]
-        public static void ResetLoggers()
-        {
-            // Reset runtime instances
-            var loggerAssets = FindObjectsOfType<ConsoleLoggerAsset>();
-            foreach (var current in loggerAssets)
-            {
-                current._logger = null;
-            }
-        }
     }
 }
diff --git a/Runtime/Loggers/FileLoggerAsset.cs b/Runtime/Loggers/FileLoggerAsset.cs
@@ -39,7 +39,9 @@ public class FileLoggerAsset : AbstractLoggerAsset
         /// </summary>
         public bool timestamp = true;
 
-        private FileLogHandler _handler;
+#pragma warning disable IDISP006
+        private FileLogHandler _handler; // Note: dispose in ResetLoggers
+#pragma warning restore IDISP006
         private ILogger _logger;
 
         /// <inheritdoc />
@@ -66,12 +68,6 @@ public override ILogger Logger
             }
         }
 
-        /// <inheritdoc />
-        public override void Dispose()
-        {
-            _handler?.Dispose();
-        }
-
         private class TimestampCache
         {
             private string _timestampCache;
@@ -148,7 +144,7 @@ public void Dispose()
         public static void ResetLoggers()
         {
             // Reset runtime instances
-            var loggerAssets = FindObjectsOfType<FileLoggerAsset>();
+            var loggerAssets = Resources.FindObjectsOfTypeAll<FileLoggerAsset>();
             foreach (var current in loggerAssets)
             {
                 current._handler?.Dispose();

diff --git a/Tests/Runtime/Loggers/CompositeLoggerAssetTest.cs b/Tests/Runtime/Loggers/CompositeLoggerAssetTest.cs
@@ -113,52 +113,5 @@ public void LogException_NestingLogger_IgnoreNested()
 
             Assert.That(logger2.Logs, Does.Contain((LogType.Exception, exception.ToString())));
         }
-
-        [Test]
-        public void Dispose_CompositeMultipleLoggers_DisposeAllLoggers()
-        {
-            var logger1 = ScriptableObject.CreateInstance<SpyLoggerAsset>();
-            var logger2 = ScriptableObject.CreateInstance<SpyLoggerAsset>();
-            var sut = ScriptableObject.CreateInstance<CompositeLoggerAsset>();
-            sut.loggerAssets.Add(logger1);
-            sut.loggerAssets.Add(logger2);
-
-            var message = TestContext.CurrentContext.Test.Name;
-            sut.Logger.Log(message);
-            sut.Dispose();
-
-            Assert.That(logger1.Disposed, Is.True);
-            Assert.That(logger2.Disposed, Is.True);
-        }
-
-        [Test]
-        public void Dispose_LoggersIncludesNull_IgnoreNull()
-        {
-            var logger2 = ScriptableObject.CreateInstance<SpyLoggerAsset>();
-            var sut = ScriptableObject.CreateInstance<CompositeLoggerAsset>();
-            sut.loggerAssets.Add(null);
-            sut.loggerAssets.Add(logger2);
-
-            var message = TestContext.CurrentContext.Test.Name;
-            sut.Logger.Log(message);
-            sut.Dispose();
-
-            Assert.That(logger2.Disposed, Is.True);
-        }
-
-        [Test]
-        public void Dispose_NestingLogger_IgnoreNested()
-        {
-            var logger2 = ScriptableObject.CreateInstance<SpyLoggerAsset>();
-            var sut = ScriptableObject.CreateInstance<CompositeLoggerAsset>();
-            sut.loggerAssets.Add(sut); // nesting
-            sut.loggerAssets.Add(logger2);
-
-            var message = TestContext.CurrentContext.Test.Name;
-            sut.Logger.Log(message);
-            sut.Dispose();
-
-            Assert.That(logger2.Disposed, Is.True);
-        }
     }
 }
diff --git a/Tests/Runtime/Loggers/ConsoleLoggerAssetTest.cs b/Tests/Runtime/Loggers/ConsoleLoggerAssetTest.cs
@@ -4,9 +4,6 @@
 using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.TestTools;
-#if UNITY_EDITOR
-using UnityEditor;
-#endif
 
 namespace DeNA.Anjin.Loggers
 {
@@ -40,22 +37,5 @@ public void FilterLogTypeSpecified(
             LogAssert.Expect(logType, message);
             LogAssert.NoUnexpectedReceived();
         }
-
-        [Test]
-        public void ResetLoggers_ResetLoggerAsset()
-        {
-            var sut = ScriptableObject.CreateInstance<ConsoleLoggerAsset>();
-            sut.filterLogType = LogType.Warning;
-            sut.Logger.Log("Before reset");
-            sut.Dispose();
-            Assume.That(sut.Logger.filterLogType, Is.EqualTo(LogType.Warning));
-
-            ConsoleLoggerAsset.ResetLoggers(); // Called when on launch autopilot
-
-            sut.filterLogType = LogType.Error;
-            sut.Logger.Log("After reset");
-            sut.Dispose();
-            Assert.That(sut.Logger.filterLogType, Is.EqualTo(LogType.Error));
-        }
     }
 }
diff --git a/Tests/Runtime/Loggers/FileLoggerAssetTest.cs b/Tests/Runtime/Loggers/FileLoggerAssetTest.cs
@@ -66,7 +66,6 @@ public async Task GetLogger_DirectoryDoesNotExist_CreateDirectoryAndWriteToFile(
             sut.timestamp = false;
 
             sut.Logger.Log(message);
-            sut.Dispose();
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -85,7 +84,6 @@ public async Task GetLogger_FileExists_OverwrittenToFile()
             sut.timestamp = false;
 
             sut.Logger.Log(message);
-            sut.Dispose();
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -114,7 +112,6 @@ public async Task GetLogger_OutputPathIsAbsolutePath_WriteToFile()
             sut.timestamp = false;
 
             sut.Logger.Log(message);
-            sut.Dispose();
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -135,28 +132,6 @@ public async Task LogFormat_WriteToFile()
 
             sut.Logger.Log(message);
             sut.Logger.Log(message);
-            sut.Dispose();
-            await Task.Yield();
-
-            var actual = File.ReadAllText(path);
-            Assert.That(actual, Is.EqualTo(message + Environment.NewLine + message + Environment.NewLine));
-        }
-
-        [Test]
-        public async Task LogFormat_Disposed_NotWriteToFile()
-        {
-            var message = TestContext.CurrentContext.Test.Name;
-            var path = LogFilePath;
-            var sut = ScriptableObject.CreateInstance<FileLoggerAsset>();
-            sut.outputPath = path;
-            sut.timestamp = false;
-
-            sut.Logger.Log(message);
-            sut.Logger.Log(message);
-            sut.Dispose();
-            await Task.Yield();
-
-            sut.Logger.Log(message); // write after disposed
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -176,7 +151,6 @@ public async Task LogFormat_WithTimestamp_WriteTimestamp()
             await UniTask.NextFrame();
             sut.Logger.Log(message);
             sut.Logger.Log(message); // using cache
-            sut.Dispose();
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -199,28 +173,6 @@ public async Task LogException_WriteToFile()
 
             var exception = CreateExceptionWithStacktrace(message);
             sut.Logger.LogException(exception);
-            sut.Dispose();
-            await Task.Yield();
-
-            var actual = File.ReadAllText(path);
-            Assert.That(actual, Is.EqualTo(exception + Environment.NewLine));
-        }
-
-        [Test]
-        public async Task LogException_Disposed_NotWriteToFile()
-        {
-            var message = TestContext.CurrentContext.Test.Name;
-            var path = LogFilePath;
-            var sut = ScriptableObject.CreateInstance<FileLoggerAsset>();
-            sut.outputPath = path;
-            sut.timestamp = false;
-
-            var exception = CreateExceptionWithStacktrace(message);
-            sut.Logger.LogException(exception);
-            sut.Dispose();
-            await Task.Yield();
-
-            sut.Logger.LogException(exception); // write after disposed
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
@@ -238,31 +190,30 @@ public async Task LogException_WithTimestamp_WriteTimestamp()
 
             var exception = CreateExceptionWithStacktrace(message);
             sut.Logger.LogException(exception);
-            sut.Dispose();
             await Task.Yield();
 
             var actual = File.ReadAllText(path);
             Assert.That(actual, Does.Match("^" + TimestampRegex + ".*"));
         }
 
         [Test]
-        public async Task ResetLoggers_ResetLoggerAsset()
+        public async Task ResetLoggers_ResetLogHandler()
         {
             var path = LogFilePath;
             var sut = ScriptableObject.CreateInstance<FileLoggerAsset>();
             sut.outputPath = path;
+            sut.timestamp = false;
             sut.Logger.Log("Before reset");
-            sut.Dispose();
             await Task.Yield();
             Assume.That(path, Does.Exist);
 
-            File.Delete(path);
             FileLoggerAsset.ResetLoggers(); // Called when on launch autopilot
 
             sut.Logger.Log("After reset");
-            sut.Dispose();
             await Task.Yield();
-            Assert.That(path, Does.Exist);
+
+            var actual = File.ReadAllText(path);
+            Assert.That(actual, Is.EqualTo("After reset" + Environment.NewLine));
         }
     }
 }
diff --git a/Tests/Runtime/TestDoubles/SpyLoggerAsset.cs b/Tests/Runtime/TestDoubles/SpyLoggerAsset.cs
@@ -11,8 +11,6 @@ namespace DeNA.Anjin.TestDoubles
 {
     public class SpyLoggerAsset : AbstractLoggerAsset
     {
-        public bool Disposed { get; private set; }
-
         private SpyLogHandler _handler;
         private ILogger _logger;
 
@@ -33,11 +31,6 @@ public override ILogger Logger
 
         public List<(LogType logType, string message)> Logs => _handler.Logs;
 
-        public override void Dispose()
-        {
-            Disposed = true;
-        }
-
         private class SpyLogHandler : ILogHandler
         {
             public readonly List<(LogType logType, string message)> Logs = new List<(LogType, string)>();