changeset 1049:3f0416e9db80

Don't use System.exit anywhere other than in Laucher. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-March/006029.html
author Severin Gehwolf <sgehwolf@redhat.com>
date Fri, 08 Mar 2013 14:28:35 +0100
parents edaf77f17cbd
children 2dc4ebc08a77
files agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java common/core/src/main/java/com/redhat/thermostat/common/Constants.java common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/ExitStatusImplTest.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java
diffstat 14 files changed, 285 insertions(+), 112 deletions(-) [+]
line wrap: on
line diff
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java	Fri Mar 08 14:28:35 2013 +0100
@@ -40,8 +40,11 @@
 
 import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceReference;
+import org.osgi.util.tracker.ServiceTracker;
 
 import com.redhat.thermostat.agent.cli.impl.db.StorageCommand;
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.cli.CommandRegistry;
 import com.redhat.thermostat.common.cli.CommandRegistryImpl;
 
@@ -49,19 +52,33 @@
 
     private CommandRegistry reg;
     private AgentApplication agentApplication;
+    @SuppressWarnings("rawtypes")
+    private ServiceTracker exitStatusTracker;
 
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     @Override
     public void start(final BundleContext context) throws Exception {
-        reg = new CommandRegistryImpl(context);
-        agentApplication = new AgentApplication(context);
-        reg.registerCommands(Arrays.asList(new ServiceCommand(),
-                new StorageCommand(), agentApplication));
+        exitStatusTracker = new ServiceTracker(context, ExitStatus.class, null) {
+            
+            @Override
+            public Object addingService(ServiceReference reference) {
+                ExitStatus exitStatus = (ExitStatus)context.getService(reference);
+                reg = new CommandRegistryImpl(context);
+                agentApplication = new AgentApplication(context, exitStatus);
+                reg.registerCommands(Arrays.asList(new ServiceCommand(context),
+                        new StorageCommand(exitStatus), agentApplication));
+                return exitStatus;
+            }
+            
+        };
+        exitStatusTracker.open();
     }
 
     @Override
     public void stop(BundleContext context) throws Exception {
         agentApplication.shutdown();
         reg.unregisterCommands();
+        exitStatusTracker.close();
     }
 }
 
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java	Fri Mar 08 14:28:35 2013 +0100
@@ -55,7 +55,7 @@
 import com.redhat.thermostat.agent.config.AgentStartupConfiguration;
 import com.redhat.thermostat.backend.BackendRegistry;
 import com.redhat.thermostat.backend.BackendService;
-import com.redhat.thermostat.common.Constants;
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.LaunchException;
 import com.redhat.thermostat.common.MultipleServiceTracker;
 import com.redhat.thermostat.common.MultipleServiceTracker.Action;
@@ -84,19 +84,22 @@
     private AgentStartupConfiguration configuration;
     private AgentOptionParser parser;
     private DbServiceFactory dbServiceFactory;
+    @SuppressWarnings("rawtypes")
     private ServiceTracker configServerTracker;
     private MultipleServiceTracker daoTracker;
+    private final ExitStatus exitStatus;
 
     private CountDownLatch shutdownLatch;
 
