make version parsing stricter, add unit tests for parsing (#309)

This commit is contained in:
Jesse Plamondon-Willard 2017-06-19 01:05:43 -04:00
parent ec914874ec
commit a011c28d40
4 changed files with 161 additions and 11 deletions

View File

@ -20,6 +20,9 @@ For players:
For modders:
* You can now specify minimum dependency versions in `manifest.json`.
* Added `System.ValueTuple.dll` to the SMAPI install package so mods can use [C# 7 value tuples](https://docs.microsoft.com/en-us/dotnet/csharp/tuples).
* Fixed `SemanticVersion` parsing some invalid versions into close approximations (like `1.apple` → `1.0-apple`).
* Fixed `SemanticVersion` not treating hyphens as separators when comparing prerelease tags.
<small>_(While that was technically correct, it leads to unintuitive behaviour like sorting `-alpha-2` _after_ `-alpha-10`, even though `-alpha.2` sorts before `-alpha.10`.)_</small>
## 1.14
See [log](https://github.com/Pathoschild/SMAPI/compare/1.13...1.14).

View File

@ -51,6 +51,7 @@
<Compile Include="..\GlobalAssemblyInfo.cs">
<Link>Properties\GlobalAssemblyInfo.cs</Link>
</Compile>
<Compile Include="Utilities\SemanticVersionTests.cs" />
<Compile Include="Utilities\SDateTests.cs" />
<Compile Include="Core\TranslationTests.cs" />
<Compile Include="Core\ModResolverTests.cs" />

View File

@ -0,0 +1,136 @@
using System;
using System.Diagnostics.CodeAnalysis;
using NUnit.Framework;
namespace StardewModdingAPI.Tests.Utilities
{
/// <summary>Unit tests for <see cref="SemanticVersion"/>.</summary>
[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<ArgumentNullException>(() => new SemanticVersion(input));
else
this.AssertAndLogException<FormatException>(() => 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<ArgumentException>(() => _ = new SDate(day, season, year), "Constructing the invalid date didn't throw the expected exception.");
//}
/*********
** Private methods
*********/
/// <summary>Assert that the expected exception type is thrown, and log the action output and thrown exception.</summary>
/// <typeparam name="T">The expected exception type.</typeparam>
/// <param name="action">The action which may throw the exception.</param>
/// <param name="message">The message to log if the expected exception isn't thrown.</param>
[SuppressMessage("ReSharper", "UnusedParameter.Local", Justification = "The message argument is deliberately only used in precondition checks since this is an assertion method.")]
private void AssertAndLogException<T>(Func<object> action, string message = null)
where T : Exception
{
this.AssertAndLogException<T>(() =>
{
object result = action();
TestContext.WriteLine($"Func result: {result}");
});
}
/// <summary>Assert that the expected exception type is thrown, and log the thrown exception.</summary>
/// <typeparam name="T">The expected exception type.</typeparam>
/// <param name="action">The action which may throw the exception.</param>
/// <param name="message">The message to log if the expected exception isn't thrown.</param>
[SuppressMessage("ReSharper", "UnusedParameter.Local", Justification = "The message argument is deliberately only used in precondition checks since this is an assertion method.")]
private void AssertAndLogException<T>(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.");
}
}
}

View File

@ -10,8 +10,14 @@ namespace StardewModdingAPI
** Properties
*********/
/// <summary>A regular expression matching a semantic version string.</summary>
/// <remarks>Derived from https://github.com/maxhauser/semver.</remarks>
private static readonly Regex Regex = new Regex(@"^(?<major>\d+)(\.(?<minor>\d+))?(\.(?<patch>\d+))?(?<build>.*)$", RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture);
/// <remarks>
/// This pattern is derived from the BNF documentation in the <a href="https://github.com/mojombo/semver">semver repo</a>,
/// 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.
/// </remarks>
private static readonly Regex Regex = new Regex(@"^(?<major>0|[1-9]\d*)\.(?<minor>0|[1-9]\d*)(\.(?<patch>0|[1-9]\d*))?(?:-(?<prerelease>([a-z0-9]+[\-\.]?)+))?$", RegexOptions.CultureInvariant | RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.ExplicitCapture);
/*********
@ -48,17 +54,22 @@ namespace StardewModdingAPI
/// <summary>Construct an instance.</summary>
/// <param name="version">The semantic version string.</param>
/// <exception cref="ArgumentNullException">The <paramref name="version"/> is null.</exception>
/// <exception cref="FormatException">The <paramref name="version"/> is not a valid semantic version.</exception>
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;
}
/// <summary>Get an integer indicating whether this version precedes (less than 0), supercedes (more than 0), or is equivalent to (0) the specified version.</summary>
@ -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
/// <param name="tag">The tag to normalise.</param>
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;
}
}