From a57443fa654c1ce6e9bbdfcc5976114f3f22a220 Mon Sep 17 00:00:00 2001 From: Koji Hasegawa Date: Wed, 27 Nov 2024 02:40:41 +0900 Subject: [PATCH 1/2] Remove IDisposable from AbstractLoggerAsset --- Runtime/Autopilot.cs | 1 - Runtime/Loggers/AbstractLoggerAsset.cs | 9 +-- Runtime/Loggers/CompositeLoggerAsset.cs | 17 +----- Runtime/Loggers/ConsoleLoggerAsset.cs | 18 ------ Runtime/Loggers/FileLoggerAsset.cs | 10 +--- .../Loggers/CompositeLoggerAssetTest.cs | 47 --------------- .../Runtime/Loggers/ConsoleLoggerAssetTest.cs | 20 ------- Tests/Runtime/Loggers/FileLoggerAssetTest.cs | 59 ++----------------- Tests/Runtime/TestDoubles/SpyLoggerAsset.cs | 7 --- 9 files changed, 12 insertions(+), 176 deletions(-) diff --git a/Runtime/Autopilot.cs b/Runtime/Autopilot.cs index dc85927..79e32fa 100644 --- 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(); } /// diff --git a/Runtime/Loggers/AbstractLoggerAsset.cs b/Runtime/Loggers/AbstractLoggerAsset.cs index 820c7e3..9b42fcb 100644 --- a/Runtime/Loggers/AbstractLoggerAsset.cs +++ b/Runtime/Loggers/AbstractLoggerAsset.cs @@ -1,7 +1,6 @@ // Copyright (c) 2023-2024 DeNA Co., Ltd. // This software is released under the MIT License. -using System; using UnityEngine; namespace DeNA.Anjin.Loggers @@ -9,21 +8,19 @@ namespace DeNA.Anjin.Loggers /// /// Abstract logger settings used for autopilot. /// - public abstract class AbstractLoggerAsset : ScriptableObject, IDisposable + public abstract class AbstractLoggerAsset : ScriptableObject { #if UNITY_EDITOR /// /// Description about this agent instance. /// - [Multiline] public string description; + [Multiline] + public string description; #endif /// /// Logger implementation used for autopilot. /// public abstract ILogger Logger { get; } - - /// - public abstract void Dispose(); } } diff --git a/Runtime/Loggers/CompositeLoggerAsset.cs b/Runtime/Loggers/CompositeLoggerAsset.cs index 27b872a..8eb7d2b 100644 --- a/Runtime/Loggers/CompositeLoggerAsset.cs +++ b/Runtime/Loggers/CompositeLoggerAsset.cs @@ -39,13 +39,7 @@ public override ILogger Logger } } - /// - public override void Dispose() - { - _handler?.Dispose(); - } - - private class CompositeLogHandler : ILogHandler, IDisposable + private class CompositeLogHandler : ILogHandler { private readonly List _loggerAssets; private readonly AbstractLoggerAsset _owner; @@ -73,15 +67,6 @@ public void LogException(Exception exception, Object context) loggerAsset.Logger.LogException(exception, context); } } - - /// - 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 index f9ec956..7122995 100644 --- 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; } } - - /// - public override void Dispose() - { - // Nothing to dispose. - } - - [InitializeOnLaunchAutopilot(InitializeOnLaunchAutopilotAttribute.InitializeLoggerOrder)] - public static void ResetLoggers() - { - // Reset runtime instances - var loggerAssets = FindObjectsOfType(); - foreach (var current in loggerAssets) - { - current._logger = null; - } - } } } diff --git a/Runtime/Loggers/FileLoggerAsset.cs b/Runtime/Loggers/FileLoggerAsset.cs index a2a32a4..eb60231 100644 --- a/Runtime/Loggers/FileLoggerAsset.cs +++ b/Runtime/Loggers/FileLoggerAsset.cs @@ -39,7 +39,9 @@ public class FileLoggerAsset : AbstractLoggerAsset /// 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; /// @@ -66,12 +68,6 @@ public override ILogger Logger } } - /// - public override void Dispose() - { - _handler?.Dispose(); - } - private class TimestampCache { private string _timestampCache; diff --git a/Tests/Runtime/Loggers/CompositeLoggerAssetTest.cs b/Tests/Runtime/Loggers/CompositeLoggerAssetTest.cs index 1329a35..973f6f8 100644 --- 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(); - var logger2 = ScriptableObject.CreateInstance(); - var sut = ScriptableObject.CreateInstance(); - 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(); - var sut = ScriptableObject.CreateInstance(); - 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(); - var sut = ScriptableObject.CreateInstance(); - 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 index d582892..f29687b 100644 --- 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(); - 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 index 2297512..bdd0b22 100644 --- 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(); - 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(); - 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,7 +190,6 @@ public async Task LogException_WithTimestamp_WriteTimestamp() var exception = CreateExceptionWithStacktrace(message); sut.Logger.LogException(exception); - sut.Dispose(); await Task.Yield(); var actual = File.ReadAllText(path); @@ -246,23 +197,23 @@ public async Task LogException_WithTimestamp_WriteTimestamp() } [Test] - public async Task ResetLoggers_ResetLoggerAsset() + public async Task ResetLoggers_ResetLogHandler() { var path = LogFilePath; var sut = ScriptableObject.CreateInstance(); 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 index ec7d7f5..bbe83d7 100644 --- 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)>(); From 6f004a29f5d2221e03df6cd0b1b35a2a9ad6f1c1 Mon Sep 17 00:00:00 2001 From: Koji Hasegawa Date: Wed, 27 Nov 2024 09:17:25 +0900 Subject: [PATCH 2/2] Fix reset log handler --- Runtime/Loggers/FileLoggerAsset.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Runtime/Loggers/FileLoggerAsset.cs b/Runtime/Loggers/FileLoggerAsset.cs index eb60231..25846fd 100644 --- a/Runtime/Loggers/FileLoggerAsset.cs +++ b/Runtime/Loggers/FileLoggerAsset.cs @@ -144,7 +144,7 @@ public void Dispose() public static void ResetLoggers() { // Reset runtime instances - var loggerAssets = FindObjectsOfType(); + var loggerAssets = Resources.FindObjectsOfTypeAll(); foreach (var current in loggerAssets) { current._handler?.Dispose();