From ba34e4729b5a0394309f25aba1f7e3e896fc77b2 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Wed, 9 Jun 2021 22:22:15 +0200 Subject: [PATCH] Add more suggestion tests --- .../proxy/command/SuggestionsProvider.java | 23 ++- .../proxy/command/VelocityCommandManager.java | 14 +- .../proxy/command/BrigadierCommandTests.java | 85 +++++++++++ .../proxy/command/CommandManagerTests.java | 1 - .../proxy/command/RawCommandTests.java | 109 ++++++++++++++ .../proxy/command/SimpleCommandTests.java | 107 ++++++++++++++ .../command/SuggestionsProviderTests.java | 137 ++++++++++-------- 7 files changed, 407 insertions(+), 69 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 259a1ff24..2de0503d4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -121,16 +121,15 @@ final class SuggestionsProvider { } /** - * Returns whether a literal node with the given name should be considered for + * Returns whether a literal node with the given lowercase name should be considered for * suggestions given the specified input. * - * @param name the literal name + * @param name the lowercase literal name * @param input the partial input * @return true if the literal should be considered; false otherwise */ private static boolean shouldConsider(final String name, final String input) { - // TODO (perf) If we expect input to be lowercase, no need to ignore case - return name.regionMatches(true, 0, input, 0, input.length()); + return name.regionMatches(false, 0, input, 0, input.length()); } /** @@ -143,7 +142,9 @@ final class SuggestionsProvider { private CompletableFuture provideAliasSuggestions( final StringReader reader, final CommandContextBuilder contextSoFar) { final S source = contextSoFar.getSource(); - final String input = reader.getRead(); + // Lowercase the alias here so all comparisons can be case-sensitive (cheaper) + // TODO Is this actually faster? + final String input = reader.getRead().toLowerCase(Locale.ENGLISH); final Collection> aliases = contextSoFar.getRootNode().getChildren(); @SuppressWarnings("unchecked") @@ -188,7 +189,13 @@ final class SuggestionsProvider { // This is a BrigadierCommand, fallback to regular suggestions reader.setCursor(0); final ParseResults parse = this.dispatcher.parse(reader, source); - return this.dispatcher.getCompletionSuggestions(parse); + try { + return this.dispatcher.getCompletionSuggestions(parse); + } catch (final Throwable e) { + // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. + LOGGER.error("Command node cannot provide suggestions for " + fullInput, e); + return Suggestions.empty(); + } } if (!argsNode.canUse(source)) { @@ -243,7 +250,7 @@ final class SuggestionsProvider { try { return node.listSuggestions(built, new SuggestionsBuilder(fullInput, start)); } catch (final Throwable e) { - // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. + // Again, plugins are naughty LOGGER.error("Arguments node cannot provide suggestions", e); return Suggestions.empty(); } @@ -266,7 +273,7 @@ final class SuggestionsProvider { try { return this.dispatcher.getCompletionSuggestions(parse); } catch (final Throwable e) { - // Again, plugins are naughty. + // Yet again, plugins are naughty. LOGGER.error("Hint node cannot provide suggestions", e); return Suggestions.empty(); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java index 6332c51f1..04fe0cda8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -25,6 +25,7 @@ import com.mojang.brigadier.ParseResults; import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.suggestion.Suggestion; import com.mojang.brigadier.tree.RootCommandNode; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandManager; @@ -150,8 +151,9 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(cmdLine, "cmdLine"); final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, true); - final ParseResults parse = this.parse(normalizedInput, source); try { + // The parse can fail if the requirement predicates throw + final ParseResults parse = this.parse(normalizedInput, source); return dispatcher.execute(parse) != BrigadierCommand.FORWARD; } catch (final CommandSyntaxException e) { boolean isSyntaxError = !e.getType().equals( @@ -207,8 +209,14 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(cmdLine, "cmdLine"); final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, false); - return suggestionsProvider.provideSuggestions(normalizedInput, source) - .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + try { + return suggestionsProvider.provideSuggestions(normalizedInput, source) + .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + } catch (final Throwable e) { + // Again, plugins are naughty + return CompletableFutures.exceptionallyCompletedFuture( + new RuntimeException("Unable to provide suggestions for " + cmdLine + " for " + source, e)); + } } /** diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index 7b14acd12..3038ce766 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -4,12 +4,16 @@ import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger; import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; import static com.mojang.brigadier.arguments.StringArgumentType.word; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.CommandSource; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -121,6 +125,41 @@ public class BrigadierCommandTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testExecuteAsyncCompletesExceptionallyOnCallbackException() { + final var expected = new RuntimeException(); + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> { + throw expected; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + final Exception wrapper = assertThrows(CompletionException.class, () -> + manager.executeAsync(source, "hello").join()); + + assertSame(expected, wrapper.getCause().getCause()); + } + + @Test + void testExecuteAsyncCompletesExceptionallyOnRequirementException() { + final var expected = new RuntimeException(); + final var node = LiteralArgumentBuilder + .literal("hello") + .requires(source1 -> { + throw expected; + }) + .executes(context -> fail()) // needed for dispatcher to consider the node + .build(); + manager.register(new BrigadierCommand(node)); + + final Exception wrapper = assertThrows(CompletionException.class, () -> + manager.executeAsync(source, "hello").join()); + + assertSame(expected, wrapper.getCause().getCause()); + } + // Suggestions @Test @@ -182,4 +221,50 @@ public class BrigadierCommandTests extends CommandTestSuite { assertSuggestions("parent child"); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfCustomSuggestionProviderFutureCompletesExceptionally() { + final var node = LiteralArgumentBuilder + .literal("parent") + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent "); + } + + @Test + void testDoesNotSuggestIfCustomSuggestionProviderThrows() { + final var node = LiteralArgumentBuilder + .literal("parent") + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + })) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent "); + } + + @Test + void testSuggestCompletesExceptionallyIfRequirementPredicateThrows() { + final var node = LiteralArgumentBuilder + .literal("parent") + .requires(source1 -> { + throw new RuntimeException(); + }) + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> fail())) + .build(); + manager.register(new BrigadierCommand(node)); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "parent ").join()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java index d6dbbbeb2..8bd9236f9 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -129,7 +129,6 @@ public class CommandManagerTests { // Un-registration - @Test void testUnregisterUnregisteredAliasIsIgnored() { manager.unregister("hello"); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java index ca8f3634b..3f8ff1bfe 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -1,12 +1,18 @@ package com.velocitypowered.proxy.command; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -96,6 +102,29 @@ public class RawCommandTests extends CommandTestSuite { // Suggestions + @Test + void testDoesNotSuggestAliasIfImpermissible() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -116,6 +145,25 @@ public class RawCommandTests extends CommandTestSuite { assertSuggestions("hello ", "people", "world"); // in alphabetical order } + @Test + void testSuggestsArgumentsAfterAliasIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsArgumentsAfterPartialArguments() { final var meta = manager.metaBuilder("numbers").build(); @@ -192,4 +240,65 @@ public class RawCommandTests extends CommandTestSuite { assertSuggestions("foo bar baz "); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestIfSuggestAsyncThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestCompletesExceptionallyIfHasPermissionThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + throw new RuntimeException(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return fail(); + } + }); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java index 3cead1f69..e4ccec57d 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -2,12 +2,16 @@ package com.velocitypowered.proxy.command; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -97,6 +101,29 @@ public class SimpleCommandTests extends CommandTestSuite { // Suggestions + @Test + void testDoesNotSuggestAliasIfImpermissible() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -117,6 +144,25 @@ public class SimpleCommandTests extends CommandTestSuite { assertSuggestions("hello ", "people", "world"); // in alphabetical order } + @Test + void testSuggestsArgumentsAfterAliasIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsArgumentsAfterPartialArguments() { final var meta = manager.metaBuilder("numbers").build(); @@ -193,4 +239,65 @@ public class SimpleCommandTests extends CommandTestSuite { assertSuggestions("foo bar baz "); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestIfSuggestAsyncThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestCompletesExceptionallyIfHasPermissionThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + throw new RuntimeException(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return fail(); + } + }); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 66fed8234..dd61069bc 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -18,18 +18,22 @@ package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.StringArgumentType.word; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.spotify.futures.CompletableFutures; +import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; import com.velocitypowered.api.command.SimpleCommand; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import org.junit.jupiter.api.Test; /** @@ -39,13 +43,12 @@ import org.junit.jupiter.api.Test; public class SuggestionsProviderTests extends CommandTestSuite { @Test - void testSuggestsAliasForEmptyInput() { - final var meta = manager.metaBuilder("foo") - .aliases("bar", "baz") - .build(); - manager.register(meta, NoSuggestionsCommand.INSTANCE); + void testSuggestsAliasesForEmptyInput() { + manager.register(manager.metaBuilder("foo").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("bar").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("baz").build(), NoSuggestionsCommand.INSTANCE); - assertSuggestions("", "bar", "baz", "foo"); + assertSuggestions("", "bar", "baz", "foo"); // in alphabetical order } @Test @@ -58,14 +61,20 @@ public class SuggestionsProviderTests extends CommandTestSuite { @Test void testSuggestsAliasesForPartialAlias() { - final var meta = manager.metaBuilder("foo") - .aliases("bar", "baz") - .build(); + manager.register(manager.metaBuilder("hello").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("hey").build(), NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hell", "hello"); + assertSuggestions("He", "hello", "hey"); + } + + @Test + void testDoesNotSuggestForFullAlias() { + final var meta = manager.metaBuilder("hello").build(); manager.register(meta, NoSuggestionsCommand.INSTANCE); - assertSuggestions("ba", "bar", "baz"); - assertSuggestions("fo", "foo"); - assertSuggestions("bar"); + assertSuggestions("hello"); + assertSuggestions("Hello"); } @Test @@ -78,9 +87,9 @@ public class SuggestionsProviderTests extends CommandTestSuite { } @Test - void testDoesNotSuggestArgumentsForPartialAlias() { + void testDoesNotSuggestArgumentsForIncorrectAlias() { final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new SimpleCommand() { + manager.register(meta, new RawCommand() { @Override public void execute(final Invocation invocation) { fail(); @@ -95,10 +104,26 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("hell "); } + // Secondary aliases + // The following tests check for inconsistencies between the primary alias node and + // a secondary alias literal. + @Test - void testSuggestsArgumentsIgnoresAliasCase() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new SimpleCommand() { + void testSuggestsAllAliases() { + final var meta = manager.metaBuilder("foo") + .aliases("bar", "baz") + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("", "bar", "baz", "foo"); + } + + @Test + void testSuggestsArgumentsViaAlias() { + final var meta = manager.metaBuilder("hello") + .aliases("hi") + .build(); + manager.register(meta, new RawCommand() { @Override public void execute(final Invocation invocation) { fail(); @@ -110,9 +135,11 @@ public class SuggestionsProviderTests extends CommandTestSuite { } }); - assertSuggestions("Hello ", "world"); + assertSuggestions("hi ", "world"); } + // Hinting + @Test void testSuggestsHintLiteral() { final var hint = LiteralArgumentBuilder @@ -169,49 +196,26 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("foo ", "bar", "baz", "qux"); assertSuggestions("foo bar", "baz", "qux"); - } - - // Exception handling - - @Test - void testSkipsSuggestionWhenNodeSuggestionProviderFutureCompletesExceptionally() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } - - @Override - public CompletableFuture> suggestAsync(final Invocation invocation) { - return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); - } - }); - - assertSuggestions("hello "); + assertSuggestions("foo baz", "bar", "qux"); } @Test - void testSkipsSuggestionWhenNodeSuggestionProviderThrows() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } + // This doesn't make much sense, but emulates Brigadier behavior + void testSuggestsImpermissibleHint() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .requires(source1 -> false) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); - @Override - public CompletableFuture> suggestAsync(final Invocation invocation) { - throw new RuntimeException(); - } - }); - - // Also logs an error to the console, but testing this is quite involved - assertSuggestions("hello "); + assertSuggestions("hello ", "hint"); } @Test - void testSkipsSuggestionWhenHintSuggestionProviderFutureCompletesExceptionally() { + void testDoesNotSuggestHintIfHintSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder .argument("hint", word()) .suggests((context, builder) -> @@ -226,7 +230,7 @@ public class SuggestionsProviderTests extends CommandTestSuite { } @Test - void testSkipsSuggestionWhenHintSuggestionProviderThrows() { + void testDoesNotSuggestHintIfCustomSuggestionProviderThrows() { final var hint = RequiredArgumentBuilder .argument("hint", word()) .suggests((context, builder) -> { @@ -238,10 +242,29 @@ public class SuggestionsProviderTests extends CommandTestSuite { .build(); manager.register(meta, NoSuggestionsCommand.INSTANCE); - // Also logs an error to the console, but testing this is quite involved assertSuggestions("hello "); } + @Test + void testSuggestCompletesExceptionallyIfHintRequirementPredicateThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .requires(source1 -> { + throw new RuntimeException(); + }) + .suggests((context, builder) -> fail()) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } + + /* + @Test void testSuggestionMergingIgnoresExceptionallyCompletedSuggestionFutures() { final var hint = RequiredArgumentBuilder @@ -265,7 +288,7 @@ public class SuggestionsProviderTests extends CommandTestSuite { }); assertSuggestions("hello ", "world"); - } + }*/ static final class NoSuggestionsCommand implements RawCommand {