Mercurial > hg > release > thermostat-1.4
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[]> {