Merge "Only attempt bugreport if module fails with device error" am: 574e7ae4c6

Original change: https://android-review.googlesource.com/c/platform/tools/tradefederation/+/1364900

Change-Id: I182a739a5c845cf974e4dcea46248f11cc64f6eb

3 files changed, 28 insertions, 20 deletions

diff --git a/src/com/android/tradefed/testtype/suite/ModuleDefinition.java b/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
index 0372eb7ef..9032b003e 100644
--- a/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
+++ b/src/com/android/tradefed/testtype/suite/ModuleDefinition.java
@@ -509,7 +509,13 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
                 // After the run, if the test failed (even after retry the final result passed) has
                 // failed, capture a bugreport.
                 if (retriableTest.getResultListener().hasLastAttemptFailed()) {
-                    captureBugreport(listener, getId());
+                    captureBugreport(
+                            listener,
+                            getId(),
+                            retriableTest
+                                    .getResultListener()
+                                    .getCurrentRunResults()
+                                    .getRunFailureDescription());
                 }
             }
         } finally {
@@ -609,7 +615,13 @@ public class ModuleDefinition implements Comparable<ModuleDefinition>, ITestColl
         return retriableTest;
     }
 
-    private void captureBugreport(ITestLogger listener, String moduleId) {
+    private void captureBugreport(
+            ITestLogger listener, String moduleId, FailureDescription failure) {
+        FailureStatus status = failure.getFailureStatus();
+        if (!FailureStatus.LOST_SYSTEM_UNDER_TEST.equals(status)
+                && !FailureStatus.SYSTEM_UNDER_TEST_CRASHED.equals(status)) {
+            return;
+        }
         for (ITestDevice device : mModuleInvocationContext.getDevices()) {
             if (device.getIDevice() instanceof StubDevice) {
                 continue;
diff --git a/tests/src/com/android/tradefed/testtype/suite/ITestSuiteTest.java b/tests/src/com/android/tradefed/testtype/suite/ITestSuiteTest.java
index 592a71dff..7d89e9366 100644
--- a/tests/src/com/android/tradefed/testtype/suite/ITestSuiteTest.java
+++ b/tests/src/com/android/tradefed/testtype/suite/ITestSuiteTest.java
@@ -59,6 +59,7 @@ import com.android.tradefed.result.ILogSaver;
 import com.android.tradefed.result.ITestInvocationListener;
 import com.android.tradefed.result.MultiFailureDescription;
 import com.android.tradefed.result.TestDescription;
+import com.android.tradefed.result.error.DeviceErrorIdentifier;
 import com.android.tradefed.result.proto.TestRecordProto.FailureStatus;
 import com.android.tradefed.retry.BaseRetryDecision;
 import com.android.tradefed.retry.IRetryDecision;
@@ -661,7 +662,9 @@ public class ITestSuiteTest {
                             fake.setTest(
                                     new StubCollectingTest(
                                             new DeviceUnresponsiveException(
-                                                    "unresponsive", "serial")));
+                                                    "unresponsive",
+                                                    "serial",
+                                                    DeviceErrorIdentifier.DEVICE_UNRESPONSIVE)));
                             testConfig.put(TEST_CONFIG_NAME, fake);
                         } catch (ConfigurationException e) {
                             CLog.e(e);
@@ -683,7 +686,8 @@ public class ITestSuiteTest {
         mMockListener.testRunStarted(
                 EasyMock.eq(TEST_CONFIG_NAME), EasyMock.eq(1), EasyMock.eq(0), EasyMock.anyLong());
         EasyMock.expectLastCall().times(1);
-        mMockListener.testRunFailed(FailureDescription.create("unresponsive"));
+        mMockListener.testRunFailed(
+                FailureDescription.create("unresponsive", FailureStatus.LOST_SYSTEM_UNDER_TEST));
         EasyMock.expect(
                         mMockDevice.logBugreport(
                                 EasyMock.eq("module-test-failure-SERIAL-bugreport"),
@@ -811,11 +815,6 @@ public class ITestSuiteTest {
         EasyMock.expectLastCall().times(1);
         Capture<FailureDescription> captured = new Capture<>();
         mMockListener.testRunFailed(EasyMock.capture(captured));
-        EasyMock.expect(
-                        mMockDevice.logBugreport(
-                                EasyMock.eq("module-test-failure-SERIAL-bugreport"),
-                                EasyMock.anyObject()))
-                .andReturn(true);
         mMockListener.testRunEnded(
                 EasyMock.anyLong(), EasyMock.<HashMap<String, Metric>>anyObject());
         EasyMock.expectLastCall().times(1);
diff --git a/tests/src/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java b/tests/src/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
index 18e3ce8ef..f9c62b5fa 100644
--- a/tests/src/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
+++ b/tests/src/com/android/tradefed/testtype/suite/ModuleDefinitionTest.java
@@ -38,7 +38,6 @@ import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.device.DeviceUnresponsiveException;
 import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.device.ITestDevice.RecoveryMode;
-import com.android.tradefed.device.StubDevice;
 import com.android.tradefed.invoker.IInvocationContext;
 import com.android.tradefed.invoker.TestInformation;
 import com.android.tradefed.invoker.TestInvocation;
@@ -57,6 +56,8 @@ import com.android.tradefed.result.MultiFailureDescription;
 import com.android.tradefed.result.ResultForwarder;
 import com.android.tradefed.result.TestDescription;
 import com.android.tradefed.result.TestRunResult;
+import com.android.tradefed.result.error.DeviceErrorIdentifier;
+import com.android.tradefed.result.proto.TestRecordProto.FailureStatus;
 import com.android.tradefed.retry.BaseRetryDecision;
 import com.android.tradefed.retry.IRetryDecision;
 import com.android.tradefed.targetprep.BaseTargetPreparer;
@@ -160,10 +161,12 @@ public class ModuleDefinitionTest {
                 TestDescription test = new TestDescription(mRunName + "class", "test" + i);
                 listener.testStarted(test);
                 if (mShouldThrow && i == mNumTest / 2) {
-                    throw new DeviceNotAvailableException("unavailable", "serial");
+                    throw new DeviceNotAvailableException(
+                            "unavailable", "serial", DeviceErrorIdentifier.DEVICE_UNAVAILABLE);
                 }
                 if (mDeviceUnresponsive) {
-                    throw new DeviceUnresponsiveException("unresponsive", "serial");
+                    throw new DeviceUnresponsiveException(
+                            "unresponsive", "serial", DeviceErrorIdentifier.DEVICE_UNRESPONSIVE);
                 }
                 if (mThrowError && i == mNumTest / 2) {
                     throw new AssertionError("assert error");
@@ -1048,12 +1051,6 @@ public class ModuleDefinitionTest {
         mMockListener.testRunEnded(
                 EasyMock.anyLong(), EasyMock.<HashMap<String, Metric>>anyObject());
 
-        // Run failed
-        EasyMock.expect(mMockDevice.getIDevice()).andReturn(EasyMock.createMock(IDevice.class));
-        EasyMock.expect(mMockDevice.getSerialNumber()).andReturn("serial");
-        EasyMock.expect(mMockDevice.logBugreport(EasyMock.anyObject(), EasyMock.anyObject()))
-                .andReturn(true);
-
         replayMocks();
         mModule.run(mModuleInfo, mMockListener);
         // Only one module
@@ -1388,7 +1385,8 @@ public class ModuleDefinitionTest {
                     EasyMock.<HashMap<String, Metric>>anyObject());
         }
         mMockListener.testFailed(EasyMock.anyObject(), (String) EasyMock.anyObject());
-        FailureDescription issues = FailureDescription.create("unresponsive");
+        FailureDescription issues =
+                FailureDescription.create("unresponsive", FailureStatus.LOST_SYSTEM_UNDER_TEST);
         mMockListener.testRunFailed(issues);
         mMockListener.testRunEnded(
                 EasyMock.anyLong(), EasyMock.<HashMap<String, Metric>>anyObject());
@@ -1564,7 +1562,6 @@ public class ModuleDefinitionTest {
         mModule.setBuild(mMockBuildInfo);
         mModule.setDevice(mMockDevice);
 
-        EasyMock.expect(mMockDevice.getIDevice()).andReturn(new StubDevice("fake"));
         EasyMock.expect(mMockPrep.isDisabled()).andReturn(false).times(2);
         // no isTearDownDisabled() expected for setup
         mMockPrep.setUp(EasyMock.eq(mModuleInfo));