From 702655cf9f2b29ea120d752db3ecc9c3e6c85fa8 Mon Sep 17 00:00:00 2001 From: NorbiPeti Date: Mon, 4 Mar 2024 02:20:35 +0100 Subject: [PATCH] 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) --- .../java/buttondevteam/lib/ChromaUtils.kt | 3 +- .../java/buttondevteam/lib/chat/Command2.kt | 8 ++- .../java/buttondevteam/lib/chat/Command2MC.kt | 7 +-- .../lib/chat/commands/CommandUtils.kt | 10 ++++ .../lib/test/TestCommandFailedException.kt | 6 ++ .../buttondevteam/lib/test/TestException.kt | 6 ++ .../lib/chat/test/Command2MCCommands.kt | 10 ++++ .../lib/chat/test/Command2MCTest.kt | 59 ++++++++++++------- .../lib/chat/test/TestCommand2MCSender.kt | 56 +++++++++++++++--- .../lib/chat/test/TestConvertedParameter.kt | 3 + .../lib/chat/test/TestConvertedUser.kt | 3 + 11 files changed, 131 insertions(+), 40 deletions(-) create mode 100644 Chroma-Core/src/main/java/buttondevteam/lib/test/TestCommandFailedException.kt create mode 100644 Chroma-Core/src/main/java/buttondevteam/lib/test/TestException.kt create mode 100644 Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedParameter.kt create mode 100644 Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedUser.kt diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/ChromaUtils.kt b/Chroma-Core/src/main/java/buttondevteam/lib/ChromaUtils.kt index ec51c9f..534de1b 100644 --- a/Chroma-Core/src/main/java/buttondevteam/lib/ChromaUtils.kt +++ b/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) diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2.kt b/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2.kt index a58727b..cc32e15 100644 --- a/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2.kt +++ b/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2.kt @@ -101,6 +101,8 @@ abstract class Command2, 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, 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, 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, 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) diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2MC.kt b/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2MC.kt index 6f4791d..3851242 100644 --- a/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2MC.kt +++ b/Chroma-Core/src/main/java/buttondevteam/lib/chat/Command2MC.kt @@ -137,14 +137,11 @@ class Command2MC : Command2('/', 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 -> - Optional.ofNullable(node.data.command).map { obj: ICommand2MC -> obj.plugin } - .map { comp: ButtonPlugin -> component.javaClass.simpleName == comp.javaClass.simpleName }.orElse(false) - }, true) + unregisterCommandIf({ node -> node.data.command.component == component }, true) } override fun handleCommand(sender: Command2MCSender, commandline: String): Boolean { diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/chat/commands/CommandUtils.kt b/Chroma-Core/src/main/java/buttondevteam/lib/chat/commands/CommandUtils.kt index 1deaf47..feab944 100644 --- a/Chroma-Core/src/main/java/buttondevteam/lib/chat/commands/CommandUtils.kt +++ b/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. */ diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/test/TestCommandFailedException.kt b/Chroma-Core/src/main/java/buttondevteam/lib/test/TestCommandFailedException.kt new file mode 100644 index 0000000..e654bfc --- /dev/null +++ b/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) diff --git a/Chroma-Core/src/main/java/buttondevteam/lib/test/TestException.kt b/Chroma-Core/src/main/java/buttondevteam/lib/test/TestException.kt new file mode 100644 index 0000000..aae6966 --- /dev/null +++ b/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) diff --git a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCCommands.kt b/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCCommands.kt index eef9842..d2cc558 100644 --- a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCCommands.kt +++ b/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? } diff --git a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCTest.kt b/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCTest.kt index d631e6d..5e5f922 100644 --- a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/Command2MCTest.kt +++ b/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) - user.playerName = "TestPlayer" - val sender = TestCommand2MCSender(user) - sender.runFailingCommand("/erroringtest") // Tests completely missing the sender parameter + val sender = createSender() + sender.assertFailingCommand("/missingtest") + sender.assertFailingCommand("/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.runCommand("/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" } - assertEquals("§cError: §cNo Chroma plugin found by that name.", sender.runCommandWithReceive("/test plugin asd")) - sender.runCrashingCommand("/test errortest") { it.cause?.cause?.message === "Hmm" } + sender.assertCommand("/test hmm", TestCommand, "hmm") + sender.assertCommand("/test plugin Chroma-Core", TestCommand, "Chroma-Core") + sender.assertCrashingCommand("/test playerfail TestPlayer") { it.cause?.message == "No suitable converter found for class buttondevteam.lib.player.TBMCPlayer param1" } + sender.assertCommandUserError("/test plugin asd", "§cError: §cNo Chroma plugin found by that name.") + 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.runCrashingCommand("/noargtest failing") { it.cause?.cause is IllegalStateException } - sender.runCommand("/noargtest sendertest", NoArgTestCommand, "TestPlayer") + sender.assertCommand("/noargtest", NoArgTestCommand, "TestPlayer") + sender.assertCommandReceiveMessage("/noargtest failing", "") // Help text + 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.runCommand("/multiargtest test2 true 19", MultiArgTestCommand, "true 19") - sender.runCommand("/multiargtest testoptional", MultiArgTestCommand, "false") - sender.runCommand("/multiargtest testoptional true", MultiArgTestCommand, "true") - sender.runCommand("/multiargtest testoptionalmulti true teszt", MultiArgTestCommand, "true teszt") - sender.runCommand("/multiargtest testoptionalmulti true", MultiArgTestCommand, "true null") - sender.runCommand("/multiargtest testoptionalmulti", MultiArgTestCommand, "false null") + sender.assertCommand("/multiargtest test hmm mhm", MultiArgTestCommand, "hmmmhm") + sender.assertCommand("/multiargtest test2 true 19", MultiArgTestCommand, "true 19") + sender.assertCommand("/multiargtest testoptional", MultiArgTestCommand, "false") + sender.assertCommand("/multiargtest testoptional true", MultiArgTestCommand, "true") + sender.assertCommand("/multiargtest testoptionalmulti true teszt", MultiArgTestCommand, "true teszt") + sender.assertCommand("/multiargtest testoptionalmulti true", MultiArgTestCommand, "true 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.runCommand("/some another cmd", TestNoMainCommand2, "TestPlayer") + sender.assertCommand("/some test cmd", TestNoMainCommand1, "TestPlayer") + sender.assertCommand("/some another cmd", TestNoMainCommand2, "TestPlayer") assertEquals( "§6---- Subcommands ----\n" + "/some another cmd\n" + diff --git a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestCommand2MCSender.kt b/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestCommand2MCSender.kt index 663d3b9..4dc93b8 100644 --- a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestCommand2MCSender.kt +++ b/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) { - assert(ButtonPlugin.command2MC.handleCommand(this, command)) { "Could not find command $command" } + private fun runCommand(command: String): Boolean { + 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 } } } \ No newline at end of file diff --git a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedParameter.kt b/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedParameter.kt new file mode 100644 index 0000000..bbe3aa5 --- /dev/null +++ b/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) diff --git a/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedUser.kt b/Chroma-Core/src/test/kotlin/buttondevteam/lib/chat/test/TestConvertedUser.kt new file mode 100644 index 0000000..71a4f9d --- /dev/null +++ b/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)