From a011c28d4000f469327ac85f79b4801a432a498b Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 19 Jun 2017 01:05:43 -0400 Subject: make version parsing stricter, add unit tests for parsing (#309) --- .../StardewModdingAPI.Tests.csproj | 1 + .../Utilities/SemanticVersionTests.cs | 136 +++++++++++++++++++++ src/StardewModdingAPI/SemanticVersion.cs | 32 +++-- 3 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 src/StardewModdingAPI.Tests/Utilities/SemanticVersionTests.cs (limited to 'src') diff --git a/src/StardewModdingAPI.Tests/StardewModdingAPI.Tests.csproj b/src/StardewModdingAPI.Tests/StardewModdingAPI.Tests.csproj index f626eb08..7129cfb7 100644 --- a/src/StardewModdingAPI.Tests/StardewModdingAPI.Tests.csproj +++ b/src/StardewModdingAPI.Tests/StardewModdingAPI.Tests.csproj @@ -51,6 +51,7 @@ Properties\GlobalAssemblyInfo.cs + diff --git a/src/StardewModdingAPI.Tests/Utilities/SemanticVersionTests.cs b/src/StardewModdingAPI.Tests/Utilities/SemanticVersionTests.cs new file mode 100644 index 00000000..355e3a84 --- /dev/null +++ b/src/StardewModdingAPI.Tests/Utilities/SemanticVersionTests.cs @@ -0,0 +1,136 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using NUnit.Framework; + +namespace StardewModdingAPI.Tests.Utilities +{ + /// Unit tests for . + [TestFixture] + internal class SemanticVersionTests + { + /********* + ** Unit tests + *********/ + /**** + ** Constructor + ****/ + [Test(Description = "Assert that the constructor sets the expected values for all valid versions.")] + [TestCase("1.0", ExpectedResult = "1.0")] + [TestCase("1.0.0", ExpectedResult = "1.0")] + [TestCase("3000.4000.5000", ExpectedResult = "3000.4000.5000")] + [TestCase("1.2-some-tag.4", ExpectedResult = "1.2-some-tag.4")] + [TestCase("1.2.3-some-tag.4", ExpectedResult = "1.2.3-some-tag.4")] + [TestCase("1.2.3-some-tag.4 ", ExpectedResult = "1.2.3-some-tag.4")] + public string Constructor_FromString(string input) + { + return new SemanticVersion(input).ToString(); + } + + [Test(Description = "Assert that the constructor sets the expected values for all valid versions.")] + [TestCase(1, 0, 0, null, ExpectedResult = "1.0")] + [TestCase(3000, 4000, 5000, null, ExpectedResult = "3000.4000.5000")] + [TestCase(1, 2, 3, "", ExpectedResult = "1.2.3")] + [TestCase(1, 2, 3, " ", ExpectedResult = "1.2.3")] + [TestCase(1, 2, 3, "some-tag.4", ExpectedResult = "1.2.3-some-tag.4")] + [TestCase(1, 2, 3, "some-tag.4 ", ExpectedResult = "1.2.3-some-tag.4")] + public string Constructor_FromParts(int major, int minor, int patch, string tag) + { + // act + ISemanticVersion version = new SemanticVersion(major, minor, patch, tag); + + // assert + Assert.AreEqual(major, version.MajorVersion, "The major version doesn't match the given value."); + Assert.AreEqual(minor, version.MinorVersion, "The minor version doesn't match the given value."); + Assert.AreEqual(patch, version.PatchVersion, "The patch version doesn't match the given value."); + Assert.AreEqual(string.IsNullOrWhiteSpace(tag) ? null : tag.Trim(), version.Build, "The tag doesn't match the given value."); + return version.ToString(); + } + + [Test(Description = "Assert that the constructor throws the expected exception for invalid versions.")] + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + [TestCase("1")] + [TestCase("01.0")] + [TestCase("1.05")] + [TestCase("1.5.06")] // leading zeros specifically prohibited by spec + [TestCase("1.2.3.4")] + [TestCase("1.apple")] + [TestCase("1.2.apple")] + [TestCase("1.2.3.apple")] + [TestCase("1..2..3")] + [TestCase("1.2.3-")] + [TestCase("1.2.3-some-tag...")] + [TestCase("1.2.3-some-tag...4")] + [TestCase("apple")] + [TestCase("-apple")] + [TestCase("-5")] + public void Constructor_FromString_WithInvalidValues(string input) + { + if (input == null) + this.AssertAndLogException(() => new SemanticVersion(input)); + else + this.AssertAndLogException(() => new SemanticVersion(input)); + } + + //[Test(Description = "Assert that the constructor throws an exception if the values are invalid.")] + //[TestCase(01, "Spring", 1)] // seasons are case-sensitive + //[TestCase(01, "springs", 1)] // invalid season name + //[TestCase(-1, "spring", 1)] // day < 0 + //[TestCase(29, "spring", 1)] // day > 28 + //[TestCase(01, "spring", -1)] // year < 1 + //[TestCase(01, "spring", 0)] // year < 1 + //[SuppressMessage("ReSharper", "AssignmentIsFullyDiscarded", Justification = "Deliberate for unit test.")] + //public void Constructor_RejectsInvalidValues(int day, string season, int year) + //{ + // // act & assert + // Assert.Throws(() => _ = new SDate(day, season, year), "Constructing the invalid date didn't throw the expected exception."); + //} + + + /********* + ** Private methods + *********/ + /// Assert that the expected exception type is thrown, and log the action output and thrown exception. + /// The expected exception type. + /// The action which may throw the exception. + /// The message to log if the expected exception isn't thrown. + [SuppressMessage("ReSharper", "UnusedParameter.Local", Justification = "The message argument is deliberately only used in precondition checks since this is an assertion method.")] + private void AssertAndLogException(Func action, string message = null) + where T : Exception + { + this.AssertAndLogException(() => + { + object result = action(); + TestContext.WriteLine($"Func result: {result}"); + }); + } + + /// Assert that the expected exception type is thrown, and log the thrown exception. + /// The expected exception type. + /// The action which may throw the exception. + /// The message to log if the expected exception isn't thrown. + [SuppressMessage("ReSharper", "UnusedParameter.Local", Justification = "The message argument is deliberately only used in precondition checks since this is an assertion method.")] + private void AssertAndLogException(Action action, string message = null) + where T : Exception + { + try + { + action(); + } + catch (T ex) + { + TestContext.WriteLine($"Exception thrown:\n{ex}"); + return; + } + catch (Exception ex) when (!(ex is AssertionException)) + { + TestContext.WriteLine($"Exception thrown:\n{ex}"); + Assert.Fail(message ?? $"Didn't throw the expected exception; expected {typeof(T).FullName}, got {ex.GetType().FullName}."); + } + + // no exception thrown + Assert.Fail(message ?? "Didn't throw an exception."); + } + } +} diff --git a/src/StardewModdingAPI/SemanticVersion.cs b/src/StardewModdingAPI/SemanticVersion.cs index a2adb657..4b27c819 100644 --- a/src/StardewModdingAPI/SemanticVersion.cs +++ b/src/StardewModdingAPI/SemanticVersion.cs @@ -10,8 +10,14 @@ namespace StardewModdingAPI ** Properties *********/ /// A regular expression matching a semantic version string. - /// Derived from https://github.com/maxhauser/semver. - private static readonly Regex Regex = new Regex(@"^(?\d+)(\.(?\d+))?(\.(?\d+))?(?.*)$", RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture); + /// + /// This pattern is derived from the BNF documentation in the semver repo, + /// with three important deviations intended to support Stardew Valley mod conventions: + /// - allows short-form "x.y" versions; + /// - allows hyphens in prerelease tags as synonyms for dots (like "-unofficial-update.3"); + /// - doesn't allow '+build' suffixes. + /// + private static readonly Regex Regex = new Regex(@"^(?0|[1-9]\d*)\.(?0|[1-9]\d*)(\.(?0|[1-9]\d*))?(?:-(?([a-z0-9]+[\-\.]?)+))?$", RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.ExplicitCapture); /********* @@ -48,17 +54,22 @@ namespace StardewModdingAPI /// Construct an instance. /// The semantic version string. + /// The is null. /// The is not a valid semantic version. public SemanticVersion(string version) { - var match = SemanticVersion.Regex.Match(version); + // parse + if (version == null) + throw new ArgumentNullException(nameof(version), "The input version string can't be null."); + var match = SemanticVersion.Regex.Match(version.Trim()); if (!match.Success) - throw new FormatException($"The input '{version}' is not a valid semantic version."); + throw new FormatException($"The input '{version}' isn't a valid semantic version."); + // initialise this.MajorVersion = int.Parse(match.Groups["major"].Value); this.MinorVersion = match.Groups["minor"].Success ? int.Parse(match.Groups["minor"].Value) : 0; this.PatchVersion = match.Groups["patch"].Success ? int.Parse(match.Groups["patch"].Value) : 0; - this.Build = match.Groups["build"].Success ? this.GetNormalisedTag(match.Groups["build"].Value) : null; + this.Build = match.Groups["prerelease"].Success ? this.GetNormalisedTag(match.Groups["prerelease"].Value) : null; } /// Get an integer indicating whether this version precedes (less than 0), supercedes (more than 0), or is equivalent to (0) the specified version. @@ -93,8 +104,8 @@ namespace StardewModdingAPI return curOlder; // compare two pre-release tag values - string[] curParts = this.Build.Split('.'); - string[] otherParts = other.Build.Split('.'); + string[] curParts = this.Build.Split('.', '-'); + string[] otherParts = other.Build.Split('.', '-'); for (int i = 0; i < curParts.Length; i++) { // longer prerelease tag supercedes if otherwise equal @@ -200,6 +211,7 @@ namespace StardewModdingAPI } } + /********* ** Private methods *********/ @@ -207,11 +219,9 @@ namespace StardewModdingAPI /// The tag to normalise. private string GetNormalisedTag(string tag) { - tag = tag?.Trim().Trim('-', '.'); - if (string.IsNullOrWhiteSpace(tag)) + tag = tag?.Trim(); + if (string.IsNullOrWhiteSpace(tag) || tag == "0") // '0' from incorrect examples in old SMAPI documentation return null; - if (tag == "0") - return null; // from incorrect examples in old SMAPI documentation return tag; } } -- cgit