From 11c87be87f485fa4ae4904cb753d313ea0c5cdf5 Mon Sep 17 00:00:00 2001 From: Devin Date: Tue, 16 Nov 2021 19:48:01 +0100 Subject: [PATCH] Add unit tests and fixes --- .../Inventories/PlayerInventory.asset | 2 +- Assets/Scenes/NewShop.unity | 4 +- Assets/Scripts/Shop/Model/BuyModel.cs | 8 +- Assets/Scripts/Shop/Model/Inventory.cs | 2 +- Assets/Scripts/Shop/Model/ShopModel.cs | 4 + Assets/Scripts/Shop/Tests/ShopUnitTests.cs | 256 +++++++++++++++++- SAShop.csproj | 40 +++ 7 files changed, 299 insertions(+), 17 deletions(-) diff --git a/Assets/Resources/Inventories/PlayerInventory.asset b/Assets/Resources/Inventories/PlayerInventory.asset index 6a4a00f..4e1d26b 100644 --- a/Assets/Resources/Inventories/PlayerInventory.asset +++ b/Assets/Resources/Inventories/PlayerInventory.asset @@ -9,7 +9,7 @@ MonoBehaviour: m_GameObject: {fileID: 0} m_Enabled: 1 m_EditorHideFlags: 0 - m_Script: {fileID: 0} + m_Script: {fileID: 11500000, guid: 6b56b3d0165a9714ab2efbdf15c22d0d, type: 3} m_Name: PlayerInventory m_EditorClassIdentifier: SAShop::InventoryInitVals Money: 5500 diff --git a/Assets/Scenes/NewShop.unity b/Assets/Scenes/NewShop.unity index 239a04b..830538a 100644 --- a/Assets/Scenes/NewShop.unity +++ b/Assets/Scenes/NewShop.unity @@ -15293,7 +15293,7 @@ GameObject: - component: {fileID: 1373665447} - component: {fileID: 1373665446} m_Layer: 5 - m_Name: ShopListBuyView + m_Name: ShopListUpgradeView m_TagString: Untagged m_Icon: {fileID: 0} m_NavMeshLayer: 0 @@ -15721,7 +15721,7 @@ GameObject: - component: {fileID: 1396837743} - component: {fileID: 1396837742} m_Layer: 5 - m_Name: ShopGridBuyView + m_Name: ShopGridUpgradeView m_TagString: Untagged m_Icon: {fileID: 0} m_NavMeshLayer: 0 diff --git a/Assets/Scripts/Shop/Model/BuyModel.cs b/Assets/Scripts/Shop/Model/BuyModel.cs index d46273a..39c3c09 100644 --- a/Assets/Scripts/Shop/Model/BuyModel.cs +++ b/Assets/Scripts/Shop/Model/BuyModel.cs @@ -32,7 +32,7 @@ public class BuyModel : ShopModel if (tradePartner == null) { Debug.Assert(false,"Could not make trade because the shop has no trade partner"); - return; + throw new NoTradePartnerException(); } // if (tradePartner.Money < GetSelectedItem().basePrice * priceModifier) @@ -54,6 +54,10 @@ public class BuyModel : ShopModel } } -public class NotEnoughMoneyException : Exception +public class NoTradePartnerException : Exception +{ +} + +public class InsufficientMoneyException : Exception { } diff --git a/Assets/Scripts/Shop/Model/Inventory.cs b/Assets/Scripts/Shop/Model/Inventory.cs index 8de24ae..0ca508b 100644 --- a/Assets/Scripts/Shop/Model/Inventory.cs +++ b/Assets/Scripts/Shop/Model/Inventory.cs @@ -114,7 +114,7 @@ public class Inventory : IModelObservable // Borrowing the model observ public void ChangeBalance(int change) { if (Money + change < 0) - throw new NotEnoughMoneyException(); // We can't change the balance if it's too expensive + throw new InsufficientMoneyException(); // We can't change the balance if it's too expensive Money += change; NotifyObservers(); } diff --git a/Assets/Scripts/Shop/Model/ShopModel.cs b/Assets/Scripts/Shop/Model/ShopModel.cs index c3537a5..5b0a8e3 100644 --- a/Assets/Scripts/Shop/Model/ShopModel.cs +++ b/Assets/Scripts/Shop/Model/ShopModel.cs @@ -76,6 +76,10 @@ public abstract class ShopModel : IModelObservable { SelectItemByIndex(index); } + else + { + throw new ArgumentException("Item is not part of this model"); + } } } diff --git a/Assets/Scripts/Shop/Tests/ShopUnitTests.cs b/Assets/Scripts/Shop/Tests/ShopUnitTests.cs index 29404c6..209fa8e 100644 --- a/Assets/Scripts/Shop/Tests/ShopUnitTests.cs +++ b/Assets/Scripts/Shop/Tests/ShopUnitTests.cs @@ -1,4 +1,5 @@ -using System.Collections; +using System; +using System.Collections; using NUnit.Framework; using UnityEngine; using UnityEngine.SceneManagement; @@ -9,6 +10,8 @@ namespace Tests public class ShopUnitTests { private ShopView shopView; //This is the grid buy view we want to test + private SellViewGrid sellView; + private ShopView upgradeView; //Setup the test scene [OneTimeSetUp] @@ -28,8 +31,15 @@ namespace Tests //The shop scene only contains one grid buy view, we use Resources.FindObjectsOfTypeAll to get the reference to it, //Resources.FFindObjectsOfTypeAll is used instead of GameObject.Find because the later can't find disabled objects - shopView = Resources.FindObjectsOfTypeAll()[0]; - + var shops = Resources.FindObjectsOfTypeAll(); + shopView = shops[2]; // For some reason this reverses it. We just want to hardcode it for this scene, no annoying for loops to compare + upgradeView = shops[1]; + + sellView = Resources.FindObjectsOfTypeAll()[0]; + + // Turn on all views so they function correctly. We do not care for the looks during unit tests, obviously. + sellView.gameObject.SetActive(true); + upgradeView.gameObject.SetActive(true); //Active the gridBuyView game object to initialize the class, if we don't do this 'void Start()' won't be called //You should active all the game objects that are involved in the test before testing the functions from their components shopView.gameObject.SetActive(true); @@ -46,7 +56,7 @@ namespace Tests } //This case tests if the grid buy view displays the correct amount of Items - [UnityTest] + [UnityTest,Order(1)] // Run this one first to see if we populated everything correctly public IEnumerator ShopGridBuyViewDisplaysCorrectAmountOfItems() { yield return null; //yield return null skips one frame, waits for the Unity scene to load @@ -59,10 +69,28 @@ namespace Tests new WaitForEndOfFrame(); //Since we are testing how many items are displayed, we should use WaitForEndOfFrame to wait until the end of the frame, //so that the view finished updating and rendering everything + yield return null; int itemCount = gridItemsPanel.transform.childCount; Assert.AreEqual(shopView.ShopModel.inventory.GetItemCount(), itemCount, "The generated item count is not equal to shopModel's itemCount"); } + + // This case tests if the shop model gets populated with items + [UnityTest,Order(0)] // Run this one before the view tests to see if the factory works + public IEnumerator ShopFactoryPopulatesShop() + { + yield return null; //yield return null skips one frame, waits for the Unity scene to load + + //Now that the scene is loaded and the gridBuyView game object was activated in SetupTests(), we can use GameObject.Find + //to find the game object we want to test + var count = 0; + Assert.DoesNotThrow(delegate + { + count = shopView.ShopModel.inventory.GetItemCount(); + }); + Assert.Greater(count,1); // Having just one item probably means something went wrong and crashed + + } //This case tests if the buyModel can throw an ArgumentOutOfRangeException when it's asked to select an item by a negative //index. Incorrect indexes can be generated from bugs in views or controllers, throwing the correct type of exceptions is @@ -88,15 +116,221 @@ namespace Tests { yield return null; - Item item = shopView.ShopModel.GetSelectedItem(); - var highlight = shopView.transform.Find("Highlight"); // Conveniently this will only return an active object named Highlight. The name's hardcoded which is bad though. - //Creates a delegate that call gridBuyView.ShopModel.SelectItemByIndex(-1), the test runner will run the function, and - //check if an ArgumentOutOfRangeException is thrown, the unit test would fail if no ArgumentOutOfRangeException - //was thrown + var selectedItem = shopView.transform.Find("GridItemsPanel"); + var active = selectedItem.GetComponentInChildren(); // In grid view, one item panel will be active. That's the selected item! shopView.ShopModel.SelectItemByIndex(shopView.ShopModel.GetSelectedItemIndex() + 1); - var newHighlight = shopView.transform.Find("Highlight"); + var newActive = selectedItem.GetComponentInChildren(); + + Assert.AreNotSame(active, newActive,"Previous item is still selected!"); + } + + // This test case tests that either no item is selected, or exactly one item is selected + [UnityTest] + public IEnumerator NoMoreThanOneItemSelectedInView() + { + yield return null; + + var selectedItem = shopView.transform.Find("GridItemsPanel"); + var active = selectedItem.GetComponentsInChildren(); // In grid view, one item panel will be active. That's the selected item! + + Assert.Greater(2,active.Length,"Too many items selected!"); // Depending on what's up, either we want none or just one selected now + shopView.ShopModel.SelectItemByIndex(shopView.ShopModel.GetSelectedItemIndex() + 1); + shopView.ShopModel.SelectItemByIndex(shopView.ShopModel.GetSelectedItemIndex() + 1); + active = selectedItem.GetComponentsInChildren(); // In grid view, one item panel will be active. That's the selected item! + UnityEngine.Assertions.Assert.AreEqual(active.Length,1,"Too many items or no item selected!"); // Now we definitely need one to be selected! + } + + // This test case tests if the currently selected item disappears from the view when the buy model confirms the selection + [UnityTest] + public IEnumerator ModelSelectionRemovesViewItem() + { + yield return null; + var selectedItem = shopView.transform.Find("GridItemsPanel"); + shopView.ShopModel.SelectItemByIndex(shopView.ShopModel.GetSelectedItemIndex() + 1); + + yield return null; + var active = selectedItem.GetComponentInChildren(); // In grid view, one item panel will be active. That's the selected item! + shopView.ShopModel.ConfirmSelectedItem(); + yield return null; + UnityEngine.Assertions.Assert.IsTrue(active == null,"Selected item view was not destroyed when bought!"); // Now we definitely need one to be selected! + } + + // Tests if the player inventory doesn't allow to overdraw money. We test on shop inventory even though that one's money's technically irrelevant, but works the same on all + [UnityTest] + public IEnumerator InventoryDoesNotAllowExceedingTransactions() + { + yield return null; + // So if we pay too much, we expect an exception + Assert.Throws(delegate + { + shopView.ShopModel.inventory.ChangeBalance(-shopView.ShopModel.inventory.Money - + 1); // Pay one money more than it's got! + }); + } + + // Tests if transaction changes actually change an inventory's balance + [UnityTest] + public IEnumerator InventoryCorrectlyHandlesBalanceChange() + { + yield return null; + + var moneyBefore = shopView.ShopModel.inventory.Money; + shopView.ShopModel.inventory.ChangeBalance(-shopView.ShopModel.inventory.Money); // Spend all our money! + + Assert.AreNotEqual(moneyBefore,shopView.ShopModel.inventory.Money); + Assert.Zero(shopView.ShopModel.inventory.Money); + shopView.ShopModel.inventory.ChangeBalance(moneyBefore + 1); // For good measure, re-add all money plus a one money bonus + + Assert.AreEqual(moneyBefore + 1,shopView.ShopModel.inventory.Money); + } + + // Tests if a model throws an error when trying to select an item directly that isn't part of it + [UnityTest] + public IEnumerator ModelSelectionErrorIfInvalid() + { + yield return null; + + Assert.Throws(delegate + { + shopView.ShopModel.SelectItem(new ItemPotion("some name","invalidIcon",5,2,PotionType.Healing)); + }); + } + + // Tests if a model selects an item correctly when it's valid + [UnityTest] + public IEnumerator ModelSelectionIndexCorrectIfValid() + { + yield return null; - Assert.IsTrue(highlight != newHighlight,"Previous item is still selected!"); + var item = shopView.ShopModel.GetSelectedItem(); + shopView.ShopModel.SelectItemByIndex(shopView.ShopModel.GetSelectedItemIndex() + 1); + + Assert.AreNotSame(item,shopView.ShopModel.GetSelectedItem()); + + Assert.DoesNotThrow(delegate + { + shopView.ShopModel.SelectItem(item); + }); + + Assert.AreSame(item,shopView.ShopModel.GetSelectedItem()); + } + + // Tests if a buy model removes an item on confirmation, and then selects another one + [UnityTest] + public IEnumerator BuyModelConfirmationRemovesItemAndSelectsNew() + { + yield return null; + + var item = shopView.ShopModel.GetSelectedItem(); + shopView.ShopModel.ConfirmSelectedItem(); + + Assert.AreNotSame(item,shopView.ShopModel.GetSelectedItem()); + + Assert.Throws(delegate + { + shopView.ShopModel.SelectItem(item); + }); + + Assert.AreNotSame(item,shopView.ShopModel.GetSelectedItem()); + } + + // Tests if a buy model removes money from an inventory it buys from + [UnityTest] + public IEnumerator BuyingItemCostsMoney() + { + yield return null; + + var money = sellView.ShopModel.inventory.Money; // In the scene setup, sellView happens to reference the player inventory the shop trades with + Assert.DoesNotThrow(delegate + { + shopView.ShopModel.ConfirmSelectedItem(); + }); + + Assert.Less(sellView.ShopModel.inventory.Money,money); + } + + // Tests if a sell model gains money from selling + [UnityTest] + public IEnumerator SellingItemGivesMoney() + { + yield return null; + + var money = sellView.ShopModel.inventory.Money; // In the scene setup, sellView happens to reference the player inventory the shop trades with + Assert.DoesNotThrow(delegate + { + sellView.ShopModel.ConfirmSelectedItem(); + }); + + Assert.Greater(sellView.ShopModel.inventory.Money,money); + } + + // Tests if selling removes the item from the sell inventory + [UnityTest] + public IEnumerator SellingItemRemovesFromInventory() + { + yield return null; + + var item = sellView.ShopModel.GetSelectedItem(); + sellView.ShopModel.ConfirmSelectedItem(); + + Assert.AreNotSame(item,sellView.ShopModel.GetSelectedItem()); + + Assert.Throws(delegate + { + sellView.ShopModel.SelectItem(item); + }); + + Assert.AreNotSame(item,sellView.ShopModel.GetSelectedItem()); + } + + // Tests if upgrading an item does not remove it + [UnityTest] + public IEnumerator UpgradingKeepsInInventory() + { + yield return null; + + var item = upgradeView.ShopModel.GetSelectedItem(); + upgradeView.ShopModel.ConfirmSelectedItem(); + + Assert.AreSame(item,upgradeView.ShopModel.GetSelectedItem()); + + Assert.DoesNotThrow(delegate + { + upgradeView.ShopModel.SelectItem(item); + }); + + Assert.AreSame(item,upgradeView.ShopModel.GetSelectedItem()); + } + + // Tests if upgrading costs money + [UnityTest] + public IEnumerator UpgradingCostsMoney() + { + yield return null; + + var money = upgradeView.ShopModel.inventory.Money; // In the scene setup, sellView happens to reference the player inventory the shop trades with + Assert.DoesNotThrow(delegate + { + upgradeView.ShopModel.ConfirmSelectedItem(); + }); + + Assert.Less(upgradeView.ShopModel.inventory.Money,money); + } + + // Tests if upgrading improves an item's stats + [UnityTest] + public IEnumerator UpgradingImprovesStats() + { + yield return null; + + var item = upgradeView.ShopModel.GetSelectedItem(); // In the scene setup, sellView happens to reference the player inventory the shop trades with + var stats = item.GetStats(); // Stats right now are turned into a generic string type. If we get a different one post-upgrade, it should've worked + Assert.DoesNotThrow(delegate + { + upgradeView.ShopModel.ConfirmSelectedItem(); + }); + + Assert.AreNotEqual(stats,upgradeView.ShopModel.GetSelectedItem().GetStats()); } } } \ No newline at end of file diff --git a/SAShop.csproj b/SAShop.csproj index 50879b8..93598b7 100644 --- a/SAShop.csproj +++ b/SAShop.csproj @@ -41,23 +41,63 @@ false + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + C:/Program Files/Unity/Hub/Editor/2019.4.17f1/Editor/Data/Managed/UnityEngine/UnityEngine.dll