Merge pull request #257 from nunit/issue-252

Improve error catching around OutputSpecifications
nunit · Jul 14, 2017 · 403740b · 403740b
2 parents 008e02f + d2695ae
commit 403740b
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 14 deletions.
diff --git a/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs b/src/NUnitConsole/nunit3-console.tests/CommandLineTests.cs
@@ -475,15 +475,15 @@ 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]);
 
             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
@@ -33,7 +33,7 @@ public void SpecMayNotBeNull()
         {
             Assert.That(
                 () => new OutputSpecification(null),
-                Throws.TypeOf<NullReferenceException>());
+                Throws.TypeOf<ArgumentNullException>());
         }
 
 

diff --git 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<OutputSpecification> 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
@@ -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
@@ -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();
+        }
     }
 }