checkFrameworkSupport improvements for accuracy
Add retry and enforce the expected outputs.
Bug: 30558560
Change-Id: I70254bebe9f0387670f4a0d0060fb57215475f0f
diff --git a/remote/src/com/android/tradefed/device/DeviceAllocationEventHandler.java b/remote/src/com/android/tradefed/device/DeviceAllocationEventHandler.java
index 382ac8d..9121098 100644
--- a/remote/src/com/android/tradefed/device/DeviceAllocationEventHandler.java
+++ b/remote/src/com/android/tradefed/device/DeviceAllocationEventHandler.java
@@ -68,6 +68,8 @@
* <li>Checking_Availability -> AVAILABLE_CHECK_FAILED -> Unavailable</li>
* <li>Checking_Availability -> AVAILABLE_CHECK_IGNORED -> Ignored</li>
* <li>Checking_Availability -> FORCE_AVAILABLE -> Available</li>
+ * <li>Checking_Availability -> STATE_CHANGE_OFFLINE -> Unavailable</li>
+ * <li>Checking_Availability -> DISCONNECTED -> Unavailable</li>
* </ul>
*/
class CheckingAvailHandler implements DeviceAllocationEventHandler {
@@ -84,6 +86,10 @@
return DeviceAllocationState.Ignored;
case FORCE_AVAILABLE:
return DeviceAllocationState.Available;
+ case STATE_CHANGE_OFFLINE:
+ return DeviceAllocationState.Unavailable;
+ case DISCONNECTED:
+ return DeviceAllocationState.Unavailable;
default:
return DeviceAllocationState.Checking_Availability;
}
diff --git a/src/com/android/tradefed/device/DeviceManager.java b/src/com/android/tradefed/device/DeviceManager.java
index 95d9bd3..3178d50 100644
--- a/src/com/android/tradefed/device/DeviceManager.java
+++ b/src/com/android/tradefed/device/DeviceManager.java
@@ -306,7 +306,8 @@
Runnable checkRunnable = new Runnable() {
@Override
public void run() {
- CLog.d("checking new device %s responsiveness", testDevice.getSerialNumber());
+ CLog.d("checking new '%s' '%s' responsiveness", testDevice.getClass().getName(),
+ testDevice.getSerialNumber());
if (testDevice.getMonitor().waitForDeviceShell(CHECK_WAIT_DEVICE_AVAIL_MS)) {
DeviceEventResponse r = mManagedDeviceList.handleDeviceEvent(testDevice,
DeviceEvent.AVAILABLE_CHECK_PASSED);
diff --git a/src/com/android/tradefed/device/ManagedTestDeviceFactory.java b/src/com/android/tradefed/device/ManagedTestDeviceFactory.java
index 26cd728..2ee5e1b 100644
--- a/src/com/android/tradefed/device/ManagedTestDeviceFactory.java
+++ b/src/com/android/tradefed/device/ManagedTestDeviceFactory.java
@@ -22,6 +22,8 @@
import com.android.ddmlib.TimeoutException;
import com.android.tradefed.device.DeviceManager.FastbootDevice;
import com.android.tradefed.log.LogUtil.CLog;
+import com.android.tradefed.util.IRunUtil;
+import com.android.tradefed.util.RunUtil;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
@@ -44,6 +46,9 @@
protected IDeviceMonitor mAllocationMonitor;
protected static final String CHECK_PM_CMD = "ls %s";
protected static final String EXPECTED_RES = "/system/bin/pm";
+ protected static final String EXPECTED_ERROR = "No such file or directory";
+ protected static final long FRAMEWORK_CHECK_SLEEP_MS = 500;
+ protected static final int FRAMEWORK_CHECK_MAX_RETRY = 3;
public ManagedTestDeviceFactory(boolean fastbootEnabled, IDeviceManager deviceManager,
IDeviceMonitor allocationMonitor) {
@@ -96,20 +101,32 @@
return true;
}
final long timeout = 60 * 1000;
- CollectingOutputReceiver receiver = createOutputReceiver();
try {
- if (!DeviceState.ONLINE.equals(idevice.getState())) {
- // Device will be 'unavailable' and recreated in DeviceManager so no need to check.
- CLog.w("Device state is not Online, assuming Framework support for now.");
- return true;
+ for (int i = 0; i < FRAMEWORK_CHECK_MAX_RETRY; i++) {
+ CollectingOutputReceiver receiver = createOutputReceiver();
+ if (!DeviceState.ONLINE.equals(idevice.getState())) {
+ // Device will be 'unavailable' and recreated in DeviceManager so no need to
+ // check.
+ CLog.w("Device state is not Online, assuming Framework support for now.");
+ return true;
+ }
+ String cmd = String.format(CHECK_PM_CMD, EXPECTED_RES);
+ idevice.executeShellCommand(cmd, receiver, timeout, TimeUnit.MILLISECONDS);
+ String output = receiver.getOutput().trim();
+ // It can only be one of the expected output or an exception if device offline
+ // otherwise we retry
+ if (EXPECTED_RES.equals(output)) {
+ return true;
+ }
+ if (output.contains(EXPECTED_ERROR)) {
+ CLog.i("No support for Framework, creating a native device. "
+ + "output: %s", receiver.getOutput());
+ return false;
+ }
+ getRunUtil().sleep(FRAMEWORK_CHECK_SLEEP_MS);
}
- String cmd = String.format(CHECK_PM_CMD, EXPECTED_RES);
- idevice.executeShellCommand(cmd, receiver, timeout, TimeUnit.MILLISECONDS);
- if (!EXPECTED_RES.equals(receiver.getOutput().trim())) {
- CLog.i("No support for Framework, creating a native device. "
- + "output: %s", receiver.getOutput());
- return false;
- }
+ CLog.w("Could not determine the framework support for '%s' after retries, assuming "
+ + "framework support.", idevice.getSerialNumber());
} catch (TimeoutException | AdbCommandRejectedException | ShellCommandUnresponsiveException
| IOException e) {
CLog.w("Exception during checkFrameworkSupport, assuming True: '%s' with device: %s",
@@ -121,6 +138,14 @@
}
/**
+ * Return the default {@link IRunUtil} instance.
+ * Exposed for testing.
+ */
+ protected IRunUtil getRunUtil() {
+ return RunUtil.getDefault();
+ }
+
+ /**
* Create a {@link CollectingOutputReceiver}.
* Exposed for testing.
*/
diff --git a/tests/src/com/android/tradefed/device/DeviceManagerTest.java b/tests/src/com/android/tradefed/device/DeviceManagerTest.java
index 1b7cbcf..b4d3db0 100644
--- a/tests/src/com/android/tradefed/device/DeviceManagerTest.java
+++ b/tests/src/com/android/tradefed/device/DeviceManagerTest.java
@@ -36,7 +36,6 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.util.List;
-import java.util.concurrent.TimeUnit;
/**
* Unit tests for {@link DeviceManager}.
@@ -146,13 +145,21 @@
mMockRunUtil = EasyMock.createMock(IRunUtil.class);
mMockTestDevice = EasyMock.createMock(IManagedTestDevice.class);
- mMockDeviceFactory = new IManagedTestDeviceFactory() {
+ mMockDeviceFactory = new ManagedTestDeviceFactory(false, null, null) {
@Override
public IManagedTestDevice createDevice(IDevice idevice) {
mMockTestDevice.setIDevice(idevice);
return mMockTestDevice;
}
-
+ @Override
+ protected CollectingOutputReceiver createOutputReceiver() {
+ return new CollectingOutputReceiver() {
+ @Override
+ public String getOutput() {
+ return "/system/bin/pm";
+ }
+ };
+ }
@Override
public void setFastbootEnabled(boolean enable) {
// ignore
@@ -500,19 +507,24 @@
* with a global exclusion filter
*/
public void testInit_excludeDevice() throws Exception {
- String expectedCmd = String.format(ManagedTestDeviceFactory.CHECK_PM_CMD,
- ManagedTestDeviceFactory.EXPECTED_RES);
- EasyMock.expect(mMockIDevice.getState()).andReturn(DeviceState.ONLINE).times(3);
- mMockIDevice.executeShellCommand(EasyMock.eq(expectedCmd),
- (CollectingOutputReceiver)EasyMock.anyObject(), EasyMock.eq(60000l),
- EasyMock.eq(TimeUnit.MILLISECONDS));
+ EasyMock.expect(mMockIDevice.getState()).andReturn(DeviceState.ONLINE);
+ mMockTestDevice.setDeviceState(TestDeviceState.ONLINE);
EasyMock.expectLastCall();
+ DeviceEventResponse der =
+ new DeviceEventResponse(DeviceAllocationState.Checking_Availability, true);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.CONNECTED_ONLINE))
+ .andReturn(der);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.AVAILABLE_CHECK_IGNORED))
+ .andReturn(null);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.ALLOCATE_REQUEST))
+ .andReturn(new DeviceEventResponse(DeviceAllocationState.Ignored, false));
replayMocks();
DeviceManager manager = createDeviceManagerNoInit();
DeviceSelectionOptions excludeFilter = new DeviceSelectionOptions();
excludeFilter.addExcludeSerial(mMockIDevice.getSerialNumber());
- manager.init(excludeFilter, null);
+ manager.init(excludeFilter, null, mMockDeviceFactory);
mDeviceListener.deviceConnected(mMockIDevice);
+ assertEquals(1, manager.getDeviceList().size());
assertNull(manager.allocateDevice());
verifyMocks();
}
@@ -526,16 +538,22 @@
EasyMock.expect(excludedDevice.getState()).andStubReturn(DeviceState.ONLINE);
EasyMock.expect(mMockIDevice.getState()).andStubReturn(DeviceState.ONLINE);
EasyMock.expect(excludedDevice.isEmulator()).andStubReturn(Boolean.FALSE);
- excludedDevice.executeShellCommand((String)EasyMock.anyObject(),
- (CollectingOutputReceiver)EasyMock.anyObject(), EasyMock.eq(60000l),
- EasyMock.eq(TimeUnit.MILLISECONDS));
- EasyMock.expectLastCall();
+ mMockTestDevice.setDeviceState(TestDeviceState.ONLINE);
+ DeviceEventResponse der =
+ new DeviceEventResponse(DeviceAllocationState.Checking_Availability, true);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.CONNECTED_ONLINE))
+ .andReturn(der);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.AVAILABLE_CHECK_IGNORED))
+ .andReturn(null);
+ EasyMock.expect(mMockTestDevice.handleAllocationEvent(DeviceEvent.ALLOCATE_REQUEST))
+ .andReturn(new DeviceEventResponse(DeviceAllocationState.Ignored, false));
replayMocks(excludedDevice);
DeviceManager manager = createDeviceManagerNoInit();
DeviceSelectionOptions includeFilter = new DeviceSelectionOptions();
includeFilter.addSerial(mMockIDevice.getSerialNumber());
- manager.init(includeFilter, null);
+ manager.init(includeFilter, null, mMockDeviceFactory);
mDeviceListener.deviceConnected(excludedDevice);
+ assertEquals(1, manager.getDeviceList().size());
// ensure excludedDevice cannot be allocated
assertNull(manager.allocateDevice());
verifyMocks(excludedDevice);
diff --git a/tests/src/com/android/tradefed/device/ManagedTestDeviceFactoryTest.java b/tests/src/com/android/tradefed/device/ManagedTestDeviceFactoryTest.java
index ec63181..a06c606 100644
--- a/tests/src/com/android/tradefed/device/ManagedTestDeviceFactoryTest.java
+++ b/tests/src/com/android/tradefed/device/ManagedTestDeviceFactoryTest.java
@@ -17,6 +17,7 @@
import com.android.ddmlib.IDevice;
import com.android.ddmlib.IDevice.DeviceState;
+import com.android.tradefed.util.IRunUtil;
import junit.framework.TestCase;
@@ -32,6 +33,7 @@
ManagedTestDeviceFactory mFactory;
IDeviceManager mMockDeviceManager;
IDeviceMonitor mMockDeviceMonitor;
+ IRunUtil mMockRunUtil;
@Override
protected void setUp() throws Exception {
@@ -39,6 +41,7 @@
mMockDeviceManager = EasyMock.createMock(IDeviceManager.class);
mMockDeviceMonitor = EasyMock.createMock(IDeviceMonitor.class);
mFactory = new ManagedTestDeviceFactory(true, mMockDeviceManager, mMockDeviceMonitor) ;
+ mMockRunUtil = EasyMock.createMock(IRunUtil.class);
}
public void testIsSerialTcpDevice() {
@@ -117,4 +120,37 @@
assertFalse(mFactory.checkFrameworkSupport(mMockDevice));
EasyMock.verify(mMockDevice);
}
+
+ /**
+ * Test that {@link ManagedTestDeviceFactory#checkFrameworkSupport(IDevice)} is retrying
+ * because device doesn't return a proper answer. It should return True for default value.
+ */
+ public void testCheckFramework_emptyReturns() throws Exception {
+ final CollectingOutputReceiver cor = new CollectingOutputReceiver();
+ mFactory = new ManagedTestDeviceFactory(true, mMockDeviceManager, mMockDeviceMonitor) {
+ @Override
+ protected CollectingOutputReceiver createOutputReceiver() {
+ String response = "";
+ cor.addOutput(response.getBytes(), 0, response.length());
+ return cor;
+ }
+ @Override
+ protected IRunUtil getRunUtil() {
+ return mMockRunUtil;
+ }
+ };
+ mMockRunUtil.sleep(500);
+ EasyMock.expectLastCall().times(ManagedTestDeviceFactory.FRAMEWORK_CHECK_MAX_RETRY);
+ IDevice mMockDevice = EasyMock.createMock(IDevice.class);
+ EasyMock.expect(mMockDevice.getSerialNumber()).andStubReturn("SERIAL");
+ EasyMock.expect(mMockDevice.getState()).andStubReturn(DeviceState.ONLINE);
+ String expectedCmd = String.format(ManagedTestDeviceFactory.CHECK_PM_CMD,
+ ManagedTestDeviceFactory.EXPECTED_RES);
+ mMockDevice.executeShellCommand(EasyMock.eq(expectedCmd), EasyMock.eq(cor),
+ EasyMock.anyLong(), EasyMock.eq(TimeUnit.MILLISECONDS));
+ EasyMock.expectLastCall().times(ManagedTestDeviceFactory.FRAMEWORK_CHECK_MAX_RETRY);
+ EasyMock.replay(mMockDevice, mMockRunUtil);
+ assertTrue(mFactory.checkFrameworkSupport(mMockDevice));
+ EasyMock.verify(mMockDevice, mMockRunUtil);
+ }
}
