Skip to content

Commit

Permalink
More logging patches
Browse files Browse the repository at this point in the history
The PhysicalConsoleLogger had shared static state that was causing the unit tests to fail.

The rabbithole of investigation led to some other small fixes and comments.

Related to c96ea6e and 3e1a67a
  • Loading branch information
atruskie committed Feb 14, 2019
1 parent c96ea6e commit 9e9e56d
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 76 deletions.
31 changes: 27 additions & 4 deletions src/Acoustics.Shared/Logging/Log4NetTextWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ namespace Acoustics.Shared.Logging
using System.Text;
using log4net;

/// <summary>
/// Forwards chars from a text writer to a log as well.
/// </summary>
/// <remarks>
/// We generally expect the log not to output to the console as well since
/// this class copies events to the log and then sends them to the base stream,
/// which should be the console.
///
/// Thus using the <see cref="NoConsole.Log"/> logger is a good choice.
/// </remarks>
public class Log4NetTextWriter : TextWriter
{
private readonly TextWriter baseStream;
Expand Down Expand Up @@ -47,8 +57,7 @@ public override void Write(char value)
case '\r':
break;
case '\n':
this.logCall(this.stringBuilder.ToString());
this.stringBuilder.Clear();
this.Flush();
break;
default:
this.stringBuilder.Append(value);
Expand All @@ -63,8 +72,9 @@ public override void Write(string value)
switch (value)
{
case string _ when value.EndsWith(Environment.NewLine):
this.logCall(this.stringBuilder.ToString());
this.stringBuilder.Clear();
var valueWithoutNewline = value.Substring(0, value.Length - Environment.NewLine.Length);
this.stringBuilder.Append(valueWithoutNewline);
this.Flush();
break;
default:
this.stringBuilder.Append(value);
Expand All @@ -73,5 +83,18 @@ public override void Write(string value)

this.baseStream.Write(value);
}

public override void Flush()
{
this.logCall(this.stringBuilder.ToString());
this.stringBuilder.Clear();
base.Flush();
}

protected override void Dispose(bool disposing)
{
this.Flush();
base.Dispose(disposing);
}
}
}
4 changes: 2 additions & 2 deletions src/Acoustics.Shared/Logging/LogExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// --------------------------------------------------------------------------------------------------------------------
// --------------------------------------------------------------------------------------------------------------------
// <copyright file="LogExtensions.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>
Expand All @@ -24,7 +24,7 @@ public static class LogExtensions

/// <summary>
/// Log a message object with the <see cref="F:LogExtensions.PromptLevel"/> level.
/// Use this mehtod only for interactive prompts that the user must see.
/// Use this method only for interactive prompts that the user must see.
/// </summary>
/// <param name="log">
/// The log.
Expand Down
11 changes: 6 additions & 5 deletions src/Acoustics.Shared/Logging/LoggedConsole.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,14 @@ public static void WriteLine(object obj)
Log.Info(obj);
}

public static void WriteError(string str)
{
Log.Error(str);
}

