Test: improve tests by checking error messages (#549)

Resolve #544.
dotnet · Jan 14, 2023 · 7b32935 · 7b32935
1 parent 97df966
commit 7b32935
Show file tree

Hide file tree

Showing 15 changed files with 109 additions and 35 deletions.
diff --git a/test/Sign.Cli.Test/Options/BaseDirectoryOptionTests.cs b/test/Sign.Cli.Test/Options/BaseDirectoryOptionTests.cs
@@ -3,13 +3,14 @@
 // See the LICENSE.txt file in the project root for more information.
 
 using System.CommandLine.Parsing;
+using System.Globalization;
 
 namespace Sign.Cli.Test
 {
     public class BaseDirectoryOptionTests : DirectoryInfoOptionTests
     {
         public BaseDirectoryOptionTests()
-            : base(new CodeCommand().BaseDirectoryOption, "-b", "--base-directory", isRequired: false)
+            : base(new CodeCommand().BaseDirectoryOption, "-b", "--base-directory")
         {
         }
 
@@ -27,7 +28,9 @@ public void Option_WhenOptionIsMissing_HasDefaultValue()
         [InlineData(@".\directory")]
         public void Option_WhenValueIsNotRooted_HasError(string relativePath)
         {
-            VerifyHasError($"{LongOption} {relativePath}");
+            VerifyHasErrors(
+                $"{LongOption} {relativePath}",
+                string.Format(CultureInfo.CurrentCulture, Resources.InvalidBaseDirectoryValue, "--base-directory"));
         }
 
         [Fact]

diff --git a/test/Sign.Cli.Test/Options/DescriptionOptionTests.cs b/test/Sign.Cli.Test/Options/DescriptionOptionTests.cs
@@ -9,7 +9,7 @@ public class DescriptionOptionTests : OptionTests<string>
         private const string ExpectedValue = "peach";
 
         public DescriptionOptionTests()
-            : base(new CodeCommand().DescriptionOption, "-d", "--description", ExpectedValue, isRequired: true)
+            : base(new CodeCommand().DescriptionOption, "-d", "--description", ExpectedValue)
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/DescriptionUrlOptionTests.cs b/test/Sign.Cli.Test/Options/DescriptionUrlOptionTests.cs
@@ -7,7 +7,7 @@ namespace Sign.Cli.Test
     public class DescriptionUrlOptionTests : UriOptionTests
     {
         public DescriptionUrlOptionTests()
-            : base(new CodeCommand().DescriptionUrlOption, "-u", "--description-url", isRequired: true)
+            : base(new CodeCommand().DescriptionUrlOption, "-u", "--description-url")
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/DirectoryInfoOptionTests.cs b/test/Sign.Cli.Test/Options/DirectoryInfoOptionTests.cs
@@ -10,8 +10,8 @@ public abstract class DirectoryInfoOptionTests : OptionTests<DirectoryInfo>
     {
         private static readonly DirectoryInfo ExpectedValue = new(Path.GetTempPath());
 
-        protected DirectoryInfoOptionTests(Option<DirectoryInfo> option, string shortOption, string longOption, bool isRequired)
-            : base(option, shortOption, longOption, ExpectedValue, isRequired)
+        protected DirectoryInfoOptionTests(Option<DirectoryInfo> option, string shortOption, string longOption)
+            : base(option, shortOption, longOption, ExpectedValue)
         {
         }
 

diff --git a/test/Sign.Cli.Test/Options/FileDigestOptionTests.cs b/test/Sign.Cli.Test/Options/FileDigestOptionTests.cs
@@ -7,7 +7,7 @@ namespace Sign.Cli.Test
     public class FileDigestOptionTests : HashAlgorithmNameOptionTests
     {
         public FileDigestOptionTests()
-            : base(new CodeCommand().FileDigestOption, "-fd", "--file-digest", isRequired: false)
+            : base(new CodeCommand().FileDigestOption, "-fd", "--file-digest")
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/HashAlgorithmNameOptionTests.cs b/test/Sign.Cli.Test/Options/HashAlgorithmNameOptionTests.cs
@@ -12,8 +12,8 @@ public abstract class HashAlgorithmNameOptionTests : OptionTests<HashAlgorithmNa
     {
         private static readonly HashAlgorithmName ExpectedValue = HashAlgorithmName.SHA256;
 
-        protected HashAlgorithmNameOptionTests(Option<HashAlgorithmName> option, string shortOption, string longOption, bool isRequired)
-            : base(option, shortOption, longOption, ExpectedValue, isRequired)
+        protected HashAlgorithmNameOptionTests(Option<HashAlgorithmName> option, string shortOption, string longOption)
+            : base(option, shortOption, longOption, ExpectedValue)
         {
         }
 
@@ -47,7 +47,9 @@ public void Option_WhenValueIsSha512_ParsesValue(string value)
         [InlineData("sha-256")]
         public void Verbosity_WhenValueIsInvalid_HasError(string value)
         {
-            VerifyHasError($"{LongOption} {value}");
+            VerifyHasErrors(
+                $"{LongOption} {value}",
+                GetFormattedResourceString(Resources.InvalidDigestValue, LongOption));
         }
 
         [Fact]

diff --git a/test/Sign.Cli.Test/Options/Int32OptionTests.cs b/test/Sign.Cli.Test/Options/Int32OptionTests.cs
@@ -10,8 +10,8 @@ public abstract class Int32OptionTests : OptionTests<int>
     {
         private const int ExpectedValue = 3;
 
-        protected Int32OptionTests(Option<int> option, string shortOption, string longOption, bool isRequired)
-            : base(option, shortOption, longOption, ExpectedValue, isRequired)
+        protected Int32OptionTests(Option<int> option, string shortOption, string longOption)
+            : base(option, shortOption, longOption, ExpectedValue)
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/MaxConcurrencyOptionTests.cs b/test/Sign.Cli.Test/Options/MaxConcurrencyOptionTests.cs
@@ -9,14 +9,16 @@ namespace Sign.Cli.Test
     public class MaxConcurrencyOptionTests : Int32OptionTests
     {
         public MaxConcurrencyOptionTests()
-            : base(new CodeCommand().MaxConcurrencyOption, "-m", "--max-concurrency", isRequired: false)
+            : base(new CodeCommand().MaxConcurrencyOption, "-m", "--max-concurrency")
         {
         }
 
         [Fact]
         public void Option_WhenValueFailsToParse_HasError()
         {
-            VerifyHasError("x");
+            const string value = "x";
+
+            VerifyHasErrors(value, GetUnrecognizedCommandOrArgumentMessage(value));
         }
 
         [Fact]
@@ -33,7 +35,9 @@ public void Option_WhenOptionIsMissing_HasDefaultValue()
         [InlineData(-1)]
         public void Option_WhenValueIsLessThanOne_HasError(int value)
         {
-            VerifyHasError($"{LongOption} {value}");
+            VerifyHasErrors(
+                $"{LongOption} {value}",
+                GetFormattedResourceString(Resources.InvalidMaxConcurrencyValue, LongOption));
         }
     }
 }
diff --git a/test/Sign.Cli.Test/Options/OptionTests.cs b/test/Sign.Cli.Test/Options/OptionTests.cs
@@ -4,25 +4,24 @@
 
 using System.CommandLine;
 using System.CommandLine.Parsing;
+using System.Globalization;
 
 namespace Sign.Cli.Test
 {
     public abstract class OptionTests<T>
     {
         private readonly T? _expectedValue;
-        private readonly bool _isRequired;
 
         protected Option<T> Option { get; }
         protected string LongOption { get; }
         protected string ShortOption { get; }
 
-        protected OptionTests(Option<T> option, string shortOption, string longOption, T? expectedValue, bool isRequired)
+        protected OptionTests(Option<T> option, string shortOption, string longOption, T? expectedValue)
         {
             Option = option;
             ShortOption = shortOption;
             LongOption = longOption;
             _expectedValue = expectedValue;
-            _isRequired = isRequired;
         }
 
         [Fact]
@@ -34,19 +33,31 @@ public void Option_WhenOptionIsMissing_HasParseErrorsOnlyIfRequired()
         [Fact]
         public void Option_WithOnlyValue_HasParseErrors()
         {
-            VerifyHasError("x");
+            const string value = "x";
+
+            if (Option.IsRequired)
+            {
+                VerifyHasErrors(
+                    value,
+                    GetOptionIsRequiredMessage(ShortOption),
+                    GetUnrecognizedCommandOrArgumentMessage(value));
+            }
+            else
+            {
+                VerifyHasErrors(value, GetUnrecognizedCommandOrArgumentMessage(value));
+            }
         }
 
         [Fact]
         public void Option_WithShortOptionAndMissingValue_HasParseErrors()
         {
-            VerifyHasError(ShortOption);
+            VerifyHasErrors(ShortOption, GetRequiredArgumentMissingMessage(ShortOption));
         }
 
         [Fact]
         public void Option_WithLongOptionAndMissingValue_HasParseErrors()
         {
-            VerifyHasError(LongOption);
+            VerifyHasErrors(LongOption, GetRequiredArgumentMissingMessage(LongOption));
         }
 
         [Fact]
@@ -63,7 +74,7 @@ public void Option_WithShortOptionThenValue_ParsesValueOnlyIfShortOptionHasSingl
             }
             else
             {
-                VerifyHasError(commandLine);
+                VerifyHasErrors(commandLine, GetUnrecognizedCommandOrArgumentMessage(commandLine));
             }
         }
 
@@ -100,20 +111,29 @@ protected virtual void VerifyEqual(T? expectedValue, T? actualValue)
             Assert.Equal(expectedValue, actualValue);
         }
 
-        protected void VerifyHasError(string commandLine)
+        protected void VerifyHasErrors(string commandLine, params string[] expectedErrorMessages)
         {
             ParseResult result = Parse(commandLine);
+            HashSet<string> expectedMessages = new(expectedErrorMessages, StringComparer.Ordinal);
+            HashSet<string> actualMessages = result.Errors
+                .Select(error => error.Message)
+                .ToHashSet(StringComparer.Ordinal);
 
-            Assert.NotEmpty(result.Errors);
+            Assert.NotEmpty(actualMessages);
+            Assert.Equal(expectedMessages, actualMessages);
         }
 
         private void VerifyIsRequired()
         {
             ParseResult result = Parse();
 
-            if (_isRequired)
+            if (Option.IsRequired)
             {
-                Assert.NotEmpty(result.Errors);
+                ParseError parseError = Assert.Single(result.Errors);
+                string actualMessage = parseError.Message;
+                string expectedMessage = GetOptionIsRequiredMessage(ShortOption);
+
+                Assert.Equal(expectedMessage, actualMessage);
             }
             else
             {
@@ -127,5 +147,34 @@ protected ParseResult Parse(string commandLine = "")
 
             return rootCommand.Parse(commandLine);
         }
+
+        protected static string GetFormattedResourceString(string resourceString, params string[] arguments)
+        {
+            return string.Format(CultureInfo.CurrentCulture, resourceString, arguments);
+        }
+
+        private static string GetRequiredArgumentMissingMessage(string argumentName)
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                "Required argument missing for option: '{0}'.",
+                argumentName);
+        }
+
+        protected static string GetOptionIsRequiredMessage(string optionName)
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                "Option '{0}' is required.",
+                optionName);
+        }
+
+        protected static string GetUnrecognizedCommandOrArgumentMessage(string name)
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                "Unrecognized command or argument '{0}'.",
+                name);
+        }
     }
 }
