From b2aff306a92edce881a9b7e6d0d60865144539d3 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Fri, 2 Oct 2026 01:16:49 -0600 Subject: [PATCH 1/2] Add unit tests for ArgumentParser and ColorChecker Characterization tests for how command arguments are joined back into messages and how config color names resolve to ChatColor, including the white fallback for mixed-case and unknown names. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../rpsystem/utils/ArgumentParserTest.java | 63 +++++++++++++++++++ .../rpsystem/utils/ColorCheckerTest.java | 62 ++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 src/test/java/dansplugins/rpsystem/utils/ArgumentParserTest.java create mode 100644 src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 9430082..fae7490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - An automated test suite, run by `mvn test` and by the existing `mvn package` step in CI, so a failing test now fails the build. The first tests lock down the `plugin.yml` contract: every permission node checked in code is registered or on an explicit knowingly-unregistered list (`rp.admin`, `rp.default`, `rp.card.show.others`, `rp.rphelp`), every registered node is checked somewhere, every registered command is dispatched by `CommandService` and vice versa, `rp.card.*` parents exactly the nine player card nodes and not `rp.card.forcesave` or `rp.card.forceload`, and the `USER_GUIDE.md` permission table matches what is registered. The `rp.card.*` guarantee is exercised through Bukkit's own `PermissibleBase` against a stub server, which is the check PR #335 could only establish with a throwaway class. This is the check that would have caught #321, #322 and #329 when they were introduced. `USER_GUIDE.md` gained the missing `rp.rphelp` entry under "Known Permission Discrepancies", which the new suite flagged on its first run. +- Unit tests for `ArgumentParser` and `ColorChecker.getColorByName`. They record current behavior without changing it: arguments are joined with single spaces and empty arguments are kept, and color names are matched only in all-lower-case or all-upper-case form (e.g. `dark_red` or `DARK_RED`), with any other name — including `Red` — resolving to white. ### Changed diff --git a/src/test/java/dansplugins/rpsystem/utils/ArgumentParserTest.java b/src/test/java/dansplugins/rpsystem/utils/ArgumentParserTest.java new file mode 100644 index 0000000..3af2813 --- /dev/null +++ b/src/test/java/dansplugins/rpsystem/utils/ArgumentParserTest.java @@ -0,0 +1,63 @@ +package dansplugins.rpsystem.utils; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * Characterizes the two ways command arguments are joined back into a message: the card setters, {@code /bird} + * and {@code /me} use {@link ArgumentParser#createStringFromFirstArgOnwards}, while {@code /whisper}, {@code /yell}, + * {@code /lo} and {@code /title} use {@link ArgumentParser#createStringFromArgs}. + */ +public class ArgumentParserTest { + private final ArgumentParser argumentParser = new ArgumentParser(); + + @Test + public void firstArgOnwardsJoinsFromTheStartingIndexWithSingleSpaces() { + String[] args = {"name", "Sir", "Roderick", "the", "Bold"}; + assertEquals("Sir Roderick the Bold", argumentParser.createStringFromFirstArgOnwards(args, 1)); + } + + @Test + public void firstArgOnwardsFromZeroJoinsEveryArgument() { + String[] args = {"draws", "a", "sword"}; + assertEquals("draws a sword", argumentParser.createStringFromFirstArgOnwards(args, 0)); + } + + @Test + public void firstArgOnwardsWithASingleRemainingArgumentHasNoTrailingSpace() { + String[] args = {"age", "42"}; + assertEquals("42", argumentParser.createStringFromFirstArgOnwards(args, 1)); + } + + @Test + public void firstArgOnwardsPastTheLastArgumentIsEmpty() { + // Callers check args.length first, but a start index at or past the end yields "", not an exception. + assertEquals("", argumentParser.createStringFromFirstArgOnwards(new String[]{"name"}, 1)); + assertEquals("", argumentParser.createStringFromFirstArgOnwards(new String[0], 0)); + } + + @Test + public void firstArgOnwardsKeepsEmptyArgumentsAsExtraSpaces() { + // Bukkit passes consecutive spaces through as empty arguments; they are joined, not collapsed. + String[] args = {"name", "Ser", "", "Brienne"}; + assertEquals("Ser Brienne", argumentParser.createStringFromFirstArgOnwards(args, 1)); + } + + @Test + public void argsJoinsEveryArgumentWithSingleSpaces() { + String[] args = {"Is", "anyone", "there?"}; + assertEquals("Is anyone there?", argumentParser.createStringFromArgs(args)); + } + + @Test + public void argsWithOneArgumentReturnsItUnchanged() { + assertEquals("Hello", argumentParser.createStringFromArgs(new String[]{"Hello"})); + } + + @Test(expected = ArrayIndexOutOfBoundsException.class) + public void argsWithNoArgumentsThrows() { + // Every caller checks args.length > 0 before calling, so this is never reached from a command. + argumentParser.createStringFromArgs(new String[0]); + } +} diff --git a/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java b/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java new file mode 100644 index 0000000..d52cf26 --- /dev/null +++ b/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java @@ -0,0 +1,62 @@ +package dansplugins.rpsystem.utils; + +import org.bukkit.ChatColor; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * Characterizes how the color names in {@code config.yml} ({@code localChatColor}, {@code emoteColor}, the alert + * colors and the rest documented in {@code CONFIG.md}) are resolved to a {@link ChatColor}. + * {@link ColorChecker#getColorByName} does not consult the plugin, so no plugin instance is needed. + */ +public class ColorCheckerTest { + private static final ChatColor[] SUPPORTED_COLORS = { + ChatColor.AQUA, ChatColor.BLACK, ChatColor.BLUE, ChatColor.DARK_AQUA, ChatColor.DARK_BLUE, + ChatColor.DARK_GRAY, ChatColor.DARK_GREEN, ChatColor.DARK_PURPLE, ChatColor.DARK_RED, ChatColor.GOLD, + ChatColor.GRAY, ChatColor.GREEN, ChatColor.LIGHT_PURPLE, ChatColor.RED, ChatColor.YELLOW, ChatColor.WHITE + }; + + private final ColorChecker colorChecker = new ColorChecker(null); + + @Test + public void everyColorIsResolvedFromItsLowerCaseName() { + for (ChatColor color : SUPPORTED_COLORS) { + assertEquals(color, colorChecker.getColorByName(color.name().toLowerCase())); + } + } + + @Test + public void everyColorIsResolvedFromItsUpperCaseName() { + for (ChatColor color : SUPPORTED_COLORS) { + assertEquals(color, colorChecker.getColorByName(color.name())); + } + } + + @Test + public void mixedCaseNamesAreNotRecognizedAndFallBackToWhite() { + // Only the all-lower-case and all-upper-case spellings are matched. + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("Red")); + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("Dark_Red")); + } + + @Test + public void unknownNamesFallBackToWhite() { + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("pink")); + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("dark-red")); + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("")); + } + + @Test + public void formattingCodesAreNotColorsAndFallBackToWhite() { + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("bold")); + assertEquals(ChatColor.WHITE, colorChecker.getColorByName("ITALIC")); + } + + @Test(expected = NullPointerException.class) + public void aNullNameThrows() { + // A switch on a null String throws. ConfigService backfills every color option on enable, so callers are + // not expected to pass null; this records the behavior rather than endorsing it. + colorChecker.getColorByName(null); + } +} From a813e83c4449301ce9aba97066fa56327f3dd2f7 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Fri, 2 Oct 2026 01:18:35 -0600 Subject: [PATCH 2/2] Pin Locale.ENGLISH when lower-casing color names in ColorCheckerTest Matches PluginYmlContractTest; under a Turkish default locale LIGHT_PURPLE would otherwise lower-case to a dotless-i spelling. Co-Authored-By: Claude Opus 5.5 --- .../java/dansplugins/rpsystem/utils/ColorCheckerTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java b/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java index d52cf26..29c1bb5 100644 --- a/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java +++ b/src/test/java/dansplugins/rpsystem/utils/ColorCheckerTest.java @@ -3,6 +3,8 @@ import org.bukkit.ChatColor; import org.junit.Test; +import java.util.Locale; + import static org.junit.Assert.assertEquals; /** @@ -22,7 +24,7 @@ public class ColorCheckerTest { @Test public void everyColorIsResolvedFromItsLowerCaseName() { for (ChatColor color : SUPPORTED_COLORS) { - assertEquals(color, colorChecker.getColorByName(color.name().toLowerCase())); + assertEquals(color, colorChecker.getColorByName(color.name().toLowerCase(Locale.ENGLISH))); } }