From 7dec51923418b269e111a266edb319ff3b0cb118 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 16 Apr 2022 18:29:52 -0400 Subject: [PATCH] fix broken unit tests --- .../NetFieldAnalyzerTests.cs | 2 +- src/SMAPI.Tests/Core/AssetNameTests.cs | 16 ++--- src/SMAPI.Tests/Core/ModResolverTests.cs | 60 ++++++++++--------- .../Clients/Wiki/ChangeDescriptor.cs | 7 ++- src/SMAPI/Framework/Content/AssetName.cs | 16 ++--- src/SMAPI/Framework/ModLoading/ModResolver.cs | 14 +++-- src/SMAPI/Framework/Translator.cs | 3 +- 7 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs index f11a59d3..a6fa5633 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -87,7 +87,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests [TestCase("SObject obj = null; if (obj.netRefProperty != null);", 24, "obj.netRefProperty", "NetRef", "object")] [TestCase("Item item = new Item(); object list = item.netList;", 38, "item.netList", "NetList", "object")] // ↓ NetList field converted to a non-interface type [TestCase("Item item = new Item(); object list = item.netCollection;", 38, "item.netCollection", "NetCollection", "object")] - [TestCase("Item item = new Item(); int x = (int)item.netIntField;", 32, "item.netIntField", "NetInt", "int")] // ↓ explicit conversion to invalid type + [TestCase("Item item = new Item(); int x = (int)item.netIntField;", 32, "item.netIntField", "NetFieldBase", "int")] // ↓ explicit conversion to invalid type [TestCase("Item item = new Item(); int x = item.netRefField as object;", 32, "item.netRefField", "NetRef", "object")] public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { diff --git a/src/SMAPI.Tests/Core/AssetNameTests.cs b/src/SMAPI.Tests/Core/AssetNameTests.cs index a1712726..655e9bae 100644 --- a/src/SMAPI.Tests/Core/AssetNameTests.cs +++ b/src/SMAPI.Tests/Core/AssetNameTests.cs @@ -78,9 +78,9 @@ namespace SMAPI.Tests.Core [TestCase("DATA\\achievements", "data/ACHIEVEMENTS", ExpectedResult = true)] [TestCase("DATA\\\\achievements", "data////ACHIEVEMENTS", ExpectedResult = true)] - // whitespace-sensitive - [TestCase("Data/Achievements", " Data/Achievements ", ExpectedResult = false)] - [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = false)] + // whitespace-insensitive + [TestCase("Data/Achievements", " Data/Achievements ", ExpectedResult = true)] + [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = true)] // other is null or whitespace [TestCase("Data/Achievements", null, ExpectedResult = false)] @@ -109,7 +109,7 @@ namespace SMAPI.Tests.Core [TestCase("Data/Achievements", "Data/Achievements", ExpectedResult = true)] [TestCase("DATA/achievements", "data/ACHIEVEMENTS", ExpectedResult = true)] [TestCase("DATA\\\\achievements", "data////ACHIEVEMENTS", ExpectedResult = true)] - [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = false)] + [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = true)] [TestCase("Data/Achievements", " ", ExpectedResult = false)] // with locale codes @@ -141,13 +141,13 @@ namespace SMAPI.Tests.Core [TestCase("DATA\\achievements", "data/ACHIEVEMENTS", ExpectedResult = true)] [TestCase("DATA\\\\achievements", "data////ACHIEVEMENTS", ExpectedResult = true)] - // leading-whitespace-sensitive - [TestCase("Data/Achievements", " Data/Achievements", ExpectedResult = false)] - [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = false)] + // whitespace-insensitive + [TestCase("Data/Achievements", " Data/Achievements", ExpectedResult = true)] + [TestCase(" Data/Achievements ", "Data/Achievements", ExpectedResult = true)] + [TestCase("Data/Achievements", " ", ExpectedResult = true)] // invalid prefixes [TestCase("Data/Achievements", null, ExpectedResult = false)] - [TestCase("Data/Achievements", " ", ExpectedResult = false)] // with locale codes [TestCase("Data/Achievements.fr-FR", "Data/Achievements", ExpectedResult = true)] diff --git a/src/SMAPI.Tests/Core/ModResolverTests.cs b/src/SMAPI.Tests/Core/ModResolverTests.cs index bd621bbf..6b2746f5 100644 --- a/src/SMAPI.Tests/Core/ModResolverTests.cs +++ b/src/SMAPI.Tests/Core/ModResolverTests.cs @@ -38,6 +38,9 @@ namespace SMAPI.Tests.Core // assert Assert.AreEqual(0, mods.Length, 0, $"Expected to find zero manifests, found {mods.Length} instead."); + + // cleanup + Directory.Delete(rootFolder, recursive: true); } [Test(Description = "Assert that the resolver correctly returns a failed metadata if there's an empty mod folder.")] @@ -56,6 +59,9 @@ namespace SMAPI.Tests.Core Assert.AreEqual(1, mods.Length, 0, $"Expected to find one manifest, found {mods.Length} instead."); Assert.AreEqual(ModMetadataStatus.Failed, mod!.Status, "The mod metadata was not marked failed."); Assert.IsNotNull(mod.Error, "The mod metadata did not have an error message set."); + + // cleanup + Directory.Delete(rootFolder, recursive: true); } [Test(Description = "Assert that the resolver correctly reads manifest data from a randomized file.")] @@ -115,6 +121,9 @@ namespace SMAPI.Tests.Core Assert.IsNotNull(mod.Manifest.Dependencies, "The dependencies field should not be null."); Assert.AreEqual(1, mod.Manifest.Dependencies.Length, "The dependencies field should contain one value."); Assert.AreEqual(originalDependency[nameof(IManifestDependency.UniqueID)], mod.Manifest.Dependencies[0].UniqueID, "The first dependency's unique ID doesn't match."); + + // cleanup + Directory.Delete(rootFolder, recursive: true); } /**** @@ -123,7 +132,7 @@ namespace SMAPI.Tests.Core [Test(Description = "Assert that validation doesn't fail if there are no mods installed.")] public void ValidateManifests_NoMods_DoesNothing() { - new ModResolver().ValidateManifests(Array.Empty(), apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); + new ModResolver().ValidateManifests(Array.Empty(), apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null, validateFilesExist: false); } [Test(Description = "Assert that validation skips manifests that have already failed without calling any other properties.")] @@ -134,7 +143,7 @@ namespace SMAPI.Tests.Core mock.Setup(p => p.Status).Returns(ModMetadataStatus.Failed); // act - new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); + new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null, validateFilesExist: false); // assert mock.VerifyGet(p => p.Status, Times.Once, "The validation did not check the manifest status."); @@ -145,13 +154,13 @@ namespace SMAPI.Tests.Core { // arrange Mock mock = this.GetMetadata("Mod A", Array.Empty(), allowStatusChange: true); - this.SetupMetadataForValidation(mock, new ModDataRecordVersionedFields(this.GetModDataRecord()) + mock.Setup(p => p.DataRecord).Returns(() => new ModDataRecordVersionedFields(this.GetModDataRecord()) { Status = ModStatus.AssumeBroken }); // act - new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); + new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null, validateFilesExist: false); // assert mock.Verify(p => p.SetStatus(ModMetadataStatus.Failed, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once, "The validation did not fail the metadata."); @@ -163,10 +172,9 @@ namespace SMAPI.Tests.Core // arrange Mock mock = this.GetMetadata("Mod A", Array.Empty(), allowStatusChange: true); mock.Setup(p => p.Manifest).Returns(this.GetManifest(minimumApiVersion: "1.1")); - this.SetupMetadataForValidation(mock); // act - new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); + new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null, validateFilesExist: false); // assert mock.Verify(p => p.SetStatus(ModMetadataStatus.Failed, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once, "The validation did not fail the metadata."); @@ -176,14 +184,18 @@ namespace SMAPI.Tests.Core public void ValidateManifests_MissingEntryDLL_Fails() { // arrange - Mock mock = this.GetMetadata(this.GetManifest(id: "Mod A", version: "1.0", entryDll: "Missing.dll"), allowStatusChange: true); - this.SetupMetadataForValidation(mock); + string directoryPath = this.GetTempFolderPath(); + Mock mock = this.GetMetadata(this.GetManifest(id: "Mod A", version: "1.0", entryDll: "Missing.dll"), allowStatusChange: true, directoryPath: directoryPath); + Directory.CreateDirectory(directoryPath); // act new ModResolver().ValidateManifests(new[] { mock.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); // assert mock.Verify(p => p.SetStatus(ModMetadataStatus.Failed, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once, "The validation did not fail the metadata."); + + // cleanup + Directory.Delete(directoryPath); } [Test(Description = "Assert that validation fails when multiple mods have the same unique ID.")] @@ -192,16 +204,13 @@ namespace SMAPI.Tests.Core // arrange Mock modA = this.GetMetadata("Mod A", Array.Empty(), allowStatusChange: true); Mock modB = this.GetMetadata(this.GetManifest(id: "Mod A", name: "Mod B", version: "1.0"), allowStatusChange: true); - Mock modC = this.GetMetadata("Mod C", Array.Empty(), allowStatusChange: false); - foreach (Mock mod in new[] { modA, modB, modC }) - this.SetupMetadataForValidation(mod); // act - new ModResolver().ValidateManifests(new[] { modA.Object, modB.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null); + new ModResolver().ValidateManifests(new[] { modA.Object, modB.Object }, apiVersion: new SemanticVersion("1.0"), getUpdateUrl: _ => null, validateFilesExist: false); // assert - modA.Verify(p => p.SetStatus(ModMetadataStatus.Failed, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once, "The validation did not fail the first mod with a unique ID."); - modB.Verify(p => p.SetStatus(ModMetadataStatus.Failed, It.IsAny(), It.IsAny(), It.IsAny()), Times.Once, "The validation did not fail the second mod with a unique ID."); + modA.Verify(p => p.SetStatus(ModMetadataStatus.Failed, ModFailReason.Duplicate, It.IsAny(), It.IsAny()), Times.AtLeastOnce, "The validation did not fail the first mod with a unique ID."); + modB.Verify(p => p.SetStatus(ModMetadataStatus.Failed, ModFailReason.Duplicate, It.IsAny(), It.IsAny()), Times.AtLeastOnce, "The validation did not fail the second mod with a unique ID."); } [Test(Description = "Assert that validation fails when the manifest references a DLL that does not exist.")] @@ -227,6 +236,9 @@ namespace SMAPI.Tests.Core // assert // if Moq doesn't throw a method-not-setup exception, the validation didn't override the status. + + // cleanup + Directory.Delete(modFolder, recursive: true); } /**** @@ -514,14 +526,20 @@ namespace SMAPI.Tests.Core /// Get a randomized basic manifest. /// The mod manifest. /// Whether the code being tested is allowed to change the mod status. - private Mock GetMetadata(IManifest manifest, bool allowStatusChange = false) + /// The directory path the mod metadata should be pointed at, or null to generate a fake path. + private Mock GetMetadata(IManifest manifest, bool allowStatusChange = false, string? directoryPath = null) { + directoryPath ??= this.GetTempFolderPath(); + Mock mod = new(MockBehavior.Strict); mod.Setup(p => p.DataRecord).Returns(this.GetModDataRecordVersionedFields()); mod.Setup(p => p.Status).Returns(ModMetadataStatus.Found); mod.Setup(p => p.DisplayName).Returns(manifest.UniqueID); + mod.Setup(p => p.DirectoryPath).Returns(directoryPath); mod.Setup(p => p.Manifest).Returns(manifest); mod.Setup(p => p.HasID(It.IsAny())).Returns((string id) => manifest.UniqueID == id); + mod.Setup(p => p.GetUpdateKeys(It.IsAny())).Returns(Enumerable.Empty()); + mod.Setup(p => p.GetRelativePathWithRoot()).Returns(directoryPath); if (allowStatusChange) { mod @@ -532,18 +550,6 @@ namespace SMAPI.Tests.Core return mod; } - /// Set up a mock mod metadata for . - /// The mock mod metadata. - /// The extra metadata about the mod from SMAPI's internal data (if any). - private void SetupMetadataForValidation(Mock mod, ModDataRecordVersionedFields? modRecord = null) - { - mod.Setup(p => p.Status).Returns(ModMetadataStatus.Found); - mod.Setup(p => p.Manifest).Returns(this.GetManifest()); - mod.Setup(p => p.DirectoryPath).Returns(Path.GetTempPath()); - mod.Setup(p => p.DataRecord).Returns(modRecord ?? this.GetModDataRecordVersionedFields()); - mod.Setup(p => p.GetUpdateKeys(It.IsAny())).Returns(Enumerable.Empty()); - } - /// Generate a default mod data record. private ModDataRecord GetModDataRecord() { diff --git a/src/SMAPI.Toolkit/Framework/Clients/Wiki/ChangeDescriptor.cs b/src/SMAPI.Toolkit/Framework/Clients/Wiki/ChangeDescriptor.cs index 8646f1cc..a2497dea 100644 --- a/src/SMAPI.Toolkit/Framework/Clients/Wiki/ChangeDescriptor.cs +++ b/src/SMAPI.Toolkit/Framework/Clients/Wiki/ChangeDescriptor.cs @@ -55,7 +55,12 @@ namespace StardewModdingAPI.Toolkit.Framework.Clients.Wiki { // get list List values = !string.IsNullOrWhiteSpace(rawField) - ? new List(rawField.Split(',')) + ? new List( + from field in rawField.Split(',') + let value = field.Trim() + where value.Length > 0 + select value + ) : new List(); // apply changes diff --git a/src/SMAPI/Framework/Content/AssetName.cs b/src/SMAPI/Framework/Content/AssetName.cs index 4c691b9a..148354a1 100644 --- a/src/SMAPI/Framework/Content/AssetName.cs +++ b/src/SMAPI/Framework/Content/AssetName.cs @@ -119,25 +119,27 @@ namespace StardewModdingAPI.Framework.Content if (prefix is null) return false; + string rawTrimmed = prefix.Trim(); + // asset keys can't have a leading slash, but NormalizeAssetName will trim them - { - string trimmed = prefix.TrimStart(); - if (trimmed.StartsWith('/') || trimmed.StartsWith('\\')) - return false; - } + if (rawTrimmed.StartsWith('/') || rawTrimmed.StartsWith('\\')) + return false; // normalize prefix { string normalized = PathUtilities.NormalizeAssetName(prefix); - string trimmed = prefix.TrimEnd(); - if (trimmed.EndsWith('/') || trimmed.EndsWith('\\')) + // keep trailing slash + if (rawTrimmed.EndsWith('/') || rawTrimmed.EndsWith('\\')) normalized += PathUtilities.PreferredAssetSeparator; prefix = normalized; } // compare + if (prefix.Length == 0) + return true; + return this.Name.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) && ( diff --git a/src/SMAPI/Framework/ModLoading/ModResolver.cs b/src/SMAPI/Framework/ModLoading/ModResolver.cs index 4a02e90d..74e7cb32 100644 --- a/src/SMAPI/Framework/ModLoading/ModResolver.cs +++ b/src/SMAPI/Framework/ModLoading/ModResolver.cs @@ -56,9 +56,10 @@ namespace StardewModdingAPI.Framework.ModLoading /// The mod manifests to validate. /// The current SMAPI version. /// Get an update URL for an update key (if valid). + /// Whether to validate that files referenced in the manifest (like ) exist on disk. This can be disabled to only validate the manifest itself. [SuppressMessage("ReSharper", "ConstantConditionalAccessQualifier", Justification = "Manifest values may be null before they're validated.")] [SuppressMessage("ReSharper", "ConditionIsAlwaysTrueOrFalse", Justification = "Manifest values may be null before they're validated.")] - public void ValidateManifests(IEnumerable mods, ISemanticVersion apiVersion, Func getUpdateUrl) + public void ValidateManifests(IEnumerable mods, ISemanticVersion apiVersion, Func getUpdateUrl, bool validateFilesExist = true) { mods = mods.ToArray(); @@ -141,11 +142,14 @@ namespace StardewModdingAPI.Framework.ModLoading } // file doesn't exist - string fileName = CaseInsensitivePathLookup.GetCachedFor(mod.DirectoryPath).GetFilePath(mod.Manifest.EntryDll!); - if (!File.Exists(Path.Combine(mod.DirectoryPath, fileName))) + if (validateFilesExist) { - mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its DLL '{mod.Manifest.EntryDll}' doesn't exist."); - continue; + string fileName = CaseInsensitivePathLookup.GetCachedFor(mod.DirectoryPath).GetFilePath(mod.Manifest.EntryDll!); + if (!File.Exists(Path.Combine(mod.DirectoryPath, fileName))) + { + mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its DLL '{mod.Manifest.EntryDll}' doesn't exist."); + continue; + } } } diff --git a/src/SMAPI/Framework/Translator.cs b/src/SMAPI/Framework/Translator.cs index b230a727..3beee250 100644 --- a/src/SMAPI/Framework/Translator.cs +++ b/src/SMAPI/Framework/Translator.cs @@ -51,7 +51,8 @@ namespace StardewModdingAPI.Framework foreach (string key in this.GetAllKeysRaw()) { string? text = this.GetRaw(key, locale, withFallback: true); - this.ForLocale.Add(key, new Translation(this.Locale, key, text)); + if (text != null) + this.ForLocale.Add(key, new Translation(this.Locale, key, text)); } }