Skip to content

Commit

Permalink
Changes from review and refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
CharliePoole committed Dec 9, 2024
1 parent 4330169 commit 2550a11
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 156 deletions.
15 changes: 1 addition & 14 deletions src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public void NoInputFiles()
[TestCase("DisposeRunners", "dispose-runners")]
[TestCase("TeamCity", "teamcity")]
[TestCase("SkipNonTestAssemblies", "skipnontestassemblies")]
[TestCase("NoResult", "noresult")]
#if NETFRAMEWORK
[TestCase("RunAsX86", "x86")]
[TestCase("ShadowCopyFiles", "shadowcopy")]
Expand Down Expand Up @@ -505,20 +506,6 @@ public void DefaultResultSpecification()
Assert.That(spec.Transform, Is.Null);
}

[Test]
public void NoResultSuppressesDefaultResultSpecification()
{
var options = ConsoleMocks.Options("test.dll", "-noresult");
Assert.That(options.ResultOutputSpecifications.Count, Is.EqualTo(0));
}

[Test]
public void NoResultSuppressesAllResultSpecifications()
{
var options = ConsoleMocks.Options("test.dll", "-result:results.xml", "-noresult", "-result:nunit2results.xml;format=nunit2");
Assert.That(options.ResultOutputSpecifications.Count, Is.EqualTo(0));
}

