Skip to content

Commit

Permalink
Avoid a warning for tests in interactive mode
Browse files Browse the repository at this point in the history
The CliHelpTest#testCommandHelp has become extremely flaky lately
because for one run, a warning saying JBang is not installable is there
and for the other it's not.

This is problematic and I really don't see the value of adding a warning
about JBang that is specifically only displayed for tests.

Also fixed a bunch of typos and made the message more neutral (let's
avoid using exclamation marks for messages and exceptions).
  • Loading branch information
gsmet committed Dec 18, 2023
1 parent 4470678 commit a00e16c
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 35 deletions.
2 changes: 1 addition & 1 deletion devtools/cli/src/main/java/io/quarkus/cli/QuarkusCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ private Supplier<QuarkusProject> quarkusProject(Optional<String> testDir) {

private PluginManager pluginManager(OutputOptionMixin output, Optional<String> testDir, boolean interactiveMode) {
PluginManagerSettings settings = PluginManagerSettings.defaultSettings()
.withInteractivetMode(interactiveMode); // Why not just getting it from output.isClieTest ? Cause args have not been parsed yet.
.withInteractiveMode(interactiveMode); // Why not just getting it from output.isCliTest ? Cause args have not been parsed yet.
return PluginManager.create(settings, output, Optional.ofNullable(Paths.get(System.getProperty("user.home"))),
getProjectRoot(testDir), quarkusProject(testDir));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ Integer addPlugin() throws IOException {
output.info(table.getContent());
if (plugin.isInProjectCatalog() && existingPlugin.filter(p -> p.isInUserCatalog()).isPresent()) {
output.warn(
"Plugin was added in the project scope, but another with the same name exists in the user scope!\nThe project scoped one will take precedence when invoked from within the project!");
"Plugin was added in the project scope, but another with the same name exists in the user scope.\nThe project scoped one will take precedence when invoked from within the project.");
}

if (plugin.isInUserCatalog() && existingPlugin.filter(p -> p.isInProjectCatalog()).isPresent()) {
output.warn(
"Plugin was added in the user scope, but another with the same name exists in the project scope!\nThe project scoped one will take precedence when invoked from within the project!");
"Plugin was added in the user scope, but another with the same name exists in the project scope.\nThe project scoped one will take precedence when invoked from within the project.");
}

return CommandLine.ExitCode.OK;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Integer listPluigns() {
.collect(Collectors.toMap(p -> p.getName(), p -> p)));

if (items.isEmpty()) {
output.info("No plugins " + (installable ? "installable" : "installed") + "!");
output.info("No plugins " + (installable ? "installable" : "installed") + ".");
} else {
PluginListTable table = new PluginListTable(
items.values().stream().filter(this::filter).collect(Collectors.toList()), showCommand);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ Integer removePlugin() throws IOException {
if (pluginManager.getInstalledPlugins().containsKey(plugin.getName())) {
if (plugin.isInProjectCatalog()) {
output.warn(
"The removed plugin was available both in user and project scopes. It was removed from the project but will remain available in the user scope!");
"The removed plugin was available both in user and project scopes. It was removed from the project scope but will remain available in the user scope.");
} else {
output.warn(
"The removed plugin was available both in user and project scopes. It was removed from the user but will remain available in the project scope!");
"The removed plugin was available both in user and project scopes. It was removed from the user scope but will remain available in the project scope.");
}
}
return CommandLine.ExitCode.OK;
}).orElseGet(() -> {
output.error("Plugin: " + name + " not found in catalog!");
output.error("Plugin: " + name + " not found in catalog.");
return CommandLine.ExitCode.USAGE;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public Integer call() throws Exception {
if (jbang.ensureJBangIsInstalled()) {
return PluginCommand.super.call();
} else {
output.error("Unable to find JBang! Command execution aborted!");
output.error("Unable to find JBang. Command execution aborted as it requires JBang.");
return ExitCode.SOFTWARE;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private Optional<PluginCommand> createPluginCommand(Plugin plugin) {
case executable:
return plugin.getLocation().map(l -> new ShellCommand(plugin.getName(), Paths.get(l), output));
default:
throw new IllegalStateException("Unknown plugin type!");
throw new IllegalStateException("Unknown plugin type");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public JBangCatalog readCatalog(Path path) {
if (!jbang.isAvailable() && !jbang.isInstallable()) {
// When jbang is not available / installable just return an empty catalog.
// We don't even return the parsed one as plugins won't be able to run without jbang anyway.
return new JBangCatalog();
return JBangCatalog.empty();
}

JBangCatalog localCatalog = super.readCatalog(path);
Expand All @@ -76,7 +76,7 @@ public JBangCatalog readCombinedCatalog(Optional<Path> projectDir, Optional<Path
if (!jbang.isAvailable() && !jbang.isInstallable()) {
// When jbang is not available / installable just return an empty catalog.
// We don't even return the parsed one as plugins won't be able to run without jbang anyway.
return new JBangCatalog();
return JBangCatalog.empty();
}

Map<String, JBangCatalog> catalogs = new HashMap<>();
Expand Down Expand Up @@ -146,7 +146,7 @@ private Map<String, JBangAlias> listAliasesOrFallback(JBangSupport jbang, String

//If there are locally installed catalogs, then go through every single one of them
//and collect the aliases.
//Unfortunaltely jbang can't return all alias in one go.
//Unfortunately jbang can't return all alias in one go.
//This is because it currently omits `@catalog` suffix in some cases.
if (!localCatalogs.isEmpty()) {
Map<String, JBangAlias> aliases = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,18 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.function.Predicate;

import io.quarkus.devtools.exec.Executable;
import io.quarkus.devtools.messagewriter.MessageWriter;
import io.quarkus.devtools.utils.Prompt;
import io.quarkus.fs.util.ZipUtils;
import io.smallrye.common.os.OS;

public class JBangSupport {

private static final boolean IS_OS_WINDOWS = System.getProperty("os.name").toLowerCase(Locale.ENGLISH).contains("windows");
private static final String JBANG_EXECUTABLE = IS_OS_WINDOWS ? "jbang.cmd" : "jbang";
private static final String JBANG_EXECUTABLE = OS.WINDOWS.isCurrent() ? "jbang.cmd" : "jbang";

private static final Predicate<Path> EXISTS_AND_WRITABLE = p -> p != null && p.toFile().exists() && p.toFile().canWrite();

Expand Down Expand Up @@ -92,7 +91,7 @@ public Optional<File> getOptionalExecutable(boolean shouldEnsureInstallation) {
}

public File getExecutable() {
return getOptionalExecutable().orElseThrow(() -> new IllegalStateException("Unable to find and install jbang!"));
return getOptionalExecutable().orElseThrow(() -> new IllegalStateException("Unable to find or install JBang"));
}

public Path getWorkingDirectory() {
Expand Down Expand Up @@ -158,7 +157,7 @@ public boolean isInstallable() {
public boolean promptForInstallation() {
// We don't want to prompt users for input when running tests.
if (interactiveMode
&& Prompt.yesOrNo(true, "JBang is needed to list / run jbang plugins, would you like to install it now ?")) {
&& Prompt.yesOrNo(true, "JBang is needed to list/run JBang plugins, would you like to install it now?")) {
return true;
}
return false;
Expand All @@ -180,15 +179,15 @@ private boolean doEnsureJBangIsInstalledInternal() {
return true;
}
if (!isInstallable()) {
output.warn("JBang is not installable!");
// this only happens for tests, do not output anything specific
return false;
}
if (promptForInstallation()) {
try {
installJBang();
return true;
} catch (Exception e) {
output.warn("Failed to install jbang!");
output.warn("Failed to install JBang", e);
return false;
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class PluginManager {
private static PluginManager INSTANCE;

private final MessageWriter output;
private final PluginMangerState state;
private final PluginManagerState state;
private final PluginManagerSettings settings;
private final PluginManagerUtil util;

Expand All @@ -41,7 +41,7 @@ public synchronized static PluginManager create(PluginManagerSettings settings,
this.settings = settings;
this.output = output;
this.util = PluginManagerUtil.getUtil(settings);
this.state = new PluginMangerState(settings, output, userHome, currentDir, quarkusProject);
this.state = new PluginManagerState(settings, output, userHome, currentDir, quarkusProject);
}

/**
Expand Down Expand Up @@ -159,7 +159,7 @@ public Optional<Plugin> removePlugin(String name) {
*/
public Optional<Plugin> removePlugin(String name, boolean userCatalog) {
PluginCatalogService pluginCatalogService = state.getPluginCatalogService();
Plugin plugin = state.getInstalledPluigns().get(name);
Plugin plugin = state.getInstalledPlugins().get(name);
if (plugin == null) {
return Optional.empty();
} else if (userCatalog) {
Expand Down Expand Up @@ -333,7 +333,7 @@ public boolean syncIfNeeded() {
}

public Map<String, Plugin> getInstalledPlugins(boolean userCatalog) {
return userCatalog ? state.userPlugins() : state.getInstalledPluigns();
return userCatalog ? state.userPlugins() : state.getInstalledPlugins();
}

public Map<String, Plugin> getInstalledPlugins() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public PluginManagerSettings withCatalogs(String... remoteJBangCatalogs) {
toRelativePath);
}

public PluginManagerSettings withInteractivetMode(boolean interactiveMode) {
public PluginManagerSettings withInteractiveMode(boolean interactiveMode) {
return new PluginManagerSettings(interactiveMode, pluginPrefix, fallbackJBangCatalog, remoteJBangCatalogs,
toRelativePath);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@
import io.quarkus.maven.dependency.ArtifactKey;
import io.quarkus.platform.catalog.processor.ExtensionProcessor;

class PluginMangerState {
class PluginManagerState {

PluginMangerState(PluginManagerSettings settings, MessageWriter output, Optional<Path> userHome, Optional<Path> currentDir,
PluginManagerState(PluginManagerSettings settings, MessageWriter output, Optional<Path> userHome, Optional<Path> currentDir,
Supplier<QuarkusProject> quarkusProject) {
this.settings = settings;
this.output = output;
this.userHome = userHome;
this.quarkusProject = quarkusProject;
Expand All @@ -35,7 +34,6 @@ class PluginMangerState {
this.util = PluginManagerUtil.getUtil(settings);
}

private final PluginManagerSettings settings;
private final MessageWriter output;
private final PluginManagerUtil util;
private final Optional<Path> userHome;
Expand Down Expand Up @@ -73,7 +71,7 @@ public Map<String, Plugin> installedPlugins() {
return allInstalledPlugins;
}

public Map<String, Plugin> getInstalledPluigns() {
public Map<String, Plugin> getInstalledPlugins() {
if (_installedPlugins == null) {
_installedPlugins = installedPlugins();
}
Expand All @@ -86,7 +84,7 @@ public Map<String, Plugin> projectPlugins() {
.collect(Collectors.toMap(p -> p.getName(), p -> p))).orElse(Collections.emptyMap());
}

public Map<String, Plugin> getProjectPluigns() {
public Map<String, Plugin> getProjectPlugins() {
if (_projectPlugins == null) {
_projectPlugins = projectPlugins();
}
Expand All @@ -99,7 +97,7 @@ public Map<String, Plugin> userPlugins() {
.collect(Collectors.toMap(p -> p.getName(), p -> p))).orElse(Collections.emptyMap());
}

public Map<String, Plugin> getUserPluigns() {
public Map<String, Plugin> getUserPlugins() {
if (_userPlugins == null) {
_userPlugins = userPlugins();
}
Expand Down Expand Up @@ -228,7 +226,7 @@ public PluginCatalog getCombinedCatalog() {

public PluginCatalog pluginCatalog(boolean userCatalog) {
return (userCatalog ? getUserCatalog() : getProjectCatalog()).or(() -> getUserCatalog())
.orElseThrow(() -> new IllegalStateException("Unable to get project and user plugin catalogs!"));
.orElseThrow(() -> new IllegalStateException("Unable to get project and user plugin catalogs"));
}

public Optional<Path> getProjectRoot() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ public String getName(Optional<GACTV> gactv, Optional<URL> url, Optional<Path> p
.or(() -> path.map(Path::getFileName).map(Path::toString)
.map(s -> s.replaceAll("\\.jar$", "").replaceAll("\\.java$", "")))
.map(n -> stripCliSuffix(n))
.map(n -> n.replaceAll("^" + prefix + "\\-cli\\-", prefix + "")) // stip cli prefix (after the quarkus bit)
.map(n -> n.replaceAll("^" + prefix + "\\-", "")) // stip quarkus prefix (after the quarkus bit)
.map(n -> n.replaceAll("@.*$", "")) // stip the @sufix
.map(n -> n.replaceAll("^" + prefix + "\\-cli\\-", prefix + "")) // strip cli prefix (after the quarkus bit)
.map(n -> n.replaceAll("^" + prefix + "\\-", "")) // strip quarkus prefix (after the quarkus bit)
.map(n -> n.replaceAll("@.*$", "")) // strip the @suffix
.orElseThrow(() -> new IllegalStateException("Could not determinate name for location."));
}

Expand Down

0 comments on commit a00e16c

Please sign in to comment.