changeset 1853:d2cc25649e4f

Set correct exit status when thermostat setup succeeds or fails PR 2904 Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-March/018339.html Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-April/018413.html
author Anirudhan Mukundan <amukunda@redhat.com>
date Tue, 05 Apr 2016 14:58:50 -0400
parents 6a68586b8b95
children 5f61f05dff9e
files setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/Activator.java setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/SetupCommand.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/SetupCommandTest.java
diffstat 3 files changed, 155 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/Activator.java	Tue Apr 05 14:56:48 2016 -0400
+++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/Activator.java	Tue Apr 05 14:58:50 2016 -0400
@@ -38,6 +38,7 @@
 
 import java.util.Map;
 
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.MultipleServiceTracker;
 import com.redhat.thermostat.common.cli.CommandRegistry;
 import com.redhat.thermostat.common.cli.CommandRegistryImpl;
@@ -62,6 +63,7 @@
         reg.registerCommand("setup", setupCommand);
 
         Class<?>[] deps = new Class<?>[] {
+                ExitStatus.class,
                 CommonPaths.class,
                 Launcher.class,
                 Keyring.class
@@ -69,9 +71,11 @@
         tracker = new MultipleServiceTracker(context, deps, new MultipleServiceTracker.Action() {
             @Override
             public void dependenciesAvailable(Map<String, Object> services) {
+                ExitStatus exitStatus = (ExitStatus) services.get(ExitStatus.class.getName());
                 CommonPaths paths = (CommonPaths) services.get(CommonPaths.class.getName());
                 Launcher launcher = (Launcher) services.get(Launcher.class.getName());
                 Keyring keyring = (Keyring) services.get(Keyring.class.getName());
+                setupCommand.setExitStatusService(exitStatus);
                 setupCommand.setPaths(paths);
                 setupCommand.setLauncher(launcher);
                 setupCommand.setKeyring(keyring);
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/SetupCommand.java	Tue Apr 05 14:56:48 2016 -0400
+++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/SetupCommand.java	Tue Apr 05 14:58:50 2016 -0400
@@ -47,6 +47,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.cli.AbstractCommand;
 import com.redhat.thermostat.common.cli.Arguments;
 import com.redhat.thermostat.common.cli.CommandContext;
@@ -87,22 +88,29 @@
                 args = mergeOriginalArgs(optionArgs, args);
             }
         }
-        
-        
-        this.paths = dependentServices.getService(CommonPaths.class);
-        requireNonNull(paths, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "CommonPaths"));
-        this.launcher = dependentServices.getService(Launcher.class);
-        requireNonNull(launcher, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "Launcher"));
-        this.keyring = dependentServices.getService(Keyring.class);
-        requireNonNull(keyring, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "Keyring"));
-        ThermostatSetup setup = createSetup();
+
+        ExitStatus exitStatus = dependentServices.getService(ExitStatus.class);
+        requireNonNull(exitStatus, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "ExitStatus"));
 
-        if (args.hasArgument(NON_GUI_OPTION_NAME) || isHeadless()) {
-            runCLISetup(setup, ctx.getConsole());
-        } else {
-            runGUISetup(setup);
+        try {
+            this.paths = dependentServices.getService(CommonPaths.class);
+            requireNonNull(paths, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "CommonPaths"));
+            this.launcher = dependentServices.getService(Launcher.class);
+            requireNonNull(launcher, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "Launcher"));
+            this.keyring = dependentServices.getService(Keyring.class);
+            requireNonNull(keyring, t.localize(LocaleResources.SERVICE_UNAVAILABLE_MESSAGE, "Keyring"));
+            ThermostatSetup setup = createSetup();
+
+            if (args.hasArgument(NON_GUI_OPTION_NAME) || isHeadless()) {
+                runCLISetup(setup, ctx.getConsole());
+            } else {
+                runGUISetup(setup);
+            }
+        } catch (CommandException e) {
+            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
+            throw e;
         }
-        
+        exitStatus.setExitStatus(ExitStatus.EXIT_SUCCESS);
         runOriginalCommand(origArgsList);
     }
 
@@ -116,7 +124,8 @@
         return new MergedSetupArguments(args, origArgsList);
     }
 
-    private void runCLISetup(ThermostatSetup setup, Console console) throws CommandException {
+    // package-private for testing
+    void runCLISetup(ThermostatSetup setup, Console console) throws CommandException {
         CLISetup cliSetup = new CLISetup(setup, console);
         cliSetup.run();
     }
@@ -149,6 +158,10 @@
         return args[0].equals("setup");
     }
 
+    public void setExitStatusService(ExitStatus exitStatus) {
+        dependentServices.addService(ExitStatus.class, exitStatus);
+    }
+
     public void setPaths(CommonPaths paths) {
         dependentServices.addService(CommonPaths.class, paths);
     }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/SetupCommandTest.java	Tue Apr 05 14:56:48 2016 -0400
+++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/SetupCommandTest.java	Tue Apr 05 14:58:50 2016 -0400
@@ -56,6 +56,8 @@
 import java.util.List;
 import java.util.Objects;
 
+import com.redhat.thermostat.common.ExitStatus;
+import com.redhat.thermostat.shared.locale.LocalizedString;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.junit.Before;
@@ -84,6 +86,7 @@
     private CommonPaths paths;
     private Launcher launcher;
     private Keyring keyring;