[Test]
public void InvalidResultSpecRecordsError()
{
Expand Down
29 changes: 15 additions & 14 deletions src/NUnitConsole/nunit3-console/ConsoleOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ public class ConsoleOptions : OptionSet
{
private static readonly string CURRENT_DIRECTORY_ON_ENTRY = Directory.GetCurrentDirectory();

private bool validated;
private bool noresult;
private bool _validated;

/// <summary>
/// An abstraction of the file system
Expand Down Expand Up @@ -52,13 +51,13 @@ internal ConsoleOptions(IDefaultOptionsProvider defaultOptionsProvider, IFileSys
public bool ListExtensions { get; private set; }

// Additional directories to be used to search for user extensions
public IList<string> ExtensionDirectories { get; } = new List<string>();
public List<string> ExtensionDirectories { get; } = new List<string>();

// Select tests

public IList<string> InputFiles { get; } = new List<string>();
public List<string> InputFiles { get; } = new List<string>();

public IList<string> TestList { get; } = new List<string>();
public List<string> TestList { get; } = new List<string>();

public IDictionary<string, string> TestParameters { get; } = new Dictionary<string, string>();

Expand Down Expand Up @@ -104,13 +103,15 @@ public string WorkDirectory
public string InternalTraceLevel { get; private set; }
public bool InternalTraceLevelSpecified { get { return InternalTraceLevel != null; } }

public bool NoResult { get; private set; }

private readonly List<OutputSpecification> resultOutputSpecifications = new List<OutputSpecification>();
public IList<OutputSpecification> ResultOutputSpecifications
public List<OutputSpecification> ResultOutputSpecifications
{
get
{
if (noresult)
return new OutputSpecification[0];
//if (noresult)
// return new List<OutputSpecification>();

if (resultOutputSpecifications.Count == 0)
resultOutputSpecifications.Add(
Expand All @@ -120,7 +121,7 @@ public IList<OutputSpecification> ResultOutputSpecifications
}
}

public IList<OutputSpecification> ExploreOutputSpecifications { get; } = new List<OutputSpecification>();
public List<OutputSpecification> ExploreOutputSpecifications { get; } = new List<OutputSpecification>();

public string ActiveConfig { get; private set; }
public bool ActiveConfigSpecified { get { return ActiveConfig != null; } }
Expand Down Expand Up @@ -162,9 +163,9 @@ public IList<OutputSpecification> ResultOutputSpecifications

public string PrincipalPolicy { get; private set; }

public IList<string> WarningMessages { get; } = new List<string>();
public List<string> WarningMessages { get; } = new List<string>();

public IList<string> ErrorMessages { get; } = new List<string>();
public List<string> ErrorMessages { get; } = new List<string>();

private void ConfigureOptions()
{
Expand Down Expand Up @@ -277,7 +278,7 @@ private void ConfigureOptions()
});

this.Add("noresult", "Don't save any test results.",
v => noresult = v != null);
v => NoResult = v != null);

this.Add("labels=", "Specify whether to write test case names to the output. Values: Off, OnOutputOnly, Before, After, BeforeAndAfter",
v => {
Expand Down Expand Up @@ -419,11 +420,11 @@ private Action<string> NetFxOnlyOption(string optionName, Action<string> action)

public bool Validate()
{
if (!validated)
if (!_validated)
{
CheckOptionCombinations();

validated = true;
_validated = true;
}

return ErrorMessages.Count == 0;
Expand Down
83 changes: 43 additions & 40 deletions src/NUnitConsole/nunit3-console/ConsoleRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using NUnit.Engine.Extensibility;
using System.Runtime.InteropServices;
using System.Text;
using System.Linq;

namespace NUnit.ConsoleRunner
{
Expand Down Expand Up @@ -72,9 +73,11 @@ public ConsoleRunner(ITestEngine engine, ConsoleOptions options, ExtendedTextWri
foreach (string extensionDirectory in _options.ExtensionDirectories)
_extensionService.FindExtensions(extensionDirectory);

_workDirectory = options.WorkDirectory ?? Directory.GetCurrentDirectory();
if (!Directory.Exists(_workDirectory))
_workDirectory = options.WorkDirectory;
if (_workDirectory != null)
Directory.CreateDirectory(_workDirectory);
else
_workDirectory = null;

if (_options.TeamCity)
{
Expand Down Expand Up @@ -164,49 +167,49 @@ private int RunTests(TestPackage package, TestFilter filter)
{
var writer = new ColorConsoleWriter(!_options.NoColor);

foreach (var spec in _options.ResultOutputSpecifications)
{
var outputPath = Path.Combine(_workDirectory, spec.OutputPath);
if (!_options.NoResult)
foreach (var spec in _options.ResultOutputSpecifications)
{
var outputPath = Path.Combine(_workDirectory, spec.OutputPath);

IResultWriter resultWriter;
IResultWriter resultWriter;

try
{
resultWriter = GetResultWriter(spec);
}
catch (Exception ex)
{
throw new NUnitEngineException($"Error encountered in resolving output specification: {spec}", ex);
}
try
{
resultWriter = GetResultWriter(spec);
}
catch (Exception ex)
{
throw new NUnitEngineException($"Error encountered in resolving output specification: {spec}", ex);
}

try
{
var outputDirectory = Path.GetDirectoryName(outputPath);
Directory.CreateDirectory(outputDirectory);
}
catch (Exception ex)
{
writer.WriteLine(ColorStyle.Error, String.Format(
"The directory in --result {0} could not be created",
spec.OutputPath));
writer.WriteLine(ColorStyle.Error, ExceptionHelper.BuildMessage(ex));
return ConsoleRunner.UNEXPECTED_ERROR;
}
try
{
var outputDirectory = Path.GetDirectoryName(outputPath);
Directory.CreateDirectory(outputDirectory);
}
catch (Exception ex)
{
writer.WriteLine(ColorStyle.Error, String.Format(
"The directory in --result {0} could not be created",
spec.OutputPath));
writer.WriteLine(ColorStyle.Error, ExceptionHelper.BuildMessage(ex));
return ConsoleRunner.UNEXPECTED_ERROR;
}

try
{
resultWriter.CheckWritability(outputPath);
}
catch (Exception ex)
{
throw new NUnitEngineException(
String.Format(
"The path specified in --result {0} could not be written to",
spec.OutputPath), ex);
try
{
resultWriter.CheckWritability(outputPath);
}
catch (Exception ex)
{
throw new NUnitEngineException(
String.Format(
"The path specified in --result {0} could not be written to",
spec.OutputPath), ex);
}
}

}

var labels = _options.DisplayTestLabels != null
? _options.DisplayTestLabels.ToUpperInvariant()
: "ON";
Expand Down Expand Up @@ -335,7 +338,7 @@ private void DisplayExtensionList()

_outWriter.WriteLine(ColorStyle.SectionHeader, "Installed Extensions");

foreach (var ep in _extensionService?.ExtensionPoints ?? new IExtensionPoint[0])
foreach (var ep in _extensionService.ExtensionPoints)
{
_outWriter.WriteLabelLine(" Extension Point: ", ep.Path);
foreach (var node in ep.Extensions)
Expand Down
28 changes: 17 additions & 11 deletions src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace NUnit.Engine.Services
{
public class ExtensionManager : IExtensionManager
public class ExtensionManager
{
static readonly Version CURRENT_ENGINE_VERSION = Assembly.GetExecutingAssembly().GetName().Version;

Expand All @@ -29,8 +29,12 @@ public class ExtensionManager : IExtensionManager
private readonly List<ExtensionNode> _extensions = new List<ExtensionNode>();
private readonly List<ExtensionAssembly> _assemblies = new List<ExtensionAssembly>();

// List of all extensionDirectories specified on command-line or in environment
private readonly List<string> _extensionDirectories = new List<string>();

// Dictionary containing all directory paths already examined
private readonly Dictionary<string, object> _directoriesExamined = new Dictionary<string, object>();

public ExtensionManager()
: this(new FileSystem())
{
Expand Down Expand Up @@ -66,7 +70,7 @@ public virtual void FindExtensionPoints(params Assembly[] targetAssemblies)
{
foreach (var assembly in targetAssemblies)
{
log.Info("Scanning {0} assembly for extension points", assembly.GetName().Name);
log.Info("FindExtensionPoints scanning {0} assembly", assembly.GetName().Name);

foreach (ExtensionPointAttribute attr in assembly.GetCustomAttributes(typeof(ExtensionPointAttribute), false))
{
Expand All @@ -86,7 +90,7 @@ public virtual void FindExtensionPoints(params Assembly[] targetAssemblies)
_extensionPoints.Add(ep);
_pathIndex.Add(ep.Path, ep);

log.Info(" Found Path={0}, Type={1}", ep.Path, ep.TypeName);
log.Info(" Found ExtensionPoint: Path={0}, Type={1}", ep.Path, ep.TypeName);
}

foreach (Type type in assembly.GetExportedTypes())
Expand All @@ -111,7 +115,7 @@ public virtual void FindExtensionPoints(params Assembly[] targetAssemblies)
_extensionPoints.Add(ep);
_pathIndex.Add(path, ep);

log.Info(" Found Path={0}, Type={1}", ep.Path, ep.TypeName);
log.Info(" Found ExtensionPoint: Path={0}, Type={1}", ep.Path, ep.TypeName);
}
}
}
Expand All @@ -125,6 +129,8 @@ public void FindExtensions(string startDir)
{
_extensionDirectories.Add(startDir);

log.Info($"FindExtensions examining extension directory {startDir}");

// Create the list of possible extension assemblies,
// eliminating duplicates, start in the provided directory.
FindExtensionAssemblies(_fileSystem.GetDirectory(startDir));
Expand All @@ -138,6 +144,8 @@ public void FindExtensions(string startDir)
/// <inheritdoc/>
public void FindStandardExtensions(Assembly hostAssembly)
{
log.Info($"FindStandardExtensions called for host {hostAssembly.FullName}");

bool isChocolateyPackage = System.IO.File.Exists(Path.Combine(hostAssembly.Location, "VERIFICATION.txt"));
string[] extensionPatterns = isChocolateyPackage
? new[] { "nunit-extension-*/**/tools/", "nunit-extension-*/**/tools/*/" }
Expand Down Expand Up @@ -273,7 +281,7 @@ private void FindExtensionAssemblies(Assembly hostAssembly, string[] patterns)
IDirectory startDir = _fileSystem.GetDirectory(AssemblyHelper.GetDirectoryName(hostAssembly));

while (startDir != null)
{
{
foreach (var pattern in patterns)
foreach (var dir in _directoryFinder.GetDirectories(startDir, pattern))
ProcessDirectory(dir, true);
Expand All @@ -290,7 +298,7 @@ private void FindExtensionAssemblies(Assembly hostAssembly, string[] patterns)
private void ProcessDirectory(IDirectory startDir, bool fromWildCard)
{
var directoryName = startDir.FullName;
if (WasVisited(startDir.FullName, fromWildCard))
if (WasVisited(directoryName, fromWildCard))
{
log.Warning($"Skipping directory '{directoryName}' because it was already visited.");
}
Expand Down Expand Up @@ -371,7 +379,7 @@ private void ProcessAddinsFile(IFile addinsFile, bool fromWildCard)

private void ProcessCandidateAssembly(string filePath, bool fromWildCard)
{
if (WasVisited(filePath, fromWildCard))
if (WasVisited(filePath, fromWildCard))
return;

MarkAsVisited(filePath, fromWildCard);
Expand Down Expand Up @@ -410,16 +418,14 @@ private void ProcessCandidateAssembly(string filePath, bool fromWildCard)
}
}

private readonly Dictionary<string, object> _visited = new Dictionary<string, object>();

private bool WasVisited(string filePath, bool fromWildcard)
{
return _visited.ContainsKey($"path={ filePath }_visited={fromWildcard}");
return _directoriesExamined.ContainsKey($"path={filePath}_visited={fromWildcard}");
}

private void MarkAsVisited(string filePath, bool fromWildcard)
{
_visited.Add($"path={ filePath }_visited={fromWildcard}", null);
_directoriesExamined.Add($"path={filePath}_visited={fromWildcard}", null);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace NUnit.Engine.Services
/// </summary>
public class ExtensionService : Service, IExtensionService
{
private readonly IExtensionManager _extensionManager;
private readonly ExtensionManager _extensionManager;

public ExtensionService()
{
Expand Down
Loading

0 comments on commit 2550a11

Please sign in to comment.