Merge "Add option to throw exception when module to install does not have"
diff --git a/invocation_interfaces/com/android/tradefed/result/ITestInvocationListener.java b/invocation_interfaces/com/android/tradefed/result/ITestInvocationListener.java
index e6f5357..ed8e1b7 100644
--- a/invocation_interfaces/com/android/tradefed/result/ITestInvocationListener.java
+++ b/invocation_interfaces/com/android/tradefed/result/ITestInvocationListener.java
@@ -73,6 +73,23 @@
     default public void invocationFailed(Throwable cause) { }
 
     /**
+     * Reports an incomplete invocation due to some error condition.
+     *
+     * <p>Will be automatically called by the TradeFederation framework.
+     *
+     * @param failure the {@link FailureDescription} describing the cause of the failure
+     */
+    public default void invocationFailed(FailureDescription failure) {
+        if (failure.getCause() != null) {
+            invocationFailed(failure.getCause());
+        } else {
+            invocationFailed(
+                    new RuntimeException(
+                            String.format("ConvertedFailure: %s", failure.getErrorMessage())));
+        }
+    }
+
+    /**
      * Allows the InvocationListener to return a summary.
      *
      * @return A {@link TestSummary} summarizing the run, or null
diff --git a/src/com/android/tradefed/device/metric/BaseDeviceMetricCollector.java b/src/com/android/tradefed/device/metric/BaseDeviceMetricCollector.java
index 4cea7b1..1f5be9d 100644
--- a/src/com/android/tradefed/device/metric/BaseDeviceMetricCollector.java
+++ b/src/com/android/tradefed/device/metric/BaseDeviceMetricCollector.java
@@ -204,6 +204,11 @@
     }
 
     @Override
+    public final void invocationFailed(FailureDescription failure) {
+        mForwarder.invocationFailed(failure);
+    }
+
+    @Override
     public final void invocationEnded(long elapsedTime) {
         mForwarder.invocationEnded(elapsedTime);
     }
diff --git a/src/com/android/tradefed/invoker/RemoteInvocationExecution.java b/src/com/android/tradefed/invoker/RemoteInvocationExecution.java
index d7843d9..a87a79e 100644
--- a/src/com/android/tradefed/invoker/RemoteInvocationExecution.java
+++ b/src/com/android/tradefed/invoker/RemoteInvocationExecution.java
@@ -359,8 +359,12 @@
                             info,
                             options,
                             runUtil);
-            mRemoteConsoleStdErr = FileUtil.readStringFromFile(stderr);
-            FileUtil.recursiveDelete(stderr);
+            if (stderr != null && stderr.exists()) {
+                mRemoteConsoleStdErr = FileUtil.readStringFromFile(stderr);
+                FileUtil.recursiveDelete(stderr);
+            } else {
+                mRemoteConsoleStdErr = "Failed to fetch stderr from remote.";
+            }
         }
 
         // If not result in progress are reported, parse the full results at the end.
diff --git a/src/com/android/tradefed/postprocessor/BasePostProcessor.java b/src/com/android/tradefed/postprocessor/BasePostProcessor.java
index afa7b8a..7b2c464 100644
--- a/src/com/android/tradefed/postprocessor/BasePostProcessor.java
+++ b/src/com/android/tradefed/postprocessor/BasePostProcessor.java
@@ -107,6 +107,11 @@
     }
 
     @Override
+    public final void invocationFailed(FailureDescription failure) {
+        mForwarder.invocationFailed(failure);
+    }
+
+    @Override
     public final void invocationEnded(long elapsedTime) {
         mForwarder.invocationEnded(elapsedTime);
     }
diff --git a/src/com/android/tradefed/result/LegacySubprocessResultsReporter.java b/src/com/android/tradefed/result/LegacySubprocessResultsReporter.java
index ae27012..a4a8a76 100644
--- a/src/com/android/tradefed/result/LegacySubprocessResultsReporter.java
+++ b/src/com/android/tradefed/result/LegacySubprocessResultsReporter.java
@@ -19,6 +19,7 @@
 import com.android.tradefed.build.IBuildInfo;
 import com.android.tradefed.invoker.IInvocationContext;
 import com.android.tradefed.log.LogUtil.CLog;
+import com.android.tradefed.testtype.IAbi;
 import com.android.tradefed.util.SubprocessEventHelper.BaseTestEventInfo;
 import com.android.tradefed.util.SubprocessEventHelper.FailedTestEventInfo;
 import com.android.tradefed.util.SubprocessEventHelper.InvocationFailedEventInfo;
@@ -28,30 +29,34 @@
 import com.android.tradefed.util.SubprocessEventHelper.TestRunFailedEventInfo;
 import com.android.tradefed.util.SubprocessEventHelper.TestRunStartedEventInfo;
 import com.android.tradefed.util.SubprocessEventHelper.TestStartedEventInfo;
+import com.android.tradefed.util.SubprocessEventHelper.TestModuleStartedEventInfo;
+import com.android.tradefed.util.SubprocessEventHelper.LogAssociationEventInfo;
 import com.android.tradefed.util.SubprocessTestResultsParser;
 
+import org.json.JSONObject;
+
+import java.io.Serializable;
 import java.util.Map;
 
 /**
- * A class for freezed subprocess results reporter which is compatible with M & N version of CTS.
- * Methods in this class were part of ITestInvocationListener in pre-O version of TF/CTS. Changes
- * are not supposed to be made on this class.
+ * A frozen implementation of the subprocess results reporter which should remain compatible with
+ * earlier versions of TF/CTS (e.g. 8+), despite changes in its superclass.
  */
 public final class LegacySubprocessResultsReporter extends SubprocessResultsReporter {
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testAssumptionFailure(TestIdentifier testId, String trace) {
         FailedTestEventInfo info =
                 new FailedTestEventInfo(testId.getClassName(), testId.getTestName(), trace);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_ASSUMPTION_FAILURE, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testEnded(TestIdentifier testId, Map<String, String> metrics) {
         testEnded(testId, System.currentTimeMillis(), metrics);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testEnded(TestIdentifier testId, long endTime, Map<String, String> metrics) {
         TestEndedEventInfo info =
                 new TestEndedEventInfo(
@@ -59,100 +64,123 @@
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_ENDED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testFailed(TestIdentifier testId, String reason) {
         FailedTestEventInfo info =
                 new FailedTestEventInfo(testId.getClassName(), testId.getTestName(), reason);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_FAILED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testIgnored(TestIdentifier testId) {
         BaseTestEventInfo info = new BaseTestEventInfo(testId.getClassName(), testId.getTestName());
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_IGNORED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testStarted(TestIdentifier testId) {
         testStarted(testId, System.currentTimeMillis());
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void testStarted(TestIdentifier testId, long startTime) {
         TestStartedEventInfo info =
                 new TestStartedEventInfo(testId.getClassName(), testId.getTestName(), startTime);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_STARTED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     public void invocationStarted(IBuildInfo buildInfo) {
         InvocationStartedEventInfo info =
                 new InvocationStartedEventInfo(buildInfo.getTestTag(), System.currentTimeMillis());
         printEvent(SubprocessTestResultsParser.StatusKeys.INVOCATION_STARTED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     @Override
     public void invocationFailed(Throwable cause) {
         InvocationFailedEventInfo info = new InvocationFailedEventInfo(cause);
         printEvent(SubprocessTestResultsParser.StatusKeys.INVOCATION_FAILED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8. */
     @Override
     public void invocationEnded(long elapsedTime) {
         // ignore
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     @Override
     public void testRunFailed(String reason) {
         TestRunFailedEventInfo info = new TestRunFailedEventInfo(reason);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_RUN_FAILED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     @Override
     public void testRunStarted(String runName, int testCount) {
         TestRunStartedEventInfo info = new TestRunStartedEventInfo(runName, testCount);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_RUN_STARTED, info);
     }
 
-    /* Legacy method based on M cts api, pls do not change*/
+    /* Legacy method compatible with TF/CTS 8+. */
     @Override
     public void testRunEnded(long time, Map<String, String> runMetrics) {
         TestRunEndedEventInfo info = new TestRunEndedEventInfo(time, runMetrics);
         printEvent(SubprocessTestResultsParser.StatusKeys.TEST_RUN_ENDED, info);
     }
 
-    /** A intentionally inop function to handle incompatibility problem in CTS 8.1 */
+    /* Legacy method compatible with TF/CTS 9+ (skipped in 8.1 and not called in 8). */
     @Override
     public void testModuleStarted(IInvocationContext moduleContext) {
-        CLog.d("testModuleStarted is called but ignored intentionally");
+        if (!Serializable.class.isAssignableFrom(IAbi.class)) {
+            // TODO(b/154349022): remove after releasing serialization fix
+            // Test packages prior to 1d6869f will fail with a not serializable exception.
+            // see https://cs.android.com/android/_/android/platform/tools/tradefederation/+/1d6869f
+            CLog.d("testModuleStarted is called but ignored intentionally");
+            return;
+        }
+        TestModuleStartedEventInfo info = new TestModuleStartedEventInfo(moduleContext);
+        printEvent(SubprocessTestResultsParser.StatusKeys.TEST_MODULE_STARTED, info);
     }
 
-    /** A intentionally inop function to handle incompatibility problem in CTS 8.1 */
+    /* Legacy method compatible with TF/CTS 9+ (skipped in 8.1 and not called in 8). */
     @Override
     public void testModuleEnded() {
-        CLog.d("testModuleEnded is called but ignored intentionally");
+        if (!Serializable.class.isAssignableFrom(IAbi.class)) {
+            // TODO(b/154349022): remove after releasing serialization fix
+            // Test packages prior to 1d6869f will fail with a not serializable exception.
+            // see https://cs.android.com/android/_/android/platform/tools/tradefederation/+/1d6869f
+            CLog.d("testModuleEnded is called but ignored intentionally");
+            return;
+        }
+        printEvent(SubprocessTestResultsParser.StatusKeys.TEST_MODULE_ENDED, new JSONObject());
     }
 
-    /** A intentionally inop function to handle incompatibility problem in CTS 8.1 */
+    /* Legacy method compatible with TF/CTS 8.1+ (not called in 8). */
     @Override
     public void testLogSaved(
             String dataName, LogDataType dataType, InputStreamSource dataStream, LogFile logFile) {
-        CLog.d("testLogSaved is called but ignored intentionally");
+        // ignore
     }
 
-    /** A intentionally inop function to handle incompatibility problem in CTS 8.1 */
+    /* Legacy method compatible with TF/CTS 9+ (skipped in 8.1 and not called in 8). */
     @Override
     public void logAssociation(String dataName, LogFile logFile) {
-        CLog.d("logAssociation is called but ignored intentionally");
+        if (!Serializable.class.isAssignableFrom(LogFile.class)) {
+            // TODO(b/154349022): remove after releasing serialization fix
+            // Test packages prior to 52fb8df will fail with a not serializable exception.
+            // see https://cs.android.com/android/_/android/platform/tools/tradefederation/+/52fb8df
+            CLog.d("logAssociation is called but ignored intentionally");
+            return;
+        }
+        LogAssociationEventInfo info = new LogAssociationEventInfo(dataName, logFile);
+        printEvent(SubprocessTestResultsParser.StatusKeys.LOG_ASSOCIATION, info);
     }
 
-    /** A intentionally inop function to handle incompatibility problem in CTS 8.1 */
+    /* Legacy method compatible with TF/CTS 8.1+ (not called in 8). */
     @Override
     public void setLogSaver(ILogSaver logSaver) {
-        CLog.d("setLogSaver is called but ignored intentionally");
+        // ignore
     }
 }
diff --git a/src/com/android/tradefed/result/ResultForwarder.java b/src/com/android/tradefed/result/ResultForwarder.java
index 750b125..ba38e54 100644
--- a/src/com/android/tradefed/result/ResultForwarder.java
+++ b/src/com/android/tradefed/result/ResultForwarder.java
@@ -118,6 +118,21 @@
         }
     }
 
+    /** {@inheritDoc} */
+    @Override
+    public void invocationFailed(FailureDescription failure) {
+        for (ITestInvocationListener listener : mListeners) {
+            try {
+                listener.invocationFailed(failure);
+            } catch (RuntimeException e) {
+                CLog.e(
+                        "Exception while invoking %s#invocationFailed",
+                        listener.getClass().getName());
+                CLog.e(e);
+            }
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
diff --git a/src/com/android/tradefed/retry/ResultAggregator.java b/src/com/android/tradefed/retry/ResultAggregator.java
index 011fc26..42af517 100644
--- a/src/com/android/tradefed/retry/ResultAggregator.java
+++ b/src/com/android/tradefed/retry/ResultAggregator.java
@@ -119,6 +119,13 @@
 
     /** {@inheritDoc} */
     @Override
+    public void invocationFailed(FailureDescription failure) {
+        super.invocationFailed(failure);
+        mAllForwarder.invocationFailed(failure);
+    }
+
+    /** {@inheritDoc} */
+    @Override
     public void invocationEnded(long elapsedTime) {
         if (!mPureRunResultForAgg.isEmpty()) {
             for (String name : mPureRunResultForAgg.keySet()) {
@@ -250,6 +257,12 @@
     }
 
     @Override
+    public void testFailed(TestDescription test, FailureDescription failure) {
+        super.testFailed(test, failure);
+        mDetailedForwarder.testFailed(test, failure);
+    }
+
+    @Override
     public void testEnded(TestDescription test, long endTime, HashMap<String, Metric> testMetrics) {
         super.testEnded(test, endTime, testMetrics);
         mDetailedForwarder.testEnded(test, endTime, testMetrics);
diff --git a/src/com/android/tradefed/util/SubprocessTestResultsParser.java b/src/com/android/tradefed/util/SubprocessTestResultsParser.java
index db9f774..bc9d88f 100644
--- a/src/com/android/tradefed/util/SubprocessTestResultsParser.java
+++ b/src/com/android/tradefed/util/SubprocessTestResultsParser.java
@@ -576,8 +576,9 @@
                         try {
                             InvocationMetricLogger.addInvocationMetrics(key, Long.parseLong(val));
                         } catch (NumberFormatException e) {
-                            CLog.e("Key %s should have a number value, instead was: %s", key, val);
-                            CLog.e(e);
+                            CLog.d("Key %s doesn't have a number value, was: %s.", key, val);
+                            // If it's not a number then, let the string concatenate
+                            InvocationMetricLogger.addInvocationMetrics(key, val);
                         }
                     } else {
                         InvocationMetricLogger.addInvocationMetrics(key, val);
diff --git a/test_result_interfaces/com/android/tradefed/result/FailureDescription.java b/test_result_interfaces/com/android/tradefed/result/FailureDescription.java
index 23d4994..1acb558 100644
--- a/test_result_interfaces/com/android/tradefed/result/FailureDescription.java
+++ b/test_result_interfaces/com/android/tradefed/result/FailureDescription.java
@@ -33,6 +33,8 @@
     private @Nullable ActionInProgress mActionInProgress = null;
     // Optional: A free-formed text that help debugging the failure
     private @Nullable String mDebugHelpMessage = null;
+    // Optional: The exception that triggered the failure
+    private @Nullable Throwable mCause = null;
 
     FailureDescription() {}
 
@@ -75,6 +77,17 @@
         return mDebugHelpMessage;
     }
 
+    /** Sets the exception that caused the failure if any. */
+    public FailureDescription setCause(Throwable cause) {
+        mCause = cause;
+        return this;
+    }
+
+    /** Returns the exception that caused the failure. Can be null. */
+    public @Nullable Throwable getCause() {
+        return mCause;
+    }
+
     /** Sets the error message. */
     public void setErrorMessage(String errorMessage) {
         mErrorMessage = errorMessage;
diff --git a/tests/src/com/android/tradefed/device/metric/BaseDeviceMetricCollectorTest.java b/tests/src/com/android/tradefed/device/metric/BaseDeviceMetricCollectorTest.java
index ea2a0dc..fa4144a 100644
--- a/tests/src/com/android/tradefed/device/metric/BaseDeviceMetricCollectorTest.java
+++ b/tests/src/com/android/tradefed/device/metric/BaseDeviceMetricCollectorTest.java
@@ -112,7 +112,7 @@
         Mockito.verify(mMockListener, times(1)).testRunStopped(0L);
         Mockito.verify(mMockListener, times(1)).testRunEnded(0L, new HashMap<String, Metric>());
         Mockito.verify(mMockListener, times(1)).testModuleEnded();
-        Mockito.verify(mMockListener, times(1)).invocationFailed(Mockito.any());
+        Mockito.verify(mMockListener, times(1)).invocationFailed(Mockito.<Throwable>any());
         Mockito.verify(mMockListener, times(1)).invocationEnded(0L);
 
         Assert.assertSame(mMockListener, mBase.getInvocationListener());
@@ -198,7 +198,7 @@
         Mockito.verify(mMockListener, times(1)).testRunFailed("test run failed");
         Mockito.verify(mMockListener, times(1)).testRunStopped(0L);
         Mockito.verify(mMockListener, times(1)).testRunEnded(0L, new HashMap<String, Metric>());
-        Mockito.verify(mMockListener, times(1)).invocationFailed(Mockito.any());
+        Mockito.verify(mMockListener, times(1)).invocationFailed(Mockito.<Throwable>any());
         Mockito.verify(mMockListener, times(1)).invocationEnded(0L);
 
         Assert.assertSame(mMockListener, mBase.getInvocationListener());
diff --git a/tests/src/com/android/tradefed/invoker/TestInvocationMultiTest.java b/tests/src/com/android/tradefed/invoker/TestInvocationMultiTest.java
index 98ed019..9257c88 100644
--- a/tests/src/com/android/tradefed/invoker/TestInvocationMultiTest.java
+++ b/tests/src/com/android/tradefed/invoker/TestInvocationMultiTest.java
@@ -193,7 +193,7 @@
         mMockTestListener.invocationStarted(mContext);
         EasyMock.expect(mMockTestListener.getSummary()).andReturn(null);
         mMockLogSaver.invocationStarted(mContext);
-        mMockTestListener.invocationFailed(EasyMock.anyObject());
+        mMockTestListener.invocationFailed(EasyMock.<Throwable>anyObject());
         mMockTestListener.testLog(EasyMock.anyObject(), EasyMock.anyObject(), EasyMock.anyObject());
         EasyMock.expect(
                         mMockLogSaver.saveLogData(
@@ -359,7 +359,7 @@
         mMockTestListener.invocationStarted(mContext);
         EasyMock.expect(mMockTestListener.getSummary()).andReturn(null);
         mMockLogSaver.invocationStarted(mContext);
-        mMockTestListener.invocationFailed(EasyMock.anyObject());
+        mMockTestListener.invocationFailed(EasyMock.<Throwable>anyObject());
         mMockTestListener.testLog(EasyMock.anyObject(), EasyMock.anyObject(), EasyMock.anyObject());
         EasyMock.expect(
                         mMockLogSaver.saveLogData(
@@ -446,7 +446,7 @@
         mMockTestListener.invocationStarted(mContext);
         EasyMock.expect(mMockTestListener.getSummary()).andReturn(null);
         mMockLogSaver.invocationStarted(mContext);
-        mMockTestListener.invocationFailed(EasyMock.anyObject());
+        mMockTestListener.invocationFailed(EasyMock.<Throwable>anyObject());
         mMockTestListener.testLog(EasyMock.anyObject(), EasyMock.anyObject(), EasyMock.anyObject());
         EasyMock.expect(
                         mMockLogSaver.saveLogData(
diff --git a/tests/src/com/android/tradefed/invoker/TestInvocationTest.java b/tests/src/com/android/tradefed/invoker/TestInvocationTest.java
index 48a2590..8f4044c 100644
--- a/tests/src/com/android/tradefed/invoker/TestInvocationTest.java
+++ b/tests/src/com/android/tradefed/invoker/TestInvocationTest.java
@@ -1073,8 +1073,8 @@
         // invocationFailed
         if (!status.equals(InvocationStatus.SUCCESS)) {
             if (stubFailures) {
-                mMockTestListener.invocationFailed(EasyMock.anyObject());
-                mMockSummaryListener.invocationFailed(EasyMock.anyObject());
+                mMockTestListener.invocationFailed(EasyMock.<Throwable>anyObject());
+                mMockSummaryListener.invocationFailed(EasyMock.<Throwable>anyObject());
             } else {
                 mMockTestListener.invocationFailed(EasyMock.eq(throwable));
                 mMockSummaryListener.invocationFailed(EasyMock.eq(throwable));
diff --git a/tests/src/com/android/tradefed/retry/ResultAggregatorTest.java b/tests/src/com/android/tradefed/retry/ResultAggregatorTest.java
index e371896..ce906cc 100644
--- a/tests/src/com/android/tradefed/retry/ResultAggregatorTest.java
+++ b/tests/src/com/android/tradefed/retry/ResultAggregatorTest.java
@@ -122,7 +122,7 @@
                 EasyMock.anyLong(),
                 EasyMock.<HashMap<String, Metric>>anyObject());
         mDetailedListener.testStarted(EasyMock.eq(test2), EasyMock.anyLong());
-        mDetailedListener.testFailed(test2, "I failed. retry me.");
+        mDetailedListener.testFailed(test2, FailureDescription.create("I failed. retry me."));
         mDetailedListener.logAssociation("test2-before-log", test2LogBefore);
         mDetailedListener.logAssociation("test2-after-log", test2LogAfter);
         mDetailedListener.testEnded(
@@ -185,7 +185,7 @@
         mAggregator.testEnded(test1, new HashMap<String, Metric>());
         mAggregator.testStarted(test2);
         mAggregator.logAssociation("test2-before-log", test2LogBefore);
-        mAggregator.testFailed(test2, "I failed. retry me.");
+        mAggregator.testFailed(test2, FailureDescription.create("I failed. retry me."));
         mAggregator.logAssociation("test2-after-log", test2LogAfter);
         mAggregator.testEnded(test2, new HashMap<String, Metric>());
         mAggregator.logAssociation("test-run1-before-log", testRun1LogBefore);