From 0ccf01fa0c17d1e40defef636c7f1499f286a10d Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 26 Jul 2012 11:42:39 +0200 Subject: [PATCH] SONAR-3698 Improve documentation, code coverage and add a salt of Guava --- .../org/sonar/api/utils/command/Command.java | 40 +++++++++++-------- .../sonar/api/utils/command/CommandTest.java | 14 +++++-- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/Command.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/Command.java index 801df4bb9e3..84216d0d0e3 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/Command.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/Command.java @@ -20,13 +20,15 @@ package org.sonar.api.utils.command; import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.SystemUtils; import java.io.File; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -35,14 +37,14 @@ import java.util.Map; * @since 2.7 */ public class Command { - - private String executable; - private List arguments = Lists.newArrayList(); + private final String executable; + private final List arguments = Lists.newArrayList(); + private final Map env = Maps.newHashMap(System.getenv()); private File directory; - private Map env = Maps.newHashMap(System.getenv()); private boolean newShell = false; private Command(String executable) { + Preconditions.checkArgument(!StringUtils.isBlank(executable), "Command executable can not be blank"); this.executable = executable; } @@ -51,7 +53,7 @@ public class Command { } public List getArguments() { - return Collections.unmodifiableList(arguments); + return ImmutableList.copyOf(arguments); } public Command addArgument(String arg) { @@ -65,7 +67,7 @@ public class Command { } public Command addArguments(String[] args) { - arguments.addAll(Arrays.asList(args)); + Collections.addAll(arguments, args); return this; } @@ -98,10 +100,13 @@ public class Command { * @since 3.2 */ public Map getEnvironmentVariables() { - return Collections.unmodifiableMap(env); + return ImmutableMap.copyOf(env); } /** + * true if a new shell should be used to execute the command. + * The default behavior is to not use a new shell. + * * @since 3.3 */ public boolean isNewShell() { @@ -109,7 +114,12 @@ public class Command { } /** - * Set to true if the script to execute has not enough rights (+x on unix). + * Set to true if a new shell should be used to execute the command. + * This is useful when the executed command is a script with no execution rights (+x on unix). + * + * On windows, the command will be executed with cmd /C executable. + * On other platforms, the command will be executed with sh executable. + * * @since 3.3 */ public Command setNewShell(boolean b) { @@ -117,19 +127,18 @@ public class Command { return this; } - String[] toStrings() { - List command = Lists.newArrayList(); + List toStrings() { + ImmutableList.Builder command = ImmutableList.builder(); if (newShell) { if (SystemUtils.IS_OS_WINDOWS) { - command.add("cmd"); - command.add("/C"); + command.add("cmd", "/C"); } else { command.add("sh"); } } command.add(executable); command.addAll(arguments); - return command.toArray(new String[command.size()]); + return command.build(); } public String toCommandLine() { @@ -147,9 +156,6 @@ public class Command { * @param executable */ public static Command create(String executable) { - if (StringUtils.isBlank(executable)) { - throw new IllegalArgumentException("Command executable can not be blank"); - } return new Command(executable); } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandTest.java index b57a671ee0b..2ae9c89b4f8 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandTest.java @@ -29,7 +29,6 @@ import java.util.Arrays; import static org.fest.assertions.Assertions.assertThat; - public class CommandTest { @Rule @@ -52,7 +51,7 @@ public class CommandTest { Command command = Command.create("java"); command.addArgument("-Xmx512m"); command.addArguments(Arrays.asList("-a", "-b")); - command.addArguments(new String[]{"-x", "-y"}); + command.addArguments(new String[] {"-x", "-y"}); assertThat(command.getExecutable()).isEqualTo("java"); assertThat(command.getArguments()).hasSize(5); assertThat(command.toCommandLine()).isEqualTo("java -Xmx512m -a -b -x -y"); @@ -89,15 +88,24 @@ public class CommandTest { } @Test - public void use_new_shell() { + public void should_use_new_shell() { if (SystemUtils.IS_OS_WINDOWS) { Command command = Command.create("foo.bat"); command.setNewShell(true); assertThat(command.toCommandLine()).isEqualTo("cmd /C foo.bat"); + assertThat(command.isNewShell()).isTrue(); } else { Command command = Command.create("foo.sh"); command.setNewShell(true); assertThat(command.toCommandLine()).isEqualTo("sh foo.sh"); + assertThat(command.isNewShell()).isTrue(); } } + + @Test + public void shouldnt_use_new_shell_by_default() { + Command command = Command.create("foo.sh"); + + assertThat(command.isNewShell()).isFalse(); + } } -- 2.39.5