+    private ExitStatus exitStatus;
     private ThermostatSetup thermostatSetup;
 
     @Before
@@ -101,6 +104,7 @@
         error = new PrintStream(errorBaos);
         launcher = mock(Launcher.class);
         keyring = mock(Keyring.class);
+        exitStatus = mock(ExitStatus.class);
         thermostatSetup = mock(ThermostatSetup.class);
 
         when(ctxt.getArguments()).thenReturn(mockArgs);
@@ -157,6 +161,7 @@
     public void testPathsNotSetFailure() {
         cmd = createSetupCommand();
 
+        cmd.setExitStatusService(mock(ExitStatus.class));
         try {
             cmd.run(ctxt);
             fail();
@@ -168,7 +173,8 @@
     @Test
     public void testLauncherNotSetFailure() {
         cmd = createSetupCommand();
-        
+
+        cmd.setExitStatusService(mock(ExitStatus.class));
         cmd.setPaths(mock(CommonPaths.class));
         try {
             cmd.run(ctxt);
@@ -179,6 +185,119 @@
     }
     
     @Test
+    public void testExitStatusNotSetFailure() {
+        cmd = createSetupCommand();
+
+        try {
+            cmd.run(ctxt);
+            fail();
+        } catch (CommandException e) {
+            assertTrue(e.getMessage().contains("ExitStatus service dependency unavailable"));
+        }
+    }
+
+    @Test
+    public void testExitStatusNonZeroWhenGUISetupFails() {
+        cmd = new SetupCommand() {
+            @Override
+            void setLookAndFeel() throws InvocationTargetException, InterruptedException {
+                // do nothing
+            }
+
+            @Override
+            void createMainWindowAndRun(ThermostatSetup setup) throws CommandException {
+                throw new CommandException(new LocalizedString("Setup fail exception"));
+            }
+
+            @Override
+            ThermostatSetup createSetup() {
+                return thermostatSetup;
+            }
+        };
+
+        Arguments args = mock(Arguments.class);
+
+        testExitStatus(args, ExitStatus.EXIT_ERROR);
+    }
+
+    @Test
+    public void testExitZeroWhenGUISetupSuccess() {
+        cmd = new SetupCommand() {
+            @Override
+            void setLookAndFeel() throws InvocationTargetException, InterruptedException {
+                // do nothing
+            }
+
+            @Override
+            void createMainWindowAndRun(ThermostatSetup setup) throws CommandException {
+            }
+
+            @Override
+            ThermostatSetup createSetup() {
+                return thermostatSetup;
+            }
+        };
+
+        Arguments args = mock(Arguments.class);
+
+        testExitStatus(args, ExitStatus.EXIT_SUCCESS);
+    }
+
+    @Test
+    public void testExitStatusNonZeroWhenCLISetupFails() {
+        cmd = new SetupCommand() {
+            @Override
+            void runCLISetup(ThermostatSetup setup, Console console) throws CommandException {
+                throw new CommandException(new LocalizedString("Setup fail exception"));
+            }
+
+            @Override
+            ThermostatSetup createSetup() {
+                return thermostatSetup;
+            }
+        };
+
+        Arguments args = mock(Arguments.class);
+        when(args.hasArgument("nonGui")).thenReturn(true);
+
+        testExitStatus(args, ExitStatus.EXIT_ERROR);
+    }
+
+    @Test
+    public void testExitStatusZeroWhenCLISetupSuccess() {
+        cmd = new SetupCommand() {
+            @Override
+            void runCLISetup(ThermostatSetup setup, Console console) throws CommandException {
+                // do nothing
+            }
+
+            @Override
+            ThermostatSetup createSetup() {
+                return thermostatSetup;
+            }
+        };
+
+        Arguments args = mock(Arguments.class);
+        when(args.hasArgument("nonGui")).thenReturn(true);
+
+        testExitStatus(args, ExitStatus.EXIT_SUCCESS);
+    }
+
+    private void testExitStatus(Arguments setupArgs, int exitVal) {
+        setServices();
+
+        CommandContext ctxt = mock(CommandContext.class);
+        when(ctxt.getArguments()).thenReturn(setupArgs);
+
+        try {
+            cmd.run(ctxt);
+        } catch (CommandException e) {
+            // ignore
+        }
+        verify(exitStatus).setExitStatus(eq(exitVal));
+    }
+
+    @Test
     public void verifyOriginalCommandRunsAfterSetup() throws CommandException {
         doTestOriginalCmdRunsAfterSetup("web-storage-service", new String[] {
            "web-storage-service"     
@@ -281,7 +400,8 @@
     @Test
     public void testKeyringNotSetFailure() {
         cmd = createSetupCommand();
-        
+
+        cmd.setExitStatusService(mock(ExitStatus.class));
         cmd.setPaths(mock(CommonPaths.class));
         cmd.setLauncher(mock(Launcher.class));
         try {
@@ -339,6 +459,7 @@
         cmd.setPaths(paths);
         cmd.setLauncher(launcher);
         cmd.setKeyring(keyring);
+        cmd.setExitStatusService(exitStatus);
     }
     
     private static class ArgsMatcher extends BaseMatcher<String[]> {