From 66c9de51cec4d35eb9eec279e8cac101c334ecc5 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sun, 2 Aug 2026 16:41:25 -0600 Subject: [PATCH 1/2] Fix lore command index handling and default-command typo - /pl edit and /pl remove now treat lineIndex as 1-based (matching COMMANDS.md/USER_GUIDE.md) instead of indexing 0-based, and validate missing/non-numeric input instead of throwing - DefaultCommand now points players at /pl help instead of the non-existent /lp help - Add JUnit 5 + Mockito as the project's first test dependencies and cover the fixed command paths Closes #7, Closes #8 Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 5 + pom.xml | 18 ++++ .../playerlore/commands/DefaultCommand.java | 2 +- .../playerlore/commands/EditCommand.java | 17 +++- .../playerlore/commands/RemoveCommand.java | 17 +++- .../commands/DefaultCommandTest.java | 23 +++++ .../playerlore/commands/EditCommandTest.java | 91 +++++++++++++++++++ .../commands/RemoveCommandTest.java | 87 ++++++++++++++++++ 8 files changed, 253 insertions(+), 7 deletions(-) create mode 100644 src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java create mode 100644 src/test/java/dansplugins/playerlore/commands/EditCommandTest.java create mode 100644 src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 12dcaa7..58204a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed +- `/pl edit` and `/pl remove` now treat `lineIndex` as 1-based, matching `COMMANDS.md`/`USER_GUIDE.md`, instead of silently indexing into the lore list as 0-based +- `/pl edit` and `/pl remove` no longer throw an uncaught exception when given a non-numeric index or no index at all; they now send a player-facing error message +- The default (no-argument) command no longer tells players to type the non-existent `/lp help`; it now correctly says `/pl help` + ## [1.1] ### Added diff --git a/pom.xml b/pom.xml index f311441..f072e91 100644 --- a/pom.xml +++ b/pom.xml @@ -14,6 +14,7 @@ 1.8 UTF-8 + 5.10.2 @@ -27,6 +28,11 @@ ${java.version} + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + org.apache.maven.plugins maven-shade-plugin @@ -80,5 +86,17 @@ 1.0 compile + + org.junit.jupiter + junit-jupiter + ${junit.version} + test + + + org.mockito + mockito-core + 5.11.0 + test + diff --git a/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java b/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java index 455ebe1..1f63b4b 100644 --- a/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java @@ -27,7 +27,7 @@ public boolean execute(CommandSender commandSender) { commandSender.sendMessage(ChatColor.AQUA + "Requested by: Rochelle"); commandSender.sendMessage(ChatColor.AQUA + "Wiki: https://github.com/dmccoystephenson/PlayerLore/wiki"); commandSender.sendMessage(""); - commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /lp help"); + commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /pl help"); return true; } diff --git a/src/main/java/dansplugins/playerlore/commands/EditCommand.java b/src/main/java/dansplugins/playerlore/commands/EditCommand.java index fdbac37..79bd7d5 100644 --- a/src/main/java/dansplugins/playerlore/commands/EditCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/EditCommand.java @@ -38,7 +38,18 @@ public boolean execute(CommandSender commandSender, String[] args) { Player player = (Player) commandSender; // get line to edit - int lineIndex = Integer.parseInt(args[0]); + if (args.length == 0) { + player.sendMessage(ChatColor.RED + "Usage: /pl edit (lineIndex) \"new line of lore\""); + return false; + } + + int lineIndex; + try { + lineIndex = Integer.parseInt(args[0]); + } catch (NumberFormatException e) { + player.sendMessage(ChatColor.RED + "Line index must be a number."); + return false; + } // get line of lore ArgumentParser argumentParser = new ArgumentParser(); @@ -69,12 +80,12 @@ public boolean execute(CommandSender commandSender, String[] args) { lore = new ArrayList<>(); } - if (lineIndex >= lore.size()) { + if (lineIndex < 1 || lineIndex > lore.size()) { player.sendMessage(ChatColor.RED + "There aren't that many lines of lore."); return false; } - lore.set(lineIndex, ChatColor.WHITE + lineOfLore); + lore.set(lineIndex - 1, ChatColor.WHITE + lineOfLore); itemMeta.setLore(lore); item.setItemMeta(itemMeta); diff --git a/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java b/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java index 881d50a..bf0ee36 100644 --- a/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java @@ -37,7 +37,18 @@ public boolean execute(CommandSender commandSender, String[] args) { Player player = (Player) commandSender; // get line to edit - int lineIndex = Integer.parseInt(args[0]); + if (args.length == 0) { + player.sendMessage(ChatColor.RED + "Usage: /pl remove (lineIndex)"); + return false; + } + + int lineIndex; + try { + lineIndex = Integer.parseInt(args[0]); + } catch (NumberFormatException e) { + player.sendMessage(ChatColor.RED + "Line index must be a number."); + return false; + } // get item ItemStack item = player.getInventory().getItemInMainHand(); @@ -59,12 +70,12 @@ public boolean execute(CommandSender commandSender, String[] args) { lore = new ArrayList<>(); } - if (lineIndex >= lore.size()) { + if (lineIndex < 1 || lineIndex > lore.size()) { player.sendMessage(ChatColor.RED + "There aren't that many lines of lore."); return false; } - lore.remove(lineIndex); + lore.remove(lineIndex - 1); itemMeta.setLore(lore); item.setItemMeta(itemMeta); diff --git a/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java b/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java new file mode 100644 index 0000000..05ccd2a --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java @@ -0,0 +1,23 @@ +package dansplugins.playerlore.commands; + +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import dansplugins.playerlore.PlayerLore; +import org.bukkit.command.CommandSender; +import org.junit.jupiter.api.Test; + +public class DefaultCommandTest { + + @Test + public void execute_pointsPlayersToTheRegisteredHelpCommand() { + PlayerLore playerLore = mock(PlayerLore.class); + CommandSender commandSender = mock(CommandSender.class); + DefaultCommand defaultCommand = new DefaultCommand(playerLore); + + defaultCommand.execute(commandSender); + + verify(commandSender).sendMessage(contains("/pl help")); + } +} diff --git a/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java new file mode 100644 index 0000000..8008730 --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java @@ -0,0 +1,91 @@ +package dansplugins.playerlore.commands; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.bukkit.ChatColor; +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class EditCommandTest { + + private EditCommand editCommand; + private Player player; + private PlayerInventory inventory; + private ItemStack item; + private ItemMeta itemMeta; + + @BeforeEach + public void setUp() { + editCommand = new EditCommand(); + player = mock(Player.class); + inventory = mock(PlayerInventory.class); + item = mock(ItemStack.class); + itemMeta = mock(ItemMeta.class); + + when(player.getInventory()).thenReturn(inventory); + when(inventory.getItemInMainHand()).thenReturn(item); + when(item.getType()).thenReturn(Material.DIAMOND_SWORD); + when(item.getItemMeta()).thenReturn(itemMeta); + } + + @Test + public void execute_editsFirstLoreLineUsing1BasedIndex() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"1", "\"new line\""}); + + assertTrue(result); + verify(itemMeta).setLore(Arrays.asList(ChatColor.WHITE + "new line")); + } + + @Test + public void execute_rejectsIndexBelow1() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"0", "\"new line\""}); + + assertFalse(result); + verify(player).sendMessage(anyString()); + } + + @Test + public void execute_rejectsIndexBeyondLoreSize() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"2", "\"new line\""}); + + assertFalse(result); + } + + @Test + public void execute_rejectsNonNumericIndex() { + boolean result = editCommand.execute(player, new String[]{"abc", "\"new line\""}); + + assertFalse(result); + } + + @Test + public void execute_rejectsMissingArgs() { + boolean result = editCommand.execute(player, new String[]{}); + + assertFalse(result); + } +} diff --git a/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java b/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java new file mode 100644 index 0000000..55d89b9 --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java @@ -0,0 +1,87 @@ +package dansplugins.playerlore.commands; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class RemoveCommandTest { + + private RemoveCommand removeCommand; + private Player player; + private PlayerInventory inventory; + private ItemStack item; + private ItemMeta itemMeta; + + @BeforeEach + public void setUp() { + removeCommand = new RemoveCommand(); + player = mock(Player.class); + inventory = mock(PlayerInventory.class); + item = mock(ItemStack.class); + itemMeta = mock(ItemMeta.class); + + when(player.getInventory()).thenReturn(inventory); + when(inventory.getItemInMainHand()).thenReturn(item); + when(item.getType()).thenReturn(Material.DIAMOND_SWORD); + when(item.getItemMeta()).thenReturn(itemMeta); + } + + @Test + public void execute_removesFirstLoreLineUsing1BasedIndex() { + List lore = new ArrayList<>(Arrays.asList("first line", "second line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"1"}); + + assertTrue(result); + verify(itemMeta).setLore(Arrays.asList("second line")); + } + + @Test + public void execute_rejectsIndexBelow1() { + List lore = new ArrayList<>(Arrays.asList("first line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"0"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsIndexBeyondLoreSize() { + List lore = new ArrayList<>(Arrays.asList("first line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"2"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsNonNumericIndex() { + boolean result = removeCommand.execute(player, new String[]{"abc"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsMissingArgs() { + boolean result = removeCommand.execute(player, new String[]{}); + + assertFalse(result); + } +} From f45565dbef6c40baf5a49ef014a11e6269b3a1cc Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sun, 2 Aug 2026 16:44:29 -0600 Subject: [PATCH 2/2] Remove unused assertEquals import in EditCommandTest Co-Authored-By: Claude Sonnet 5 --- .../java/dansplugins/playerlore/commands/EditCommandTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java index 8008730..b99212e 100644 --- a/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java +++ b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java @@ -1,6 +1,5 @@ package dansplugins.playerlore.commands; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyString;