Merge pull request #2 from chickensoft-games/test/file-writer

test: file writer

.github/workflows/auto_release.yaml

@@ -33,8 +33,6 @@ jobs:
       - name: 🧾 Checkout
         uses: actions/checkout@v4
         with:
-          # Use your GitHub Personal Access Token variable here.
-          token: ${{ secrets.GH_BASIC }}
           lfs: true
           submodules: 'recursive'
 

.github/workflows/release.yaml

@@ -35,7 +35,6 @@ jobs:
       - name: 🧾 Checkout
         uses: actions/checkout@v4
         with:
-          token: ${{ secrets.GH_BASIC }}
           lfs: true
           submodules: 'recursive'
           fetch-depth: 0 # So we can get all tags.

.github/workflows/tests.yaml

@@ -31,7 +31,6 @@ jobs:
       - name: 🧾 Checkout
         uses: actions/checkout@v4
         with:
-          token: ${{ secrets.GH_BASIC }}
           lfs: true
           submodules: 'recursive'
 

Chickensoft.Log.Tests/test/src/FileWriterTest.cs

@@ -0,0 +1,115 @@
+namespace Chickensoft.Log.Tests;
+
+using System;
+using System.IO;
+using Shouldly;
+
+public class FileWriterStreamTester : IDisposable {
+  private bool _isDisposed;
+  private readonly MemoryStream _memoryStream;
+
+  public FileWriterStreamTester(
+    string filename = FileWriter.DEFAULT_FILE_NAME
+  ) {
+    _memoryStream = new MemoryStream();
+
+    FileWriter.AppendText = fileName => new StreamWriter(
+      _memoryStream,
+      System.Text.Encoding.UTF8,
+      bufferSize: 1024,
+      leaveOpen: true
+    );
+  }
+
+  public string GetString() {
+    _memoryStream.Position = 0;
+    using var reader = new StreamReader(_memoryStream);
+    var result = reader.ReadToEnd();
+    return result;
+  }
+
+  public void Dispose() {
+    if (_isDisposed) { return; }
+
+    GC.SuppressFinalize(this);
+    _isDisposed = true;
+
+    FileWriter.AppendText = FileWriter.AppendTextDefault;
+    _memoryStream.Dispose();
+  }
+}
+
+public class FileWriterTest {
+  [Fact]
+  public void DefaultFileName() {
+    FileWriter.DefaultFileName.ShouldBe(FileWriter.DEFAULT_FILE_NAME);
+
+    var filename = "test.log";
+    FileWriter.DefaultFileName = filename;
+    FileWriter.DefaultFileName.ShouldBe(filename);
+
+    FileWriter.DefaultFileName = FileWriter.DEFAULT_FILE_NAME;
+  }
+
+  [Fact]
+  public void DefaultInstance() {
+    var writer = FileWriter.Instance();
+    writer.ShouldNotBeNull();
+    writer.ShouldBeOfType<FileWriter>();
+  }
+
+  [Fact]
+  public void NewInstance() {
+    var filename = "test.log";
+    var writer = FileWriter.Instance(filename);
+    writer.ShouldNotBeNull();
+    writer.ShouldBeOfType<FileWriter>();
+  }
+
+  [Fact]
+  public void ReusesInstanceAndRemoves() {
+    var filename = "test.log";
+    var writer1 = FileWriter.Instance(filename);
+    var writer2 = FileWriter.Instance(filename);
+    writer1.ShouldBeSameAs(writer2);
+
+    FileWriter.Remove(filename).ShouldBeSameAs(writer1);
+    FileWriter.Remove(filename).ShouldBeNull();
+  }
+
+  [Fact]
+  public void WriteMessage() {
+    using var tester = new FileWriterStreamTester();
+
+    var writer = FileWriter.Instance();
+    var value = "test message";
+
+    writer.WriteMessage(value);
+
+    tester.GetString().ShouldBe(value + Environment.NewLine);
+  }
+
+  [Fact]
+  public void WriteWarning() {
+    using var tester = new FileWriterStreamTester();
+
+    var writer = FileWriter.Instance();
+    var value = "test message";
+
+    writer.WriteWarning(value);
+
+    tester.GetString().ShouldBe(value + Environment.NewLine);
+  }
+
+  [Fact]
+  public void WriteError() {
+    using var tester = new FileWriterStreamTester();
+
+    var writer = FileWriter.Instance();
+    var value = "test message";
+
+    writer.WriteError(value);
+
+    tester.GetString().ShouldBe(value + Environment.NewLine);
+  }
+}