diff --git a/test/Sign.Cli.Test/Options/OutputOptionTests.cs b/test/Sign.Cli.Test/Options/OutputOptionTests.cs
@@ -9,7 +9,7 @@ public class OutputOptionTests : OptionTests<string?>
         private const string ExpectedValue = "peach";
 
         public OutputOptionTests()
-            : base(new CodeCommand().OutputOption, "-o", "--output", ExpectedValue, isRequired: false)
+            : base(new CodeCommand().OutputOption, "-o", "--output", ExpectedValue)
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/PublisherNameOptionTests.cs b/test/Sign.Cli.Test/Options/PublisherNameOptionTests.cs
@@ -9,7 +9,7 @@ public class PublisherNameOptionTests : OptionTests<string?>
         private const string? ExpectedValue = "peach";
 
         public PublisherNameOptionTests()
-            : base(new CodeCommand().PublisherNameOption, "-pn", "--publisher-name", ExpectedValue, isRequired: false)
+            : base(new CodeCommand().PublisherNameOption, "-pn", "--publisher-name", ExpectedValue)
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/TimestampDigestOptionTests.cs b/test/Sign.Cli.Test/Options/TimestampDigestOptionTests.cs
@@ -7,7 +7,7 @@ namespace Sign.Cli.Test
     public class TimestampDigestOptionTests : HashAlgorithmNameOptionTests
     {
         public TimestampDigestOptionTests()
-            : base(new CodeCommand().TimestampDigestOption, "-td", "--timestamp-digest", isRequired: false)
+            : base(new CodeCommand().TimestampDigestOption, "-td", "--timestamp-digest")
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/TimestampUrlOptionTests.cs b/test/Sign.Cli.Test/Options/TimestampUrlOptionTests.cs
@@ -7,7 +7,7 @@ namespace Sign.Cli.Test
     public class TimestampUrlOptionTests : UriOptionTests
     {
         public TimestampUrlOptionTests()
-            : base(new CodeCommand().TimestampUrlOption, "-t", "--timestamp-url", isRequired: true)
+            : base(new CodeCommand().TimestampUrlOption, "-t", "--timestamp-url")
         {
         }
     }

diff --git a/test/Sign.Cli.Test/Options/UriOptionTests.cs b/test/Sign.Cli.Test/Options/UriOptionTests.cs
@@ -10,15 +10,29 @@ public abstract class UriOptionTests : OptionTests<Uri?>
     {
         private static readonly Uri ExpectedValue = new("https://domain.test");
 
-        protected UriOptionTests(Option<Uri?> option, string shortOption, string longOption, bool isRequired)
-            : base(option, shortOption, longOption, ExpectedValue, isRequired)
+        protected UriOptionTests(Option<Uri?> option, string shortOption, string longOption)
+            : base(option, shortOption, longOption, ExpectedValue)
         {
         }
 
         [Fact]
         public void Option_WhenValueFailsToParse_HasError()
         {
-            VerifyHasError("3");
+            const string value = "3";
+
+            if (Option.IsRequired)
+            {
+                VerifyHasErrors(
+                    value,
+                    GetOptionIsRequiredMessage(ShortOption),
+                    GetUnrecognizedCommandOrArgumentMessage(value));
+            }
+            else
+            {
+                VerifyHasErrors(
+                    value,
+                    GetUnrecognizedCommandOrArgumentMessage(value));
+            }
         }
 
         [Theory]
@@ -37,7 +51,9 @@ public void Option_WithShortOptionAndValidUrl_ParsesValue(string url)
         [InlineData("file:///file.bin")]
         public void Option_WithShortOptionAndInvalidUrl_HasErrors(string invalidUrl)
         {
-            VerifyHasError($"{ShortOption} {invalidUrl}");
+            VerifyHasErrors(
+                $"{ShortOption} {invalidUrl}",
+                GetFormattedResourceString(Resources.InvalidUrlValue, LongOption));
         }
 
         protected override void VerifyEqual(Uri? expectedValue, Uri? actualValue)

diff --git a/test/Sign.Cli.Test/Options/VerbosityOptionTests.cs b/test/Sign.Cli.Test/Options/VerbosityOptionTests.cs
@@ -11,7 +11,7 @@ public class VerbosityOptionTests : OptionTests<LogLevel>
         private const LogLevel ExpectedValue = LogLevel.Debug;
 
         public VerbosityOptionTests()
-            : base(new CodeCommand().VerbosityOption, "-v", "--verbosity", ExpectedValue, isRequired: false)
+            : base(new CodeCommand().VerbosityOption, "-v", "--verbosity", ExpectedValue)
         {
         }