From d2695ae1215dd69bf694827d9ac86dee533461d6 Mon Sep 17 00:00:00 2001 From: Chris Maddock Date: Sun, 9 Jul 2017 15:40:10 +0100 Subject: [PATCH] Improve error catching around OutputSpecifications --- .../nunit3-console.tests/CommandLineTests.cs | 33 +++++++++++++--- .../OutputSpecificationTests.cs | 2 +- .../nunit3-console/CommandLineOptions.cs | 38 +++++++++++++++++-- .../nunit3-console/ConsoleRunner.cs | 14 ++++++- .../nunit3-console/OutputSpecification.cs | 13 ++++++- 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs b/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs index 56babfdcd..b7069687b 100644 --- a/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs +++ b/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs @@ -475,7 +475,7 @@ public void ResultOptionWithFilePathAndFormat() [Test] public void ResultOptionWithFilePathAndTransform() { - ConsoleOptions options = new ConsoleOptions("tests.dll", "-result:results.xml;transform=transform.xslt"); + ConsoleOptions options = new ConsoleOptions("tests.dll", "-result:results.xml;transform=TextSummary.xslt"); Assert.True(options.Validate()); Assert.AreEqual(1, options.InputFiles.Count, "assembly should be set"); Assert.AreEqual("tests.dll", options.InputFiles[0]); @@ -483,7 +483,7 @@ public void ResultOptionWithFilePathAndTransform() OutputSpecification spec = options.ResultOutputSpecifications[0]; Assert.AreEqual("results.xml", spec.OutputPath); Assert.AreEqual("user", spec.Format); - Assert.AreEqual("transform.xslt", spec.Transform); + Assert.AreEqual("TextSummary.xslt", spec.Transform); } [Test] @@ -506,7 +506,7 @@ public void ResultOptionWithoutFileNameIsInvalid() [Test] public void ResultOptionMayBeRepeated() { - ConsoleOptions options = new ConsoleOptions("tests.dll", "-result:results.xml", "-result:nunit2results.xml;format=nunit2", "-result:myresult.xml;transform=mytransform.xslt"); + ConsoleOptions options = new ConsoleOptions("tests.dll", "-result:results.xml", "-result:nunit2results.xml;format=nunit2", "-result:myresult.xml;transform=TextSummary.xslt"); Assert.True(options.Validate(), "Should be valid"); var specs = options.ResultOutputSpecifications; @@ -525,7 +525,7 @@ public void ResultOptionMayBeRepeated() var spec3 = specs[2]; Assert.AreEqual("myresult.xml", spec3.OutputPath); Assert.AreEqual("user", spec3.Format); - Assert.AreEqual("mytransform.xslt", spec3.Transform); + Assert.AreEqual("TextSummary.xslt", spec3.Transform); } [Test] @@ -554,6 +554,27 @@ public void NoResultSuppressesAllResultSpecifications() Assert.AreEqual(0, options.ResultOutputSpecifications.Count); } + [Test] + public void InvalidResultSpecRecordsError() + { + var options = new ConsoleOptions("test.dll", "-result:userspecifed.xml;format=nunit2;format=nunit3"); + Assert.That(options.ResultOutputSpecifications, Has.Exactly(1).Items + .And.Exactly(1).Property(nameof(OutputSpecification.OutputPath)).EqualTo("TestResult.xml")); + Assert.That(options.ErrorMessages, Has.Exactly(1).Contains("conflicting format options").IgnoreCase); + } + + [Test] + public void MissingXsltFileRecordsError() + { + const string missingXslt = "missing.xslt"; + Assert.That(missingXslt, Does.Not.Exist); + + var options = new ConsoleOptions("test.dll", $"-result:userspecifed.xml;transform={missingXslt}"); + Assert.That(options.ResultOutputSpecifications, Has.Exactly(1).Items + .And.Exactly(1).Property(nameof(OutputSpecification.Transform)).Null); + Assert.That(options.ErrorMessages, Has.Exactly(1).Contains($"{missingXslt} could not be found").IgnoreCase); + } + #endregion #region Explore Option @@ -599,7 +620,7 @@ public void ExploreOptionWithFilePathAndFormat() [Test] public void ExploreOptionWithFilePathAndTransform() { - ConsoleOptions options = new ConsoleOptions("tests.dll", "-explore:results.xml;transform=myreport.xslt"); + ConsoleOptions options = new ConsoleOptions("tests.dll", "-explore:results.xml;transform=TextSummary.xslt"); Assert.True(options.Validate()); Assert.AreEqual(1, options.InputFiles.Count, "assembly should be set"); Assert.AreEqual("tests.dll", options.InputFiles[0]); @@ -608,7 +629,7 @@ public void ExploreOptionWithFilePathAndTransform() OutputSpecification spec = options.ExploreOutputSpecifications[0]; Assert.AreEqual("results.xml", spec.OutputPath); Assert.AreEqual("user", spec.Format); - Assert.AreEqual("myreport.xslt", spec.Transform); + Assert.AreEqual("TextSummary.xslt", spec.Transform); } [Test] diff --git a/src/NUnitConsole/nunit3-console.tests/OutputSpecificationTests.cs b/src/NUnitConsole/nunit3-console.tests/OutputSpecificationTests.cs index c29aa28a1..e06e46634 100644 --- a/src/NUnitConsole/nunit3-console.tests/OutputSpecificationTests.cs +++ b/src/NUnitConsole/nunit3-console.tests/OutputSpecificationTests.cs @@ -33,7 +33,7 @@ public void SpecMayNotBeNull() { Assert.That( () => new OutputSpecification(null), - Throws.TypeOf()); + Throws.TypeOf()); } diff --git a/src/NUnitConsole/nunit3-console/CommandLineOptions.cs b/src/NUnitConsole/nunit3-console/CommandLineOptions.cs index 08cc84267..097baa624 100644 --- a/src/NUnitConsole/nunit3-console/CommandLineOptions.cs +++ b/src/NUnitConsole/nunit3-console/CommandLineOptions.cs @@ -335,13 +335,12 @@ protected virtual void ConfigureOptions() v => ErrFile = RequiredValue(v, "--err")); this.Add("result=", "An output {SPEC} for saving the test results.\nThis option may be repeated.", - v => resultOutputSpecifications.Add(new OutputSpecification(RequiredValue(v, "--resultxml")))); + v => ResolveOutputSpecification(RequiredValue(v, "--resultxml"), resultOutputSpecifications)); this.Add("explore:", "Display or save test info rather than running tests. Optionally provide an output {SPEC} for saving the test info. This option may be repeated.", v => { Explore = true; - if (v != null) - ExploreOutputSpecifications.Add(new OutputSpecification(v)); + ResolveOutputSpecification(v, ExploreOutputSpecifications); }); this.Add("noresult", "Don't save any test results.", @@ -384,6 +383,37 @@ protected virtual void ConfigureOptions() }); } -#endregion + private void ResolveOutputSpecification(string value, IList outputSpecifications) + { + if (value == null) + return; + + OutputSpecification spec; + + try + { + spec = new OutputSpecification(value); + } + catch (ArgumentException e) + { + ErrorMessages.Add(e.Message); + return; + } + + if (spec.Transform != null) + { + var transformPath = Path.Combine(WorkDirectory, spec.Transform); + + if (!File.Exists(transformPath)) + { + ErrorMessages.Add($"Transform {spec.Transform} could not be found. (Path searched: {transformPath})"); + return; + } + } + + outputSpecifications.Add(spec); + } + + #endregion } } diff --git a/src/NUnitConsole/nunit3-console/ConsoleRunner.cs b/src/NUnitConsole/nunit3-console/ConsoleRunner.cs index 06c8f8806..f6beb76ae 100644 --- a/src/NUnitConsole/nunit3-console/ConsoleRunner.cs +++ b/src/NUnitConsole/nunit3-console/ConsoleRunner.cs @@ -171,9 +171,21 @@ private int RunTests(TestPackage package, TestFilter filter) foreach (var spec in _options.ResultOutputSpecifications) { var outputPath = Path.Combine(_workDirectory, spec.OutputPath); + + IResultWriter resultWriter; + + try + { + resultWriter = GetResultWriter(spec); + } + catch (Exception ex) + { + throw new NUnitEngineException($"Error encountered in resolving output specification: {spec}", ex); + } + try { - GetResultWriter(spec).CheckWritability(outputPath); + resultWriter.CheckWritability(outputPath); } catch (SystemException ex) { diff --git a/src/NUnitConsole/nunit3-console/OutputSpecification.cs b/src/NUnitConsole/nunit3-console/OutputSpecification.cs index 707b95afb..c173c470a 100644 --- a/src/NUnitConsole/nunit3-console/OutputSpecification.cs +++ b/src/NUnitConsole/nunit3-console/OutputSpecification.cs @@ -22,6 +22,7 @@ // *********************************************************************** using System; +using System.Text; namespace NUnit.Common { @@ -40,7 +41,7 @@ public class OutputSpecification public OutputSpecification(string spec) { if (spec == null) - throw new NullReferenceException("Output spec may not be null"); + throw new ArgumentNullException(nameof(spec), "Output spec may not be null"); string[] parts = spec.Split(';'); this.OutputPath = parts[0]; @@ -50,7 +51,7 @@ public OutputSpecification(string spec) string[] opt = parts[i].Split('='); if (opt.Length != 2) - throw new ArgumentException(); + throw new ArgumentException($"Invalid output specification: {spec}"); switch (opt[0].Trim()) { @@ -105,5 +106,13 @@ public OutputSpecification(string spec) public string Transform { get; private set; } #endregion + + public override string ToString() + { + var sb = new StringBuilder($"OutputPath: {OutputPath}"); + if (Format != null) sb.Append($", Format: {Format}"); + if (Transform != null) sb.Append($", Transform: {Transform}"); + return sb.ToString(); + } } }