Mercurial > hg > release > thermostat-1.4

changeset 1782:5345e379a7a6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Exempt help invocations from setup hook. Reviewed-by: aazores Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-September/015911.html PR2615
author Severin Gehwolf <sgehwolf@redhat.com>
date Wed, 09 Sep 2015 12:50:27 +0200
parents 3227fa662692
children 1de39fd67cbc
files distribution/tools/verify-bash-completion.sh launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java
diffstat 3 files changed, 54 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/distribution/tools/verify-bash-completion.sh	Thu Sep 10 11:51:45 2015 +0200
+++ b/distribution/tools/verify-bash-completion.sh	Wed Sep 09 12:50:27 2015 +0200
@@ -63,12 +63,8 @@
 
 function __init {
     USER_THERMOSTAT_HOME="$TARGET/bash-completion-test-user-home"
-    echo "Skipping initial thermostat setup..."
-    mkdir -p $USER_THERMOSTAT_HOME/data
     echo "Exporting custom home..."
     export USER_THERMOSTAT_HOME
-    touch ${USER_THERMOSTAT_HOME}/data/setup-complete.stamp
-
     echo "Checking that help works..."
     ${TARGET}/image/bin/thermostat help >/dev/null
 
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java	Thu Sep 10 11:51:45 2015 +0200
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java	Wed Sep 09 12:50:27 2015 +0200
@@ -41,7 +41,9 @@
 import java.io.PrintStream;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -87,11 +89,18 @@
  */
 public class LauncherImpl implements Launcher {
 
+    private static final Set<String> HELP_SET;
     private static final String HELP_COMMAND_NAME = "help";
+    private static final String HELP_OPTION = "--help";
 
     private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
     private static final Logger logger = LoggingUtils.getLogger(LauncherImpl.class);
 
+    static {
+        HELP_SET = new HashSet<>();
+        HELP_SET.add(HELP_COMMAND_NAME);
+        HELP_SET.add(HELP_OPTION);
+    }
     private final AtomicInteger usageCount = new AtomicInteger(0);
     private final BundleContext context;
     private final BundleManager registry;
@@ -156,7 +165,7 @@
                 cmdCtxFactory.getConsole().getOutput().println(coreVersion.getVersionInfo());
             } else {
                 // With web-always-on we need to make sure that the setup ran.
-                if (isThermostatConfigured()) {
+                if (isSomeHelpInvocation(args) || isThermostatConfigured()) {
                     logger.log(Level.FINE, "Running command without setup interception.");
                     runCommandFromArguments(args, listeners, inShell);
                 } else {
@@ -186,6 +195,15 @@
         }
     }
     
+    private boolean isSomeHelpInvocation(String[] args) {
+        for (String arg: args) {
+            if (HELP_SET.contains(arg)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private void runSetupThenInterceptedCommand(String[] originalCmdArgs) {
         String origCmdArgs = convertOriginalArgsToString(originalCmdArgs);
         String[] setupArgs = { "setup",
@@ -272,7 +290,7 @@
 
     private void runCommand(String cmdName, String[] cmdArgs, Collection<ActionListener<ApplicationState>> listeners, boolean inShell) {
         // treat 'foo --help' as 'help foo'
-        if (!cmdName.equals(HELP_COMMAND_NAME) && Arrays.asList(cmdArgs).contains("--help")) {
+        if (!cmdName.equals(HELP_COMMAND_NAME) && Arrays.asList(cmdArgs).contains(HELP_OPTION)) {
             runCommand(HELP_COMMAND_NAME, new String[] { cmdName } , listeners, inShell);
             return;
         }
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java	Thu Sep 10 11:51:45 2015 +0200
+++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java	Wed Sep 09 12:50:27 2015 +0200
@@ -36,6 +36,7 @@
 
 package com.redhat.thermostat.launcher.internal;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -48,11 +49,9 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.concurrent.ExecutorService;
 import java.util.logging.Handler;
 import java.util.logging.Level;
@@ -61,8 +60,6 @@
 
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
-import org.hamcrest.BaseMatcher;
-import org.hamcrest.Description;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -154,6 +151,7 @@
     private CurrentEnvironment environment;
     private CommonPaths paths;
 
+    @SuppressWarnings("unchecked")
     @Before
     public void setUp() throws CommandInfoNotFoundException, BundleException, IOException {
         setupCommandContextFactory();
@@ -611,6 +609,7 @@
 
     @Test
     public void verifyListenersAdded() {
+        @SuppressWarnings("unchecked")
         ActionListener<ApplicationState> listener = mock(ActionListener.class);
         Collection<ActionListener<ApplicationState>> listeners = new ArrayList<>();
         listeners.add(listener);
@@ -692,7 +691,7 @@
     @Test
     public void verifyOriginalCmdArgsArePassedOnToSetup() {
         String[] argsList = new String[] { "list-vms", "--dbUrl=foo" };
-        List<Pair<String[], Boolean>> resultList = doOriginalCmdArgsArePassedOnToSetupTest(argsList);
+        List<Pair<String[], Boolean>> resultList = runAsUnconfiguredThermostat(argsList);
         assertEquals("Expected to run only setup", 1, resultList.size());
         Pair<String[], Boolean> actual = resultList.get(0);
         assertFalse("Expected to run outside shell", actual.getSecond());
@@ -703,7 +702,7 @@
     @Test
     public void verifyOriginalCmdArgsArePassedOnToSetup2() {
         String[] argsList = new String[] { "web-storage-service" };
-        List<Pair<String[], Boolean>> resultList = doOriginalCmdArgsArePassedOnToSetupTest(argsList);
+        List<Pair<String[], Boolean>> resultList = runAsUnconfiguredThermostat(argsList);
         assertEquals("Expected to run only setup", 1, resultList.size());
         Pair<String[], Boolean> actual = resultList.get(0);
         assertFalse("Expected to run outside shell", actual.getSecond());
@@ -711,7 +710,35 @@
         assertArrayEquals(expectedList, actual.getFirst());
     }
     
-    private List<Pair<String[], Boolean>> doOriginalCmdArgsArePassedOnToSetupTest(String[] args) {
+    /*
+     * Bash completion uses help which expects help to always run
+     * (no setup required).
+     */
+    @Test
+    public void verifyHelpIsNotRunThroughSetupHook() {
+        String[] argsList = new String[] { "help" };
+        
+        List<Pair<String[], Boolean>> resultList = runAsUnconfiguredThermostat(argsList);
+        assertEquals("Expected to run only help", 1, resultList.size());
+        Pair<String[], Boolean> actual = resultList.get(0);
+        assertFalse("Expected to run outside shell", actual.getSecond());
+        String[] expectedList = new String[] { "help" };
+        assertArrayEquals(expectedList, actual.getFirst());
+    }
+    
+    @Test
+    public void verifyCommandHelpOptionIsNotRunThroughSetupHook() {
+        String[] argsList = new String[] { "web-storage-service", "--help" };
+        
+        List<Pair<String[], Boolean>> resultList = runAsUnconfiguredThermostat(argsList);
+        assertEquals("Expected to run only web-storage-service --help", 1, resultList.size());
+        Pair<String[], Boolean> actual = resultList.get(0);
+        assertFalse("Expected to run outside shell", actual.getSecond());
+        String[] expectedList = new String[] { "web-storage-service", "--help" };
+        assertArrayEquals(expectedList, actual.getFirst());
+    }
+    
+    private List<Pair<String[], Boolean>> runAsUnconfiguredThermostat(String[] args) {
         CommonPaths setupPaths = mock(CommonPaths.class);
         File mockFile = mock(File.class);
         when(mockFile.exists()).thenReturn(false);
@@ -736,13 +763,6 @@
         return runList;
     }
     
-    private void assertArrayEquals(String[] expected, String[] actual) {
-        assertTrue(expected.length == actual.length);
-        for (int i = 0; i < expected.length; i++) {
-            assertEquals(expected[i], actual[i]);
-        }
-    }
-    
     private static class TestLogHandler extends Handler {
         
         private boolean loggedThermostatHome;