Improve and extend command tests

- Added param converter tests
- Refactored test functions to have more descriptive names and have documentation and to make their behaviour more predictable (at a glance)
- Implement separation of user error in the command system and make tests for it more specific
- Improve handling of testing messages received from commands (like the help text)

Chroma-Core/src/main/java/buttondevteam/lib/ChromaUtils.kt

@@ -1,6 +1,7 @@
 package buttondevteam.lib
 
 import buttondevteam.core.MainPlugin
+import buttondevteam.lib.test.TestException
 import org.bukkit.Bukkit
 import org.bukkit.command.CommandSender
 import org.bukkit.entity.Player
@@ -90,7 +91,7 @@ object ChromaUtils {
     fun throwWhenTested(exception: Throwable, message: String) {
         if (isTest) {
             // Propagate exception back to the tests
-            throw Exception(message, exception)
+            throw TestException(message, exception)
         } else {
             // Otherwise we don't run the code directly, so we need to handle this here
             TBMCCoreAPI.SendException(message, exception, MainPlugin.instance)

Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2.kt

@@ -101,6 +101,8 @@ abstract class Command2<TC : ICommand2<TP>, TP : Command2Sender>(
      *
      * @param sender The sender who sent the command
      * @param commandline The command line, including the leading command char
+     *
+     * @return Whether the main command exists
      */
     open fun handleCommand(sender: TP, commandline: String): Boolean {
         val results = dispatcher.parse(commandline.removePrefix("/"), sender)
@@ -348,7 +350,7 @@ abstract class Command2<TC : ICommand2<TP>, TP : Command2Sender>(
         val sender = context.source
 
         if (!sd.hasPermission(sender)) {
-            sender.sendMessage("${ChatColor.RED}You don't have permission to use this command")
+            CommandUtils.reportUserError(sender, "${ChatColor.RED}You don't have permission to use this command")
             return 1
         }
         // TODO: WIP
@@ -357,7 +359,7 @@ abstract class Command2<TC : ICommand2<TP>, TP : Command2Sender>(
         if (convertedSender == null) {
             //TODO: Should have a prettier display of Command2 classes here
             val type = sd.senderType.simpleName.fold("") { s, ch -> s + if (ch.isUpperCase()) " " + ch.lowercase() else ch }.trim()
-            sender.sendMessage("${ChatColor.RED}You need to be a $type to use this command.")
+            CommandUtils.reportUserError(sender, "${ChatColor.RED}You need to be a $type to use this command.")
             executeHelpText(context) //Send what the command is about, could be useful for commands like /member where some subcommands aren't player-only
             return 0
         }
@@ -384,7 +386,7 @@ abstract class Command2<TC : ICommand2<TP>, TP : Command2Sender>(
                         ?: error("No suitable converter found for ${argument.type} ${argument.name}")
                     val result = converter.converter.apply(userArgument)
                     if (result == null) {
-                        sender.sendMessage("${ChatColor.RED}Error: ${converter.errormsg}")
+                        CommandUtils.reportUserError(sender, "${ChatColor.RED}Error: ${converter.errormsg}")
                         return null
                     }
                     params.add(result)

Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2MC.kt

@@ -137,14 +137,11 @@ class Command2MC : Command2<ICommand2MC, Command2MCSender>('/', true), Listener
     }
 
     fun unregisterCommands(plugin: ButtonPlugin) {
-        unregisterCommandIf({ node -> Optional.ofNullable(node.data.command).map { obj -> obj.plugin }.map { obj -> plugin == obj }.orElse(false) }, true)
+        unregisterCommandIf({ node -> node.data.command.plugin == plugin }, true)
     }
 
     fun unregisterCommands(component: Component<*>) {
-        unregisterCommandIf({ node ->
+        unregisterCommandIf({ node -> node.data.command.component == component }, true)
-            Optional.ofNullable(node.data.command).map { obj: ICommand2MC -> obj.plugin }
-                .map { comp: ButtonPlugin -> component.javaClass.simpleName == comp.javaClass.simpleName }.orElse(false)
-        }, true)
     }
 
     override fun handleCommand(sender: Command2MCSender, commandline: String): Boolean {

Chroma-Core/src/main/java/buttondevteam/lib/chat/commands/CommandUtils.kt

@@ -1,6 +1,8 @@
 package buttondevteam.lib.chat.commands
 
+import buttondevteam.lib.ChromaUtils
 import buttondevteam.lib.chat.*
+import buttondevteam.lib.test.TestCommandFailedException
 import com.google.common.base.Defaults
 import com.google.common.primitives.Primitives
 import com.mojang.brigadier.builder.ArgumentBuilder
@@ -20,6 +22,14 @@ object CommandUtils {
             .lowercase(Locale.getDefault())
     }
 
+    /**
+     * Sends the given message (with no additional formatting) to the given sender or throws a special exception during testing.
+     */
+    fun reportUserError(sender: Command2Sender, message: String) {
+        if (ChromaUtils.isTest) throw TestCommandFailedException(message)
+        else sender.sendMessage(message)
+    }
+
     /**
      * Performs the given action on the given node and all of its nodes recursively and creates new nodes.
      */

Chroma-Core/src/main/java/buttondevteam/lib/test/TestCommandFailedException.kt

@@ -0,0 +1,6 @@
+package buttondevteam.lib.test
+
+/**
+ * Indicates that the command execution failed for some reason. Note that this doesn't necessarily mean the test failed.
+ */
+class TestCommandFailedException(message: String) : Exception(message)

Chroma-Core/src/main/java/buttondevteam/lib/test/TestException.kt

@@ -0,0 +1,6 @@
+package buttondevteam.lib.test
+
+/**
+ * An exception that occurs just because we're running tests. It's used to test for various error cases.
+ */
+class TestException(message: String, cause: Throwable) : Exception(message, cause)

Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCCommands.kt

@@ -122,6 +122,16 @@ abstract class Command2MCCommands {
     @CommandClass
     object TestEmptyCommand : ICommand2MC()
 
+    @CommandClass
+    object TestParamConverterCommand : ICommand2MC(), ITestCommand2MC {
+        override var testCommandReceived: String? = null
+
+        @Command2.Subcommand
+        fun def(sender: Command2MCSender, something: TestConvertedParameter) {
+            testCommandReceived = something.value
+        }
+    }
+
     interface ITestCommand2MC {
         var testCommandReceived: String?
     }

Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCTest.kt

@@ -57,11 +57,21 @@ class Command2MCTest {
     }
 
     @Test
+    @Order(5)
     fun testHasPermission() {
     }
 
     @Test
+    @Order(4)
     fun testAddParamConverter() {
+        TestParamConverterCommand.register()
+        ButtonPlugin.command2MC.addParamConverter(TestConvertedParameter::class.java, {
+            if (it == "test") null
+            else TestConvertedParameter(it)
+        }, "Failed to convert test param!") { arrayOf("test1", "test2").asIterable() }
+        val sender = createSender()
+        sender.assertCommand("/testparamconverter hmm", TestParamConverterCommand, "hmm")
+        sender.assertCommandUserError("/testparamconverter test", "§cError: §cFailed to convert test param!")
     }
 
     @Test
@@ -74,10 +84,9 @@ class Command2MCTest {
     @Test
     @Order(3)
     fun testHandleCommand() {
-        val user = ChromaGamerBase.getUser(UUID.randomUUID().toString(), TBMCPlayer::class.java)
+        val sender = createSender()
-        user.playerName = "TestPlayer"
+        sender.assertFailingCommand("/missingtest")
-        val sender = TestCommand2MCSender(user)
+        sender.assertFailingCommand("/erroringtest") // Tests completely missing the sender parameter
-        sender.runFailingCommand("/erroringtest") // Tests completely missing the sender parameter
         testTestCommand(sender)
         testNoArgTestCommand(sender)
         testMultiArgTestCommand(sender)
@@ -101,15 +110,21 @@ class Command2MCTest {
         )
     }
 
+    private fun createSender(): TestCommand2MCSender {
+        val user = ChromaGamerBase.getUser(UUID.randomUUID().toString(), TBMCPlayer::class.java)
+        user.playerName = "TestPlayer"
+        return TestCommand2MCSender(user)
+    }
+
     /**
      * Tests parameter conversion, help text and errors.
      */
     private fun testTestCommand(sender: TestCommand2MCSender) {
-        sender.runCommand("/test hmm", TestCommand, "hmm")
+        sender.assertCommand("/test hmm", TestCommand, "hmm")
-        sender.runCommand("/test plugin Chroma-Core", TestCommand, "Chroma-Core")
+        sender.assertCommand("/test plugin Chroma-Core", TestCommand, "Chroma-Core")
-        sender.runCrashingCommand("/test playerfail TestPlayer") { it.cause?.message == "No suitable converter found for class buttondevteam.lib.player.TBMCPlayer param1" }
+        sender.assertCrashingCommand("/test playerfail TestPlayer") { it.cause?.message == "No suitable converter found for class buttondevteam.lib.player.TBMCPlayer param1" }
-        assertEquals("§cError: §cNo Chroma plugin found by that name.", sender.runCommandWithReceive("/test plugin asd"))
+        sender.assertCommandUserError("/test plugin asd", "§cError: §cNo Chroma plugin found by that name.")
-        sender.runCrashingCommand("/test errortest") { it.cause?.cause?.message === "Hmm" }
+        sender.assertCrashingCommand("/test errortest") { it.cause?.cause?.message === "Hmm" }
         assertEquals(
             "Test command\n" +
                 "Used for testing\n" +
@@ -124,29 +139,29 @@ class Command2MCTest {
      * Tests having no arguments for the command and different sender types.
      */
     private fun testNoArgTestCommand(sender: TestCommand2MCSender) {
-        sender.runCommand("/noargtest", NoArgTestCommand, "TestPlayer")
+        sender.assertCommand("/noargtest", NoArgTestCommand, "TestPlayer")
-        sender.runCrashingCommand("/noargtest failing") { it.cause?.cause is IllegalStateException }
+        sender.assertCommandReceiveMessage("/noargtest failing", "") // Help text
-        sender.runCommand("/noargtest sendertest", NoArgTestCommand, "TestPlayer")
+        sender.assertCommand("/noargtest sendertest", NoArgTestCommand, "TestPlayer")
     }
 
     /**
      * Tests parameter type conversion with multiple (optional) parameters.
      */
     private fun testMultiArgTestCommand(sender: TestCommand2MCSender) {
-        sender.runCommand("/multiargtest test hmm mhm", MultiArgTestCommand, "hmmmhm")
+        sender.assertCommand("/multiargtest test hmm mhm", MultiArgTestCommand, "hmmmhm")
-        sender.runCommand("/multiargtest test2 true 19", MultiArgTestCommand, "true 19")
+        sender.assertCommand("/multiargtest test2 true 19", MultiArgTestCommand, "true 19")
-        sender.runCommand("/multiargtest testoptional", MultiArgTestCommand, "false")
+        sender.assertCommand("/multiargtest testoptional", MultiArgTestCommand, "false")
-        sender.runCommand("/multiargtest testoptional true", MultiArgTestCommand, "true")
+        sender.assertCommand("/multiargtest testoptional true", MultiArgTestCommand, "true")
-        sender.runCommand("/multiargtest testoptionalmulti true teszt", MultiArgTestCommand, "true teszt")
+        sender.assertCommand("/multiargtest testoptionalmulti true teszt", MultiArgTestCommand, "true teszt")
-        sender.runCommand("/multiargtest testoptionalmulti true", MultiArgTestCommand, "true null")
+        sender.assertCommand("/multiargtest testoptionalmulti true", MultiArgTestCommand, "true null")
-        sender.runCommand("/multiargtest testoptionalmulti", MultiArgTestCommand, "false null")
+        sender.assertCommand("/multiargtest testoptionalmulti", MultiArgTestCommand, "false null")
     }
 
     /**
      * Tests more type of parameters and wrong param type.
      */
     private fun testTestParamsCommand(sender: TestCommand2MCSender) {
-        sender.runCommand("/testparams 12 34 56 78", TestParamsCommand, "12 34 56.0 78.0 Player0")
+        sender.assertCommand("/testparams 12 34 56 78", TestParamsCommand, "12 34 56.0 78.0 Player0")
         assertEquals("§cExpected integer at position 11: ...estparams <--[HERE]", sender.runCommandWithReceive("/testparams asd 34 56 78"))
         // TODO: Change test when usage help is added
     }
@@ -155,8 +170,8 @@ class Command2MCTest {
      * Tests a command that has no default handler.
      */
     private fun testSomeCommand(sender: TestCommand2MCSender) {
-        sender.runCommand("/some test cmd", TestNoMainCommand1, "TestPlayer")
+        sender.assertCommand("/some test cmd", TestNoMainCommand1, "TestPlayer")
-        sender.runCommand("/some another cmd", TestNoMainCommand2, "TestPlayer")
+        sender.assertCommand("/some another cmd", TestNoMainCommand2, "TestPlayer")
         assertEquals(
             "§6---- Subcommands ----\n" +
                 "/some another cmd\n" +

Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestCommand2MCSender.kt

@@ -4,6 +4,7 @@ import buttondevteam.core.component.channel.Channel
 import buttondevteam.lib.architecture.ButtonPlugin
 import buttondevteam.lib.chat.Command2MCSender
 import buttondevteam.lib.player.TBMCPlayer
+import buttondevteam.lib.test.TestCommandFailedException
 import kotlin.test.assertEquals
 import kotlin.test.assertFails
 
@@ -15,7 +16,7 @@ class TestCommand2MCSender(user: TBMCPlayer) : Command2MCSender(user, Channel.gl
         if (allowMessageReceive) {
             messageReceived += message + "\n"
         } else {
-            error(message)
+            error("Received unexpected message during test: $message")
         }
     }
 
@@ -26,25 +27,62 @@ class TestCommand2MCSender(user: TBMCPlayer) : Command2MCSender(user, Channel.gl
     private fun withMessageReceive(action: () -> Unit): String {
         messageReceived = ""
         allowMessageReceive = true
-        action()
+        val result = kotlin.runCatching { action() }
         allowMessageReceive = false
+        result.getOrThrow()
         return messageReceived.trim()
     }
 
-    fun runCommand(command: String, obj: Command2MCCommands.ITestCommand2MC, expected: String) {
+    private fun runCommand(command: String): Boolean {
-        assert(ButtonPlugin.command2MC.handleCommand(this, command)) { "Could not find command $command" }
+        return ButtonPlugin.command2MC.handleCommand(this, command)
+    }
+
+    private fun assertCommand(command: String) {
+        assert(runCommand(command)) { "Could not find command $command" }
+    }
+
+    /**
+     * Asserts that the command could be found and the command sets the appropriate property to the expected value.
+     */
+    fun assertCommand(command: String, obj: Command2MCCommands.ITestCommand2MC, expected: String) {
+        assertCommand(command)
         assertEquals(expected, obj.testCommandReceived)
     }
 
+    /**
+     * Runs the command and returns any value sent to the sender.
+     */
+    @Deprecated("This isn't an assertion function, use the assertion ones instead.")
     fun runCommandWithReceive(command: String): String {
-        return withMessageReceive { ButtonPlugin.command2MC.handleCommand(this, command) }
+        return withMessageReceive { runCommand(command) }
     }
 
-    fun runFailingCommand(command: String) {
+    /**
-        assert(!ButtonPlugin.command2MC.handleCommand(this, command)) { "Could execute command $command that shouldn't work" }
+     * Tests for when the command either runs successfully and sends back some text or displays help text.
+     */
+    fun assertCommandReceiveMessage(command: String, expectedMessage: String) {
+        assertEquals(expectedMessage, withMessageReceive { assertCommand(command) })
     }
 
-    fun runCrashingCommand(command: String, errorCheck: (Throwable) -> Boolean) {
+    /**
-        assert(errorCheck(assertFails { ButtonPlugin.command2MC.handleCommand(this, command) })) { "Command exception failed test!" }
+     * Tests for when the command cannot be executed at all.
+     */
+    fun assertFailingCommand(command: String) {
+        assert(!runCommand(command)) { "Could execute command $command that shouldn't work" }
+    }
+
+    /**
+     * Tests for when the command execution encounters an exception. This includes test exceptions as well.
+     */
+    fun assertCrashingCommand(command: String, errorCheck: (Throwable) -> Boolean) {
+        val ex = assertFails { runCommand(command) }
+        assert(errorCheck(ex)) { "Command exception failed test! Exception: ${ex.stackTraceToString()}" }
+    }
+
+    /**
+     * Tests for expected user errors. Anything that results in the command system (and not the command itself) sending anything to the user.
+     */
+    fun assertCommandUserError(command: String, message: String) {
+        assertCrashingCommand(command) { ex -> ex.cause?.let { it is TestCommandFailedException && it.message == message } ?: false }
     }
 }

Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedParameter.kt

@@ -0,0 +1,3 @@
+package buttondevteam.lib.chat.test
+
+class TestConvertedParameter(val value: String)

Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedUser.kt

@@ -0,0 +1,3 @@
+package buttondevteam.lib.chat.test
+
+class TestConvertedUser(val name: String)