Revert "proper update device state at end of invocation"

Bug: 26556751
Bug: 26548149
Bug: 22950992

This reverts commit 3a093e55a99e960e54c6077bfeca5a7fa383af9d.

Change-Id: Ib359824a4ee67727fb4cd5008004a0657d689481

3 files changed, 28 insertions, 98 deletions

diff --git a/src/com/android/tradefed/command/CommandScheduler.java b/src/com/android/tradefed/command/CommandScheduler.java
index 143b75ede..c2d0b97bb 100644
--- a/src/com/android/tradefed/command/CommandScheduler.java
+++ b/src/com/android/tradefed/command/CommandScheduler.java
@@ -42,7 +42,6 @@ import com.android.tradefed.device.IDeviceManager;
 import com.android.tradefed.device.IDeviceMonitor;
 import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.device.NoDeviceException;
-import com.android.tradefed.device.TestDeviceState;
 import com.android.tradefed.invoker.IRescheduler;
 import com.android.tradefed.invoker.ITestInvocation;
 import com.android.tradefed.invoker.TestInvocation;
@@ -444,7 +443,7 @@ public class CommandScheduler extends Thread implements ICommandScheduler, IComm
 
         @Override
         public void run() {
-            FreeDeviceState deviceState = null;
+            FreeDeviceState deviceState = FreeDeviceState.AVAILABLE;
             mStartTime = System.currentTimeMillis();
             ITestInvocation instance = getInvocation();
             IConfiguration config = mCmd.getConfiguration();
@@ -468,23 +467,16 @@ public class CommandScheduler extends Thread implements ICommandScheduler, IComm
             } catch (Throwable e) {
                 CLog.e(e);
             } finally {
-                // update device state if not initialized
-                if (deviceState == null) {
-                    TestDeviceState tds = mDevice.getDeviceState();
-                    if (TestDeviceState.ONLINE.equals(tds)) {
-                        deviceState = FreeDeviceState.AVAILABLE;
-                    } else {
-                        deviceState = FreeDeviceState.UNAVAILABLE;
-                    }
-                }
-                for (final IScheduledInvocationListener listener : mListeners) {
-                    listener.invocationComplete(mDevice, deviceState);
-                }
-                removeInvocationThread(this);
                 long elapsedTime = System.currentTimeMillis() - mStartTime;
                 CLog.i("Updating command %d with elapsed time %d ms",
-                        mCmd.getCommandTracker().getId(), elapsedTime);
+                       mCmd.getCommandTracker().getId(), elapsedTime);
+                // remove invocation thread first so another invocation can be started on device
+                // when freed
+                removeInvocationThread(this);
                 mCmd.commandFinished(elapsedTime);
+                for (final IScheduledInvocationListener listener : mListeners) {
+                    listener.invocationComplete(mDevice, deviceState);
+                }
             }
         }
 
diff --git a/tests/src/com/android/tradefed/command/CommandSchedulerTest.java b/tests/src/com/android/tradefed/command/CommandSchedulerTest.java
index 503f5b4c1..0fcb782a3 100644
--- a/tests/src/com/android/tradefed/command/CommandSchedulerTest.java
+++ b/tests/src/com/android/tradefed/command/CommandSchedulerTest.java
@@ -29,7 +29,6 @@ import com.android.tradefed.device.FreeDeviceState;
 import com.android.tradefed.device.IDeviceManager;
 import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.device.MockDeviceManager;
-import com.android.tradefed.device.TestDeviceState;
 import com.android.tradefed.invoker.IRescheduler;
 import com.android.tradefed.invoker.ITestInvocation;
 import com.android.tradefed.log.ITerribleFailureHandler;