public static void WriteErrorLine(string format, params object[] args)
{
if (args.Length == 0)
{
Log.Error(format);
return;
}

var str = string.Format(format, args);
Log.Error(str);
}
Expand Down
3 changes: 3 additions & 0 deletions src/Acoustics.Shared/Logging/Logging.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ 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;
Expand Down Expand Up @@ -59,6 +60,8 @@ internal Logging(
Level defaultLevel,
bool quietConsole)
{
LogManager.ResetConfiguration();

this.repository = (Hierarchy)LogManager.GetRepository();

this.repository.LevelMap.Add(LogExtensions.PromptLevel);
Expand Down
4 changes: 2 additions & 2 deletions src/Acoustics.Shared/Logging/NoConsole.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="NoConsole.cs" company="QutEcoacoustics">
// <copyright file="NoConsole.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>

Expand All @@ -10,7 +10,7 @@ namespace System

/// <summary>
/// A quiet logger that only logs to the log file. Requires appropriate logger
/// connfiguration.
/// configuration.
/// </summary>
public static class NoConsole
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// --------------------------------------------------------------------------------------------------------------------
// --------------------------------------------------------------------------------------------------------------------
// <copyright file="AnalyseLongRecording.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>
Expand Down Expand Up @@ -379,7 +379,7 @@ public static T FindAndCheckAnalyser<T>(string analysisIdentifier, string partia
if (analyser == null)
{
var error = $"We can not determine what analysis you want to run. We tried to search for \"{searchName}\"";
LoggedConsole.WriteError(error);
LoggedConsole.WriteErrorLine(error);
var knownAnalyzers = analysers.Aggregate(string.Empty, (a, i) => a + $" {i.Identifier}\n");
LoggedConsole.WriteLine("Available analysers are:\n" + knownAnalyzers);

Expand Down
4 changes: 2 additions & 2 deletions src/AnalysisPrograms/AudioCutter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="AudioCutter.cs" company="QutEcoacoustics">
// <copyright file="AudioCutter.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>

Expand Down Expand Up @@ -273,7 +273,7 @@ private static async Task<double> CreateSegment(
}
catch (IOException ioex)
{
LoggedConsole.WriteError($"Failed to cut segment {itemNumber} of {itemCount}:" + ioex.Message);
LoggedConsole.WriteErrorLine($"Failed to cut segment {itemNumber} of {itemCount}:" + ioex.Message);
return double.NaN;
}

Expand Down
2 changes: 1 addition & 1 deletion src/AnalysisPrograms/Production/MainEntryUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ private static void AttachExceptionHandler()

private static CommandLineApplication CreateCommandLineApplication()
{
var console = PhysicalConsoleLogger.Default;
var console = new PhysicalConsoleLogger();
var app = CommandLineApplication = new CommandLineApplication<MainArgs>(console);

app.HelpTextGenerator = new CustomHelpTextGenerator { EnvironmentOptions = EnvironmentOptions };
Expand Down
4 changes: 1 addition & 3 deletions src/AnalysisPrograms/Production/PhysicalConsoleLogger.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="PhysicalConsoleLogger.cs" company="QutEcoacoustics">
// <copyright file="PhysicalConsoleLogger.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>

Expand Down Expand Up @@ -28,8 +28,6 @@ public event ConsoleCancelEventHandler CancelKeyPress
remove => Console.CancelKeyPress -= value;
}

public static PhysicalConsoleLogger Default { get; } = new PhysicalConsoleLogger();

public TextWriter Out => this.outWriter;

public TextWriter Error => this.errorWriter;
Expand Down
4 changes: 2 additions & 2 deletions src/AudioAnalysisTools/SpectralClustering.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// --------------------------------------------------------------------------------------------------------------------
// --------------------------------------------------------------------------------------------------------------------
// <copyright file="SpectralClustering.cs" company="QutEcoacoustics">
// All code in this file and all associated files are the copyright and property of the QUT Ecoacoustics Research Group (formerly MQUTeR, and formerly QUT Bioacoustics Research Group).
// </copyright>
Expand Down Expand Up @@ -620,7 +620,7 @@ public static void SaveAndViewSpectrogramImage(Image image, string opDir, string
var fiImage = new FileInfo(imagePath);
if (fiImage.Exists)
{
LoggedConsole.WriteError("Showing image is no longer supported");
LoggedConsole.WriteErrorLine("Showing image is no longer supported");
}
}

Expand Down
22 changes: 22 additions & 0 deletions tests/Acoustics.Test/AnalysisPrograms/MainEntryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace Acoustics.Test.AnalysisPrograms
using System;
using System.Collections.ObjectModel;
using System.Threading.Tasks;
using Accord.Math.Optimization;
using Acoustics.Shared;
using Acoustics.Test.TestHelpers;
using global::AnalysisPrograms;
Expand All @@ -16,6 +17,7 @@ namespace Acoustics.Test.AnalysisPrograms
[DoNotParallelize]
public class MainEntryTests
{
[DoNotParallelize]
[TestMethod]
public async Task DefaultCliWorks()
{
Expand All @@ -30,6 +32,7 @@ public async Task DefaultCliWorks()
}
}

[DoNotParallelize]
[TestMethod]
public async Task DefaultHelpWorks()
{
Expand All @@ -41,9 +44,11 @@ public async Task DefaultHelpWorks()

this.AssertContainsCopyright(console.Lines);
this.AssertContainsGitHashAndVersion(console.Lines);
StringAssert.StartsWith(console.Lines[6], Meta.Description);
}
}

[DoNotParallelize]
[TestMethod]
public async Task DefaultVersionWorks()
{
Expand All @@ -55,6 +60,23 @@ public async Task DefaultVersionWorks()

this.AssertContainsCopyright(console.Lines);
this.AssertContainsGitHashAndVersion(console.Lines);
StringAssert.StartsWith(console.Lines[3], BuildMetadata.VersionString);
}
}

[DoNotParallelize]
[TestMethod]
public async Task CheckEnvironmentWorks()
{
using (var console = new ConsoleRedirector())
{
var code = await MainEntry.Main(new[] { "CheckEnvironment" });

Assert.AreEqual(0, code);

this.AssertContainsCopyright(console.Lines);
this.AssertContainsGitHashAndVersion(console.Lines);
StringAssert.Contains(console.Lines[4], "SUCCESS - Valid environment");
}
}

Expand Down
79 changes: 26 additions & 53 deletions tests/Acoustics.Test/TestHelpers/ConsoleRedirector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,47 +4,46 @@ namespace Acoustics.Test.TestHelpers
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Text;

internal class ConsoleRedirector : IDisposable
{
private readonly ArrayStringWriter consoleOutput = new ArrayStringWriter();
private readonly ArrayStringWriter capturedOutput = new ArrayStringWriter();
private readonly TextWriter originalConsoleOutput;

public ConsoleRedirector()
{
this.originalConsoleOutput = Console.Out;
Console.SetOut(this.consoleOutput);
Console.Out.Close();
Console.SetOut(this.capturedOutput);
}

public ReadOnlyCollection<string> Lines => this.consoleOutput.Lines;
public ReadOnlyCollection<string> Lines => this.capturedOutput.Lines;

public void Dispose()
{
Console.SetOut(this.originalConsoleOutput);
LoggedConsole.Write("------CAPTURED CONSOLE OUTPUT--------\n" + this.GetString());
this.consoleOutput.Dispose();
var writer = new StreamWriter(Console.OpenStandardOutput())
{
AutoFlush = true,
};
Console.SetOut(writer);

LoggedConsole.Write($"------CAPTURED CONSOLE OUTPUT ({this.Lines.Count} lines)--------\n{this.GetString()}");
this.capturedOutput.Dispose();
}

public string GetString() => string.Join(Environment.NewLine, this.consoleOutput.Lines);
public string GetString() => string.Join(Environment.NewLine, this.capturedOutput.Lines);

public class ArrayStringWriter : TextWriter
{
private static readonly char[] NewLineChars = Environment.NewLine.ToCharArray();
private readonly List<string> lines;
private bool halfLine;

public ArrayStringWriter(int size)
{
this.lines = new List<string>(size)
{
string.Empty,
};

if (NewLineChars.Length > 2)
{
throw new PlatformNotSupportedException("Cannot process a new line token that had more than two chars");
}
}

public ArrayStringWriter()
Expand All @@ -58,53 +57,26 @@ public ArrayStringWriter()

public override void Write(char c)
{
bool match = NewLineChars[0] == c;

if (NewLineChars.Length == 1)
switch (c)
{
if (match)
{
case '\r':
break;
case '\n':
this.lines.Add(string.Empty);
}
else
{
this.lines[this.lines.Count - 1] += c;
}
}
else
{
if (match)
{
this.halfLine = true;
}
else if (this.halfLine)
{
this.halfLine = false;
if (NewLineChars[1] == c)
{
this.lines.Add(string.Empty);
}
else
{
this.lines[this.lines.Count - 1] += c;
}
}
else
{
break;
default:
this.lines[this.lines.Count - 1] += c;
}
break;
}
}

public override void Write(string value)
{
this.halfLine = false;
this.Append(value);
}

public override void WriteLine(string value)
{
this.halfLine = false;
this.AddNewLine(value);
}

Expand All @@ -127,15 +99,16 @@ private void Append(string s)

var split = s.Split(new[] { Environment.NewLine }, StringSplitOptions.None);

if (split.Length == 1)
if (split.Length > 0)
{
this.lines[this.lines.Count - 1] += split[0];
}
else if (split.Length > 1)

if (split.Length > 1)
{
foreach (var line in split)
foreach (var line in split.Skip(1))
{
this.AddNewLine(line);
this.lines.Add(line);
}
}
}
Expand Down

0 comments on commit 9e9e56d

Please sign in to comment.