-    public AgentApplication(BundleContext bundleContext) {
-        this(bundleContext, new ConfigurationCreator(), new DbServiceFactory());
+    public AgentApplication(BundleContext bundleContext, ExitStatus exitStatus) {
+        this(bundleContext, exitStatus, new ConfigurationCreator(), new DbServiceFactory());
     }
 
-    AgentApplication(BundleContext bundleContext, ConfigurationCreator configurationCreator, DbServiceFactory dbServiceFactory) {
+    AgentApplication(BundleContext bundleContext, ExitStatus exitStatus, ConfigurationCreator configurationCreator, DbServiceFactory dbServiceFactory) {
         this.bundleContext = bundleContext;
         this.configurationCreator = configurationCreator;
         this.dbServiceFactory = dbServiceFactory;
+        this.exitStatus = exitStatus;
     }
     
     private void parseArguments(Arguments args) throws InvalidConfigurationException {
@@ -104,6 +107,7 @@
         parser.parse();
     }
     
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     private void runAgent(CommandContext ctx) {
         long startTime = System.currentTimeMillis();
         configuration.setStartTime(startTime);
@@ -237,7 +241,7 @@
         
     }
 
-    private Agent startAgent(final Logger logger, final Storage storage,
+    Agent startAgent(final Logger logger, final Storage storage,
             AgentInfoDAO agentInfoDAO, BackendInfoDAO backendInfoDAO) {
         BackendRegistry backendRegistry = null;
         try {
@@ -245,7 +249,11 @@
             
         } catch (Exception e) {
             logger.log(Level.SEVERE, "Could not get BackendRegistry instance.", e);
-            System.exit(Constants.EXIT_ERROR);
+            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
+            shutdown();
+            // Since this would throw NPE's down the line if we continue in this
+            // method, let's fail right and early :)
+            throw new RuntimeException(e);
         }
 
         final Agent agent = new Agent(backendRegistry, configuration, storage, agentInfoDAO, backendInfoDAO);
@@ -259,7 +267,8 @@
             logger.log(Level.SEVERE,
                     "Agent could not start, probably because a configured backend could not be activated.",
                     le);
-            System.exit(Constants.EXIT_ERROR);
+            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
+            shutdown();
         }
         logger.fine("Agent started.");
 
@@ -267,7 +276,7 @@
         logger.info("agent started.");
         return agent;
     }
-
+    
     private void handleConnected(final ConfigurationServer configServer,
             final Logger logger, final CountDownLatch shutdownLatch) {
         Class<?>[] deps = new Class<?>[] {
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java	Fri Mar 08 14:28:35 2013 +0100
@@ -71,10 +71,6 @@
     private BundleContext context;
     private Launcher launcher;
 
-    public ServiceCommand() {
-        this(FrameworkUtil.getBundle(ServiceCommand.class).getBundleContext());
-    }
-
     public ServiceCommand(BundleContext context) {
         this.context = context;
         listeners = new ArrayList<>();
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java	Fri Mar 08 14:28:35 2013 +0100
@@ -39,11 +39,6 @@
 import java.io.File;
 import java.io.IOException;
 
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.ServiceReference;
-
-import com.redhat.thermostat.common.Constants;
 import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand;
 import com.redhat.thermostat.common.cli.Arguments;
@@ -60,17 +55,12 @@
 
     private DBStartupConfiguration configuration;
     private DBOptionParser parser;
-    private BundleContext context;
+    private final ExitStatus exitStatus;
     
     private MongoProcessRunner runner;
     
-    public StorageCommand() {
-        this(FrameworkUtil.getBundle(StorageCommand.class).getBundleContext());
-    }
-    
-    // For testing
-    StorageCommand(BundleContext context) {
-        this.context = context;
+    public StorageCommand(ExitStatus exitStatus) {
+        this.exitStatus = exitStatus;
     }
     
     private void parseArguments(Arguments args) throws InvalidConfigurationException {
@@ -134,7 +124,7 @@
         } catch (ApplicationException | InvalidConfigurationException | IOException e) {
             // something went wrong set status appropriately. This makes sure
             // that the JVM exits with this status.
-            setExitStatus(Constants.EXIT_ERROR);
+            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
             // rethrow
             throw e;
         }
@@ -149,7 +139,7 @@
         } catch (ApplicationException | InvalidConfigurationException | InterruptedException | IOException e) {
             // something went wrong set status appropriately. This makes sure
             // that the JVM exits with this status.
-            setExitStatus(Constants.EXIT_ERROR);
+            exitStatus.setExitStatus(ExitStatus.EXIT_ERROR);
             throw e;
         }
         getNotifier().fireAction(ApplicationState.STOP);
@@ -167,14 +157,6 @@
             throw new InvalidConfigurationException("database directories do not exist...");
         }
     }
-    
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    private void setExitStatus(final int newStatus) {
-        ServiceReference exitStatusRef = context.getServiceReference(ExitStatus.class);
-        ExitStatus status = (ExitStatus) context.getService(exitStatusRef);
-        status.setExitStatus(newStatus);
-        context.ungetService(exitStatusRef);
-    }
 
     public DBStartupConfiguration getConfiguration() {
         return configuration;
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java	Fri Mar 08 14:28:35 2013 +0100
@@ -43,29 +43,19 @@
 import static org.mockito.Mockito.mock;
 
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.osgi.framework.Bundle;
-import org.osgi.framework.FrameworkUtil;
-import org.powermock.api.mockito.PowerMockito;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.redhat.thermostat.agent.cli.impl.db.StorageCommand;
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.cli.Command;
 import com.redhat.thermostat.testutils.StubBundleContext;
 
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({FrameworkUtil.class})
 public class ActivatorTest {
 
     @Test
-    public void verifyActivatorRegistersCommands() throws Exception {
-
-        PowerMockito.mockStatic(FrameworkUtil.class);
-        Bundle mockBundle = mock(Bundle.class);
-        when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(mockBundle);
-        
+    public void verifyActivatorRegistersCommands() throws Exception {        
         StubBundleContext bundleContext = new StubBundleContext();
+        ExitStatus exitStatus = mock(ExitStatus.class);
+        bundleContext.registerService(ExitStatus.class, exitStatus, null);
         
         Activator activator = new Activator();
 
@@ -77,7 +67,7 @@
 
         activator.stop(bundleContext);
 
-        assertEquals(0, bundleContext.getAllServices().size());
+        assertEquals(1, bundleContext.getAllServices().size());
     }
 }
 
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java	Fri Mar 08 14:28:35 2013 +0100
@@ -39,26 +39,39 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Logger;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.InvalidSyntaxException;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
-import com.redhat.thermostat.agent.cli.impl.AgentApplication;
+import com.redhat.thermostat.agent.Agent;
 import com.redhat.thermostat.agent.cli.impl.AgentApplication.ConfigurationCreator;
 import com.redhat.thermostat.agent.command.ConfigurationServer;
 import com.redhat.thermostat.agent.config.AgentStartupConfiguration;
+import com.redhat.thermostat.backend.BackendRegistry;
+import com.redhat.thermostat.common.ExitStatus;
+import com.redhat.thermostat.common.LaunchException;
 import com.redhat.thermostat.common.cli.Arguments;
 import com.redhat.thermostat.common.cli.CommandContext;
 import com.redhat.thermostat.common.cli.CommandException;
@@ -72,15 +85,19 @@
 import com.redhat.thermostat.storage.dao.BackendInfoDAO;
 import com.redhat.thermostat.testutils.StubBundleContext;
 
+@PrepareForTest({ AgentApplication.class })
+@RunWith(PowerMockRunner.class)
 public class AgentApplicationTest {
 
     // TODO: Test i18nized versions when they come.
 
     private StubBundleContext context;
 
-    private AgentApplication agent;
     private ConfigurationServer configServer;
     private DbService dbService;
+    private ConfigurationCreator configCreator;
+    private ExitStatus exitStatus;
+    private DbServiceFactory dbServiceFactory;
     
     @Before
     public void setUp() throws InvalidConfigurationException {
@@ -90,7 +107,7 @@
         AgentStartupConfiguration config = mock(AgentStartupConfiguration.class);
         when(config.getDBConnectionString()).thenReturn("test string; please ignore");
 
-        ConfigurationCreator configCreator = mock(ConfigurationCreator.class);
+        configCreator = mock(ConfigurationCreator.class);
         when(configCreator.create()).thenReturn(config);
 
         Storage storage = mock(Storage.class);
@@ -101,33 +118,96 @@
         context.registerService(BackendInfoDAO.class.getName(), backendInfoDAO, null);
         configServer = mock(ConfigurationServer.class);
         context.registerService(ConfigurationServer.class.getName(), configServer, null);
-        DbServiceFactory dbServiceFactory = mock(DbServiceFactory.class);
+        dbServiceFactory = mock(DbServiceFactory.class);
         dbService = mock(DbService.class);
         when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService);
 
-        agent = new AgentApplication(context, configCreator, dbServiceFactory);
+        exitStatus = mock(ExitStatus.class);
     }
 
     @After
     public void tearDown() {
-        agent = null;
+        context = null;
+        configServer = null;
+        dbService = null;
+        configCreator = null;
+        dbServiceFactory = null;
+        exitStatus = null;
     }
 
     @Test
     public void testName() {
+        AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory);
         String name = agent.getName();
         assertEquals("agent", name);
     }
 
     @Test
     public void testDescAndUsage() {
+        AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory);
         assertNotNull(agent.getDescription());
         assertNotNull(agent.getUsage());
     }
 
     @Test
     public void testAgentStartup() throws CommandException, InterruptedException {
+        final AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory);
+        final CountDownLatch latch = new CountDownLatch(1);
+        final CommandException[] ce = new CommandException[1];
         final long timeoutMillis = 5000L;
+        
+        startAgentRunThread(timeoutMillis, agent, ce, latch);
+        
+        boolean ret = latch.await(timeoutMillis, TimeUnit.MILLISECONDS);
+        if (ce[0] != null) {
+            throw ce[0];
+        }
+        if (!ret) {
+            fail("Timeout expired!");
+        }
+        
+    }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void verifyBackendRegistryProblemsSetsExitStatus() throws Exception {
+        whenNew(BackendRegistry.class).withParameterTypes(BundleContext.class)
+                .withArguments(any(BundleContext.class))
+                .thenThrow(InvalidSyntaxException.class);
+        final AgentApplication agent = new AgentApplication(context,
+                exitStatus, configCreator, dbServiceFactory);
+        try {
+            agent.startAgent(mock(Logger.class), null, null, null);
+        } catch (RuntimeException e) {
+            assertEquals(InvalidSyntaxException.class, e.getCause().getClass());
+        }
+        verify(exitStatus).setExitStatus(ExitStatus.EXIT_ERROR);
+    }
+    
+    @Test
+    public void verifyAgentLaunchExceptionSetsExitStatus() throws Exception {
+        whenNew(BackendRegistry.class).withParameterTypes(BundleContext.class)
+                .withArguments(any(BundleContext.class))
+                .thenReturn(mock(BackendRegistry.class));
+        Agent mockAgent = mock(Agent.class);
+        whenNew(Agent.class).withParameterTypes(BackendRegistry.class,
+                AgentStartupConfiguration.class, Storage.class,
+                AgentInfoDAO.class, BackendInfoDAO.class).withArguments(
+                any(BackendRegistry.class),
+                any(AgentStartupConfiguration.class), any(Storage.class),
+                any(AgentInfoDAO.class), any(BackendInfoDAO.class)).thenReturn(mockAgent);
+        doThrow(LaunchException.class).when(mockAgent).start();
+        final AgentApplication agent = new AgentApplication(context,
+                exitStatus, configCreator, dbServiceFactory);
+        try {
+            agent.startAgent(mock(Logger.class), null, null, null);
+        } catch (RuntimeException e) {
+            fail("Should not have thrown RuntimeException");
+        }
+        verify(exitStatus).setExitStatus(ExitStatus.EXIT_ERROR);
+    }
+
+    private void startAgentRunThread(final long timoutMillis, final AgentApplication agent, final CommandException[] ce, final CountDownLatch latch) {
         Arguments args = mock(Arguments.class);
         final CommandContext commandContext = mock(CommandContext.class);
         when(commandContext.getArguments()).thenReturn(args);
@@ -147,9 +227,6 @@
             
         }).when(dbService).connect();
 
-        final CountDownLatch latch = new CountDownLatch(1);
-        
-        final CommandException[] ce = new CommandException[1];
         // Run agent in a new thread so we can timeout on failure
         Thread t = new Thread(new Runnable() {
             
@@ -174,14 +251,6 @@
         });
         
         t.start();
-        boolean ret = latch.await(timeoutMillis, TimeUnit.MILLISECONDS);
-        if (ce[0] != null) {
-            throw ce[0];
-        }
-        if (!ret) {
-            fail("Timeout expired!");
-        }
-        
     }
 
 }
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java	Fri Mar 08 14:28:35 2013 +0100
@@ -55,12 +55,9 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceReference;
 
 import com.redhat.thermostat.common.ActionEvent;
 import com.redhat.thermostat.common.ActionListener;