@@ -138,17 +137,6 @@ public class CommandSchedulerTest extends TestCase {
     }
 
     /**
-     * Verify all mock objects but only assert that an expected number of devices are available
-     */
-    private void verifyMocks(int expectedNumAvailable, Object... additionalMocks) {
-        EasyMock.verify(mMockConfigFactory, mMockConfiguration, mMockInvocation);
-        for (Object mock : additionalMocks) {
-            EasyMock.verify(mock);
-        }
-        mMockManager.assertNumDevicesAvailable(expectedNumAvailable);
-    }
-
-    /**
      * Verify all mock objects
      */
     private void verifyMocks(Object... additionalMocks) {
@@ -156,14 +144,14 @@ public class CommandSchedulerTest extends TestCase {
         for (Object mock : additionalMocks) {
             EasyMock.verify(mock);
         }
-        mMockManager.assertDevicesAvailable();
+        mMockManager.assertDevicesFreed();
     }
 
     /**
      * Test {@link CommandScheduler#run()} when no configs have been added
      */
     public void testRun_empty() throws InterruptedException {
-        mMockManager.setUpDevices(1);
+        mMockManager.setNumDevices(1);
         replayMocks();
         mScheduler.start();
         while (!mScheduler.isAlive()) {
@@ -211,7 +199,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testRun_oneConfig() throws Throwable {
         String[] args = new String[] {};
-        mMockManager.setUpDevices(2);
+        mMockManager.setNumDevices(2);
         setCreateConfigExpectations(args, 1);
         setExpectedInvokeCalls(1);
         mMockConfiguration.validateOptions();
@@ -224,46 +212,12 @@ public class CommandSchedulerTest extends TestCase {
     }
 
     /**
-     * Test {@link CommandScheduler#run()} when two devices are allocated for tests but only one
-     * available after test
-     */
-    public void testRun_oneConfigLoopNotAvailable() throws Throwable {
-        String[] args = new String[] {};
-        // track if exception occurs on scheduler thread
-        UncaughtExceptionHandler defaultHandler = Thread.getDefaultUncaughtExceptionHandler();
-        try {
-            ExceptionTracker tracker = new ExceptionTracker();
-            Thread.setDefaultUncaughtExceptionHandler(tracker);
-            mMockManager.setUpDevices(
-                    new TestDeviceState[]{TestDeviceState.ONLINE, TestDeviceState.NOT_AVAILABLE});
-            // config should only be created three times
-            setCreateConfigExpectations(args, 3);
-            mCommandOptions.setLoopMode(true);
-            mCommandOptions.setMinLoopTime(50);
-            Object notifier = waitForExpectedInvokeCalls(2);
-            mMockConfiguration.validateOptions();
-            replayMocks();
-            mScheduler.start();
-            mScheduler.addCommand(args);
-            synchronized (notifier) {
-                notifier.wait(1 * 1000);
-            }
-            mScheduler.shutdown();
-            mScheduler.join();
-            verifyMocks(1, new Object[]{});
-            assertNull("exception occurred on background thread!", tracker.mThrowable);
-        } finally {
-            Thread.setDefaultUncaughtExceptionHandler(defaultHandler);
-        }
-    }
-
-    /**
      * Test {@link CommandScheduler#removeAllCommands()} for idle case, where command is waiting for
      * device.
      */
     public void testRemoveAllCommands() throws Throwable {
         String[] args = new String[] {};
-        mMockManager.setUpDevices(0);
+        mMockManager.setNumDevices(0);
         setCreateConfigExpectations(args, 1);
         mMockConfiguration.validateOptions();
         replayMocks();
@@ -281,7 +235,7 @@ public class CommandSchedulerTest extends TestCase {
     public void testRun_dryRun() throws Throwable {
         String[] dryRunArgs = new String[] {"--dry-run"};
         mCommandOptions.setDryRunMode(true);
-        mMockManager.setUpDevices(2);
+        mMockManager.setNumDevices(2);
         setCreateConfigExpectations(dryRunArgs, 1);
 
         // add a second command, to verify the first dry-run command did not get added
@@ -315,7 +269,6 @@ public class CommandSchedulerTest extends TestCase {
 
         ITestDevice mockDevice = EasyMock.createMock(ITestDevice.class);
         EasyMock.expect(mockDevice.getSerialNumber()).andStubReturn("serial");
-        EasyMock.expect(mockDevice.getDeviceState()).andReturn(TestDeviceState.ONLINE);
         IScheduledInvocationListener mockListener = EasyMock
                 .createMock(IScheduledInvocationListener.class);
         mockListener.invocationComplete(mockDevice, FreeDeviceState.AVAILABLE);
@@ -378,7 +331,7 @@ public class CommandSchedulerTest extends TestCase {
         try {
             ExceptionTracker tracker = new ExceptionTracker();
             Thread.setDefaultUncaughtExceptionHandler(tracker);
-            mMockManager.setUpDevices(1);
+            mMockManager.setNumDevices(1);
             // config should only be created three times
             setCreateConfigExpectations(args, 3);
             mCommandOptions.setLoopMode(true);
@@ -431,7 +384,7 @@ public class CommandSchedulerTest extends TestCase {
             EasyMock.expect(mockWtf.onTerribleFailure((String)EasyMock.anyObject(),
                     (Throwable)EasyMock.anyObject())).andReturn(Boolean.TRUE);
             String[] args = new String[] {};
-            mMockManager.setUpDevices(2);
+            mMockManager.setNumDevices(2);
             setCreateConfigExpectations(args, 1);
             mMockConfiguration.validateOptions();
             replayMocks(mockGc, mockWtf);
@@ -453,7 +406,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testRun_configSerial() throws Throwable {
         String[] args = new String[] {};
-        mMockManager.setUpDevices(2);
+        mMockManager.setNumDevices(2);
         setCreateConfigExpectations(args, 2);
         // allocate and free a device to get its serial
         ITestDevice dev = mMockManager.allocateDevice();
@@ -480,7 +433,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testRun_configExcludeSerial() throws Throwable {
         String[] args = new String[] {};
-        mMockManager.setUpDevices(2);
+        mMockManager.setNumDevices(2);
         setCreateConfigExpectations(args, 2);
         // allocate and free a device to get its serial
         ITestDevice dev = mMockManager.allocateDevice();
@@ -503,9 +456,10 @@ public class CommandSchedulerTest extends TestCase {
     /**
      * Test {@link CommandScheduler#run()} when one config has been rescheduled
      */
+    @SuppressWarnings("unchecked")
     public void testRun_rescheduled() throws Throwable {
         String[] args = new String[] {};
-        mMockManager.setUpDevices(2);
+        mMockManager.setNumDevices(2);
         setCreateConfigExpectations(args, 1);
         mMockConfiguration.validateOptions();
         final IConfiguration rescheduledConfig = EasyMock.createMock(IConfiguration.class);
@@ -546,7 +500,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testAddCommandFile() throws ConfigurationException {
         // set number of devices to 0 so we can verify command presence
-        mMockManager.setUpDevices(0);
+        mMockManager.setNumDevices(0);
         List<String> extraArgs = Arrays.asList("--bar");
         setCreateConfigExpectations(new String[] {"foo", "--bar"}, 1);
         mMockConfiguration.validateOptions();
@@ -575,7 +529,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testAddCommandFile_reload() throws ConfigurationException {
         // set number of devices to 0 so we can verify command presence
-        mMockManager.setUpDevices(0);
+        mMockManager.setNumDevices(0);
         String[] addCommandArgs = new String[]{"fromcommand"};
         List<String> extraArgs = Arrays.asList("--bar");
 
@@ -630,7 +584,7 @@ public class CommandSchedulerTest extends TestCase {
      */
     public void testAddCommandFile_twice() throws ConfigurationException {
         // set number of devices to 0 so we can verify command presence
-        mMockManager.setUpDevices(0);
+        mMockManager.setNumDevices(0);
         String[] cmdFile1Args = new String[] {"fromFile1"};
         setCreateConfigExpectations(cmdFile1Args, 1);
         setCreateConfigExpectations(cmdFile1Args, 1);
@@ -668,7 +622,7 @@ public class CommandSchedulerTest extends TestCase {
      * Test {@link CommandScheduler#shutdown()} when no devices are available.
      */
     public void testShutdown() throws Exception {
-        mMockManager.setUpDevices(0);
+        mMockManager.setNumDevices(0);
         mScheduler.start();
         while (!mScheduler.isAlive()) {
             Thread.sleep(10);
diff --git a/tests/src/com/android/tradefed/device/MockDeviceManager.java b/tests/src/com/android/tradefed/device/MockDeviceManager.java
index 2f32b7144..a016dc871 100644
--- a/tests/src/com/android/tradefed/device/MockDeviceManager.java
+++ b/tests/src/com/android/tradefed/device/MockDeviceManager.java
@@ -25,7 +25,6 @@ import org.easymock.EasyMock;
 import org.junit.Assert;
 
 import java.io.PrintWriter;
-import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -42,11 +41,10 @@ public class MockDeviceManager implements IDeviceManager {
     private DeviceMonitorMultiplexer mDvcMon = new DeviceMonitorMultiplexer();
 
     public MockDeviceManager(int numDevices) {
-        setUpDevices(numDevices);
+        setNumDevices(numDevices);
     }
 
-    public void setUpDevices(TestDeviceState[] deviceStates) {
-        int numDevices = deviceStates.length;
+    public void setNumDevices(int numDevices) {
         mAvailableDeviceQueue.clear();
         mTotalDevices = numDevices;
         for (int i = 0; i < numDevices; i++) {
@@ -56,18 +54,12 @@ public class MockDeviceManager implements IDeviceManager {
             EasyMock.expect(mockIDevice.getSerialNumber()).andReturn("serial" + i).anyTimes();
             EasyMock.expect(mockDevice.getIDevice()).andReturn(mockIDevice).anyTimes();
             EasyMock.expect(mockDevice.getDeviceState()).andReturn(
-                    deviceStates[i]).anyTimes();
+                    TestDeviceState.ONLINE).anyTimes();
             EasyMock.replay(mockDevice, mockIDevice);
             mAvailableDeviceQueue.add(mockDevice);
         }
     }
 
-    public void setUpDevices(int numDevices) {
-        TestDeviceState[] deviceStates = new TestDeviceState[numDevices];
-        Arrays.fill(deviceStates, TestDeviceState.ONLINE);
-        setUpDevices(deviceStates);
-    }
-
     private static class TestDeviceMatcher implements IMatcher<ITestDevice> {
         private IDeviceSelection mDeviceOptions;
 
@@ -183,16 +175,8 @@ public class MockDeviceManager implements IDeviceManager {
      * Verifies that all devices were returned to queue.
      * @throws AssertionError
      */
-    public void assertDevicesAvailable() throws AssertionError {
-        assertNumDevicesAvailable(mTotalDevices);
-    }
-
-    /**
-     * Verifies that expected number of devices were returned to queue.
-     * @throws AssertionError
-     */
-    public void assertNumDevicesAvailable(int expectedAvailable) throws AssertionError {
-        Assert.assertEquals("unexpected number of available devices", expectedAvailable,
+    public void assertDevicesFreed() throws AssertionError {
+        Assert.assertEquals("allocated device was not returned to queue", mTotalDevices,
                 mAvailableDeviceQueue.size());
     }