TestRunResult aggregation was incomplete, fix it

The incomplete aggregation results in testRunFailure not
being carried to final reporters.
We are not reporting properly failures and potentially
messing up the state of CTS retry too.

Test: unit tests
atest_tradefed.sh run commandAndExit template/local_min --template:map test=atest --include-filter HelloWorldTests --log-level VERBOSE --template:map reporters=google/template/reporters/atest --sponge-label atest --log-level-display verbose
Bug: 110831320

Change-Id: I6b0e0dd1b7ac88d3dac705b31e7bfd54e0eff07c
diff --git a/src/com/android/tradefed/result/TestRunResult.java b/src/com/android/tradefed/result/TestRunResult.java
index c835eb5..37db2a3 100644
--- a/src/com/android/tradefed/result/TestRunResult.java
+++ b/src/com/android/tradefed/result/TestRunResult.java
@@ -20,6 +20,8 @@
 import com.android.tradefed.metrics.proto.MetricMeasurement.Metric;
 import com.android.tradefed.util.proto.TfMetricProtoUtil;
 
+import com.google.common.base.Joiner;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -373,7 +375,13 @@
         Map<String, LogFile> finalRunLoggedFiles = new HashMap<>();
         Map<TestDescription, TestResult> finalTestResults =
                 new HashMap<TestDescription, TestResult>();
-
+        // Keep track of if one of the run attempt failed.
+        boolean isFailed = false;
+        List<String> runErrors = new ArrayList<>();
+        // Keep track of if one of the run is not complete
+        boolean isComplete = true;
+        // Keep track of elapsed time
+        long elapsedTime = 0L;
         for (TestRunResult eachRunResult : testRunResults) {
             // Check all mTestRunNames are the same.
             if (!testRunName.equals(eachRunResult.getName())) {
@@ -383,6 +391,15 @@
                                         + "different (%s, %s)",
                                 testRunName, eachRunResult.getName()));
             }
+            elapsedTime += eachRunResult.getElapsedTime();
+            if (eachRunResult.isRunFailure()) {
+                // if one of the run fail for now consider the aggregate a failure.
+                runErrors.add(eachRunResult.getRunFailureMessage());
+                isFailed = true;
+            }
+            if (!eachRunResult.isRunComplete()) {
+                isComplete = false;
+            }
             // Keep the last TestRunResult's RunMetrics, ProtoMetrics and logFiles.
             // TODO: Currently we keep a single item when multiple TestRunResult have the same
             // keys. In the future, we may want to improve this logic.
@@ -430,6 +447,15 @@
         finalRunResult.mRunProtoMetrics = finalRunProtoMetrics;
         finalRunResult.mRunLoggedFiles = finalRunLoggedFiles;
         finalRunResult.mTestResults = finalTestResults;
+        // Report completion status
+        if (isFailed) {
+            finalRunResult.mRunFailureError = Joiner.on("\n").join(runErrors);
+        } else {
+            finalRunResult.mRunFailureError = null;
+        }
+        finalRunResult.mIsRunComplete = isComplete;
+        // Report total elapsed times
+        finalRunResult.mElapsedTime = elapsedTime;
         return finalRunResult;
     }
 }
diff --git a/tests/src/com/android/tradefed/result/TestRunResultTest.java b/tests/src/com/android/tradefed/result/TestRunResultTest.java
index b5fd406..71ac0ba 100644
--- a/tests/src/com/android/tradefed/result/TestRunResultTest.java
+++ b/tests/src/com/android/tradefed/result/TestRunResultTest.java
@@ -360,4 +360,78 @@
         assertTrue(result.getRunLoggedFiles().containsKey("run log"));
         assertEquals("path2", result.getRunLoggedFiles().get("run log").getPath());
     }
+
+    /** Ensure that the merging logic among multiple testRunResults for run failures is correct. */
+    @Test
+    public void testMergeRetriedRunResults_runFailures() {
+        // Test Setup.
+        TestDescription testcase = new TestDescription("Foo", "foo");
+        TestRunResult result1 = new TestRunResult();
+        TestRunResult result2 = new TestRunResult();
+        TestRunResult result3 = new TestRunResult();
+        // Mimic the ModuleDefinition run.
+        result1.testRunStarted("fake run", 1);
+        result1.testStarted(testcase);
+        result1.testEnded(testcase, new HashMap<String, Metric>());
+        result1.testRunEnded(0, new HashMap<String, Metric>());
+
+        result2.testRunStarted("fake run", 1);
+        result2.testStarted(testcase);
+        result2.testEnded(testcase, new HashMap<String, Metric>());
+        result2.testRunFailed("Second run failed.");
+        result2.testRunEnded(0, new HashMap<String, Metric>());
+
+        result3.testRunStarted("fake run", 1);
+        result3.testStarted(testcase);
+        result3.testEnded(testcase, new HashMap<String, Metric>());
+        result3.testRunFailed("Third run failed.");
+        result3.testRunEnded(0, new HashMap<String, Metric>());
+
+        List<TestRunResult> testResultList =
+                new ArrayList<>(Arrays.asList(result1, result2, result3));
+        TestRunResult result = TestRunResult.merge(testResultList);
+
+        // Verify result.
+        assertEquals("fake run", result.getName());
+        assertTrue(result.isRunFailure());
+        assertTrue(result.isRunComplete());
+        assertEquals("Second run failed.\nThird run failed.", result.getRunFailureMessage());
+    }
+
+    /** Ensure that the merging logic among multiple testRunResults for one incomplete run. */
+    @Test
+    public void testMergeRetriedRunResults_incompleteRun() {
+        // Test Setup.
+        TestDescription testcase = new TestDescription("Foo", "foo");
+        TestRunResult result1 = new TestRunResult();
+        TestRunResult result2 = new TestRunResult();
+        TestRunResult result3 = new TestRunResult();
+        // Mimic the ModuleDefinition run.
+        result1.testRunStarted("fake run", 1);
+        result1.testStarted(testcase);
+        result1.testEnded(testcase, new HashMap<String, Metric>());
+        result1.testRunEnded(0, new HashMap<String, Metric>());
+
+        result2.testRunStarted("fake run", 1);
+        result2.testStarted(testcase);
+        result2.testEnded(testcase, new HashMap<String, Metric>());
+        // Missing testRunEnded
+
+        result3.testRunStarted("fake run", 1);
+        result3.testStarted(testcase);
+        result3.testEnded(testcase, new HashMap<String, Metric>());
+        result3.testRunFailed("Third run failed.");
+        result3.testRunEnded(0, new HashMap<String, Metric>());
+
+        List<TestRunResult> testResultList =
+                new ArrayList<>(Arrays.asList(result1, result2, result3));
+        TestRunResult result = TestRunResult.merge(testResultList);
+
+        // Verify result.
+        assertEquals("fake run", result.getName());
+        assertTrue(result.isRunFailure());
+        // One of the run was incomplete so we report incomplete.
+        assertFalse(result.isRunComplete());
+        assertEquals("Third run failed.", result.getRunFailureMessage());
+    }
 }