Chickensoft.Log.Tests/test/src/LogTest.cs

@@ -237,4 +237,19 @@ public void PrintsStackTraceWithMessage() {
       Invoked.Once
     );
   }
+
+  [Fact]
+  public void AddsAndRemovesWriters() {
+    var writerA = new Mock<ILogWriter>();
+    var writerB = new Mock<ILogWriter>();
+    var log = new Log(nameof(LogTest), [writerA.Object]);
+
+    log.AddWriter(writerB.Object);
+
+    log._writers.ShouldBe([writerA.Object, writerB.Object]);
+
+    log.RemoveWriter(writerA.Object);
+
+    log._writers.ShouldBe([writerB.Object]);
+  }
 }

Chickensoft.Log/src/Assembly.cs

@@ -0,0 +1,3 @@
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Chickensoft.Log.Tests")]

Chickensoft.Log/src/FileWriter.cs

@@ -1,22 +1,32 @@
 namespace Chickensoft.Log;
 
 using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
 using System.IO;
 
 /// <summary>
 /// An <see cref="ILogWriter"/> that directs output of an <see cref="ILog"/>
 /// to a file.
 /// </summary>
-[ExcludeFromCodeCoverage(Justification = "File output is untestable")]
 public sealed class FileWriter : ILogWriter {
+  internal delegate StreamWriter AppendTextDelegate(string text);
+  internal static AppendTextDelegate AppendTextDefault { get; } =
+    File.AppendText;
+  internal static AppendTextDelegate AppendText { get; set; } =
+    AppendTextDefault;
+
   // protect static members from simultaneous thread access
   private static readonly object _singletonLock = new();
   // Implemented as a pseudo-singleton to enforce one truncation per file per
   // execution
   private static readonly Dictionary<string, FileWriter> _instances = [];
 
-  private static string _defaultFileName = "output.log";
+  /// <summary>The default filename for logs.</summary>
+  public const string DEFAULT_FILE_NAME = "output.log";
+
+#pragma warning disable IDE0032 // Use auto property
+  private static string _defaultFileName = DEFAULT_FILE_NAME;
+#pragma warning restore IDE0032 // Use auto property
+
   /// <summary>
   /// The default file name that will be used when creating a
   /// <see cref="FileWriter"/> if no filename is specified. Defaults to
@@ -87,6 +97,24 @@ public static FileWriter Instance() {
     }
   }
 
+  /// <summary>
+  /// Remove a <see cref="FileWriter"/> that had previously been created.
+  /// While not necessary, this can free up resources if writing to many
+  /// different log files.
+  /// </summary>
+  /// <param name="fileName">Filename for the log.</param>
+  /// <returns>The file writer, if one existed for the given filename.
+  /// Otherwise, just null.</returns>
+  public static FileWriter? Remove(string fileName) {
+    lock (_singletonLock) {
+      if (_instances.TryGetValue(fileName, out var writer)) {
+        _instances.Remove(fileName);
+        return writer;
+      }
+    }
+    return null;
+  }
+
   private readonly object _writingLock = new();
 
   /// <summary>
@@ -102,14 +130,10 @@ private FileWriter(string fileName) {
     }
   }
 
-  [SuppressMessage("Style",
-    "IDE0063:Use simple 'using' statement",
-    Justification = "Prefer block, to explicitly delineate scope")]
   private void WriteLine(string message) {
     lock (_writingLock) {
-      using (var sw = File.AppendText(FileName)) {
-        sw.WriteLine(message);
-      }
+      using var sw = AppendText(FileName);
+      sw.WriteLine(message);
     }
   }
 

Chickensoft.Log/src/Log.cs

@@ -13,7 +13,7 @@ public sealed class Log : ILog {
   /// <inheritdoc/>
   public string Name { get; }
 
-  private readonly List<ILogWriter> _writers = [];
+  internal readonly List<ILogWriter> _writers = [];
 
   /// <summary>
   /// The formatter that will be used to format messages before writing them