-import com.redhat.thermostat.common.Constants;
 import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.cli.CommandContext;
 import com.redhat.thermostat.common.cli.CommandException;
@@ -68,7 +65,6 @@
 import com.redhat.thermostat.common.config.InvalidConfigurationException;
 import com.redhat.thermostat.common.tools.ApplicationException;
 import com.redhat.thermostat.common.tools.ApplicationState;
-import com.redhat.thermostat.testutils.StubBundleContext;
 
 public class StorageCommandTest {
     
@@ -77,11 +73,11 @@
     private static final String DB = "storage/db";
 
     private String tmpDir;
-    private BundleContext context;
+    private ExitStatus exitStatus;
     
     @Before
     public void setup() {
-        context = mock(BundleContext.class);
+        exitStatus = mock(ExitStatus.class);
         // need to create a dummy config file for the test
         try {
             Random random = new Random();
@@ -113,7 +109,7 @@
     
     @After
     public void tearDown() {
-        context = null;
+        exitStatus = null;
     }
     
     @Test
@@ -125,7 +121,7 @@
         CommandContext ctx = mock(CommandContext.class);
         when(ctx.getArguments()).thenReturn(args);
 
-        StorageCommand service = new StorageCommand(context) {
+        StorageCommand service = new StorageCommand(exitStatus) {
             @Override
             MongoProcessRunner createRunner() {
                 throw new AssertionError("dry run should never create an actual runner");
@@ -152,7 +148,7 @@
         // TODO: stop not tested yet, but be sure it's not called from the code
         doThrow(new ApplicationException("mock exception")).when(runner).stopService();
         
-        StorageCommand service = new StorageCommand(context) {
+        StorageCommand service = new StorageCommand(exitStatus) {
             @Override
             MongoProcessRunner createRunner() {
                 return runner;
@@ -233,8 +229,7 @@
     
     @Test
     public void exceptionSetsExitStatusOnFailure() throws Exception {
-        context = new StubBundleContext();
-        ExitStatus exitStatusImpl = new ExitStatus() {
+        this.exitStatus = new ExitStatus() {
             
             private int exitStatus = -1;
             
@@ -248,8 +243,7 @@
                 return exitStatus;
             }
         };
-        context.registerService(ExitStatus.class, exitStatusImpl, null);
-        assertEquals(-1, getExitStatusService(context).getExitStatus());
+        assertEquals(-1, this.exitStatus.getExitStatus());
         StorageCommand command = prepareService(false);
         final CountDownLatch latch = new CountDownLatch(1);
         final boolean[] result = new boolean[1];
@@ -273,13 +267,12 @@
         latch.await();
         // should have failed
         assertTrue(result[0]);
-        assertEquals(Constants.EXIT_ERROR, getExitStatusService(context).getExitStatus());
+        assertEquals(ExitStatus.EXIT_ERROR, this.exitStatus.getExitStatus());
     }
     
     @Test
     public void exitStatusRemainsUntouchedOnSuccess() throws Exception {
-        context = new StubBundleContext();
-        ExitStatus exitStatusImpl = new ExitStatus() {
+        this.exitStatus = new ExitStatus() {
             
             private int exitStatus = -1;
             
@@ -293,8 +286,6 @@
                 return exitStatus;
             }
         };
-        context.registerService(ExitStatus.class, exitStatusImpl, null);
-        assertEquals(-1, getExitStatusService(context).getExitStatus());
         StorageCommand command = prepareService(true);
         final CountDownLatch latch = new CountDownLatch(1);
         final boolean[] result = new boolean[1];
@@ -319,14 +310,7 @@
         // should have worked
         assertTrue(result[0]);
         // this impl of ExitStatus has a default value of -1
-        assertEquals(-1, getExitStatusService(context).getExitStatus());
-    }
-    
-    @SuppressWarnings({ "rawtypes", "unchecked" })
-    private ExitStatus getExitStatusService(BundleContext context) {
-        ServiceReference ref = context.getServiceReference(ExitStatus.class);
-        ExitStatus exitStatus = (ExitStatus)context.getService(ref);
-        return exitStatus;
+        assertEquals(-1, this.exitStatus.getExitStatus());
     }
 
     private CommandContext prepareContext() {
@@ -340,14 +324,14 @@
 
     @Test
     public void testName() {
-        StorageCommand dbService = new StorageCommand(context);
+        StorageCommand dbService = new StorageCommand(exitStatus);
         String name = dbService.getName();
         assertEquals("storage", name);
     }
 
     @Test
     public void testDescAndUsage() {
-        StorageCommand dbService = new StorageCommand(context);
+        StorageCommand dbService = new StorageCommand(exitStatus);
         assertNotNull(dbService.getDescription());
         assertNotNull(dbService.getUsage());
     }
--- a/common/core/src/main/java/com/redhat/thermostat/common/Constants.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/common/core/src/main/java/com/redhat/thermostat/common/Constants.java	Fri Mar 08 14:28:35 2013 +0100
@@ -42,9 +42,6 @@
  */
 public class Constants {
 
-    public static final int EXIT_ERROR = 1;
-    public static final int EXIT_SUCCESS = 0;
-
     public static final String GENERIC_SERVICE_CLASSNAME = "GenericClassName";
 
 }
--- a/common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java	Fri Mar 08 14:28:35 2013 +0100
@@ -1,3 +1,38 @@
+/*
+ * Copyright 2012, 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
 package com.redhat.thermostat.common;
 
 import com.redhat.thermostat.annotations.Service;
@@ -6,17 +41,30 @@
  * Service which can be used for certain commands to set the exit status on
  * JVM termination. If the status is set multiple times, the last thread which
  * calls {@link #setExitStatus(int)} wins.
+ * 
  */
 @Service
 public interface ExitStatus {
 
     /**
-     * Sets the exit status, which will be used as status when the
-     * JVM terminates.
+     * JVM exit status indicating successful termination.
+     */
+    public static final int EXIT_SUCCESS = 0;
+    
+    /**
+     * JVM exit status indicating a generic error.
+     */
+    public static final int EXIT_ERROR = 1;
+
+    /**
+     * Sets the exit status, which will be used as status when the JVM
+     * terminates.
      * 
      * @param newExitStatus
+     * @throws IllegalArgumentException
+     *             If newExitStatus < 0 or newExitStatus > 255
      */
-    void setExitStatus(int newExitStatus);
+    void setExitStatus(int newExitStatus) throws IllegalArgumentException;
     
     /**
      * Get the currently set exit status.
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java	Fri Mar 08 14:28:35 2013 +0100
@@ -45,7 +45,6 @@
 import org.osgi.util.tracker.ServiceTracker;
 import org.osgi.util.tracker.ServiceTrackerCustomizer;
 
-import com.redhat.thermostat.common.Constants;
 import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.Launcher;
 import com.redhat.thermostat.common.cli.CommandContextFactory;
@@ -94,7 +93,7 @@
                     new CommandContextFactory(context), bundleService);
             launcherReg = context.registerService(Launcher.class.getName(), launcher, null);
             bundleManReg = context.registerService(BundleManager.class, bundleService, null);
-            ExitStatus exitStatus = new ExitStatusImpl(Constants.EXIT_SUCCESS);
+            ExitStatus exitStatus = new ExitStatusImpl(ExitStatus.EXIT_SUCCESS);
             exitStatusReg = context.registerService(ExitStatus.class, exitStatus, null);
             return keyring;
         }
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java	Fri Mar 08 14:28:35 2013 +0100
@@ -14,7 +14,12 @@
         this.exitStatus = initialExitStatus;
     }
     
-    public synchronized void setExitStatus(int newExitStatus) {
+    public synchronized void setExitStatus(int newExitStatus)
+            throws IllegalArgumentException {
+        if (newExitStatus < 0 || newExitStatus > 255) {
+            throw new IllegalArgumentException(
+                    "Status (" + newExitStatus + ") out ouf range. 0 <= status <= 255.");
+        }
         this.exitStatus = newExitStatus;
     }
     
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java	Fri Mar 08 14:28:35 2013 +0100
@@ -51,7 +51,6 @@
 import com.redhat.thermostat.common.ActionListener;
 import com.redhat.thermostat.common.ActionNotifier;
 import com.redhat.thermostat.common.ApplicationService;
-import com.redhat.thermostat.common.Constants;
 import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.Launcher;
 import com.redhat.thermostat.common.Version;
@@ -169,7 +168,7 @@
             }
 
             // default to success for exit status
-            int exitStatus = Constants.EXIT_SUCCESS;
+            int exitStatus = ExitStatus.EXIT_SUCCESS;
             if (context != null) {
                 ServiceReference storageRef = context.getServiceReference(Storage.class);
                 if (storageRef != null) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ExitStatusImplTest.java	Fri Mar 08 14:28:35 2013 +0100
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2012, 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.launcher.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+
+import com.redhat.thermostat.common.ExitStatus;
+
+public class ExitStatusImplTest {
+
+    @Test
+    public void initialValue() {
+        ExitStatus status = new ExitStatusImpl(0);
+        assertEquals(0, status.getExitStatus());
+    }
+    
+    @Test
+    public void valueOutOfRange() {
+        ExitStatus status = new ExitStatusImpl(0);
+        try {
+            status.setExitStatus(-1);
+            fail("should have thrown IllegalArgumentExcpt");
+        } catch (IllegalArgumentException e) {
+            // pass
+            assertEquals(0, status.getExitStatus());
+        }
+        try {
+            status.setExitStatus(256);
+            fail("should have thrown exptn");
+        } catch (IllegalArgumentException e) {
+            // pass
+            assertEquals(0, status.getExitStatus());
+        }
+        try {
+            status.setExitStatus(150);
+            assertEquals(150, status.getExitStatus());
+        } catch (IllegalArgumentException e) {
+            fail();
+        }
+    }
+}
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java	Tue Mar 26 11:17:08 2013 -0400
+++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java	Fri Mar 08 14:28:35 2013 +0100
@@ -70,7 +70,7 @@
 import com.redhat.thermostat.common.ActionListener;
 import com.redhat.thermostat.common.ActionNotifier;
 import com.redhat.thermostat.common.ApplicationService;
-import com.redhat.thermostat.common.Constants;
+import com.redhat.thermostat.common.ExitStatus;
 import com.redhat.thermostat.common.Version;
 import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand;
 import com.redhat.thermostat.common.cli.Arguments;
@@ -533,7 +533,7 @@
         } catch (ExitException e) {
             // pass, by default launcher exits with an exit status
             // of 0.
-            assertEquals(Constants.EXIT_SUCCESS, e.getExitStatus());
+            assertEquals(ExitStatus.EXIT_SUCCESS, e.getExitStatus());
         }
     }