GTTF: Test launcher - correctly handle non-zero exit code with all tests passing

base/test/launcher/unit_test_launcher.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/test/launcher/unit_test_launcher.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 210 matching lines @@
              Unretained(this),
              callback_state));
   }
 
   void GTestCallback(const GTestCallbackState& callback_state,
                      int exit_code,
                      const TimeDelta& elapsed_time,
                      bool was_timeout,
                      const std::string& output) {
     DCHECK(thread_checker_.CalledOnValidThread());
-    std::vector<std::string> tests_to_relaunch_after_interruption;
+    std::vector<std::string> tests_to_relaunch;
     ProcessTestResults(callback_state.test_launcher,
                        callback_state.test_names,
                        callback_state.output_file,
                        output,
                        exit_code,
                        was_timeout,
-                       &tests_to_relaunch_after_interruption);
+                       &tests_to_relaunch);
 
-    RunBatch(callback_state.test_launcher,
-             tests_to_relaunch_after_interruption);
+    // Relaunch requested tests in parallel, but only use single
+    // test per batch for more precise results (crashes, test passes
+    // but non-zero exit codes etc).
+    for (size_t i = 0; i < tests_to_relaunch.size(); i++) {
+      std::vector<std::string> batch;
+      batch.push_back(tests_to_relaunch[i]);
+      RunBatch(callback_state.test_launcher, batch);
+    }
 
     // The temporary file's directory is also temporary.
     DeleteFile(callback_state.output_file.DirName(), true);
   }
 
   void SerialGTestCallback(const GTestCallbackState& callback_state,
                            const std::vector<std::string>& test_names,
                            int exit_code,
                            const TimeDelta& elapsed_time,
                            bool was_timeout,
                            const std::string& output) {
     DCHECK(thread_checker_.CalledOnValidThread());
-    std::vector<std::string> tests_to_relaunch_after_interruption;
+    std::vector<std::string> tests_to_relaunch;
     bool called_any_callbacks =
         ProcessTestResults(callback_state.test_launcher,
                            callback_state.test_names,
                            callback_state.output_file,
                            output,
                            exit_code,
                            was_timeout,
-                           &tests_to_relaunch_after_interruption);
+                           &tests_to_relaunch);
 
     // There is only one test, there cannot be other tests to relaunch
     // due to a crash.
-    DCHECK(tests_to_relaunch_after_interruption.empty());
+    DCHECK(tests_to_relaunch.empty());
 
     // There is only one test, we should have called back with its result.
     DCHECK(called_any_callbacks);
 
     // The temporary file's directory is also temporary.
     DeleteFile(callback_state.output_file.DirName(), true);
 
     MessageLoop::current()->PostTask(
         FROM_HERE,
         Bind(&UnitTestLauncherDelegate::RunSerially,
              Unretained(this),
              callback_state.test_launcher,
              test_names));
   }
 
   static bool ProcessTestResults(
       TestLauncher* test_launcher,
       const std::vector<std::string>& test_names,
       const base::FilePath& output_file,
       const std::string& output,
       int exit_code,
       bool was_timeout,
-      std::vector<std::string>* tests_to_relaunch_after_interruption) {
+      std::vector<std::string>* tests_to_relaunch) {
     std::vector<TestResult> test_results;
     bool crashed = false;
     bool have_test_results =
         ProcessGTestOutput(output_file, &test_results, &crashed);
 
     bool called_any_callback = false;
 
     if (have_test_results) {
       // TODO(phajdan.jr): Check for duplicates and mismatches between
       // the results we got from XML file and tests we intended to run.
       std::map<std::string, TestResult> results_map;
       for (size_t i = 0; i < test_results.size(); i++)
         results_map[test_results[i].full_name] = test_results[i];
 
       bool had_interrupted_test = false;
 
+      // Results to be reported back to the test launcher.
+      std::vector<TestResult> final_results;
+
       for (size_t i = 0; i < test_names.size(); i++) {
         if (ContainsKey(results_map, test_names[i])) {
           TestResult test_result = results_map[test_names[i]];
           if (test_result.status == TestResult::TEST_CRASH) {
             had_interrupted_test = true;
 
             if (was_timeout) {
               // Fix up the test status: we forcibly kill the child process
               // after the timeout, so from XML results it looks just like
               // a crash.
               test_result.status = TestResult::TEST_TIMEOUT;
             }
           } else if (test_result.status == TestResult::TEST_SUCCESS ||
                      test_result.status == TestResult::TEST_FAILURE) {
             // We run multiple tests in a batch with a timeout applied
             // to the entire batch. It is possible that with other tests
             // running quickly some tests take longer than the per-test timeout.
             // For consistent handling of tests independent of order and other
             // factors, mark them as timing out.
             if (test_result.elapsed_time >
                 TestTimeouts::test_launcher_timeout()) {
               test_result.status = TestResult::TEST_TIMEOUT;
             }
           }
           test_result.output_snippet =
               GetTestOutputSnippet(test_result, output);
-          test_launcher->OnTestFinished(test_result);
-          called_any_callback = true;
+          final_results.push_back(test_result);
         } else if (had_interrupted_test) {
-          tests_to_relaunch_after_interruption->push_back(test_names[i]);
+          tests_to_relaunch->push_back(test_names[i]);
         } else {
           // TODO(phajdan.jr): Explicitly pass the info that the test didn't
           // run for a mysterious reason.
           LOG(ERROR) << "no test result for " << test_names[i];
           TestResult test_result;
           test_result.full_name = test_names[i];
           test_result.status = TestResult::TEST_UNKNOWN;
           test_result.output_snippet =
               GetTestOutputSnippet(test_result, output);
-          test_launcher->OnTestFinished(test_result);
-          called_any_callback = true;
+          final_results.push_back(test_result);
         }
       }
 
       // TODO(phajdan.jr): Handle the case where processing XML output
       // indicates a crash but none of the test results is marked as crashing.
 
-      // TODO(phajdan.jr): Handle the case where the exit code is non-zero
-      // but results file indicates that all tests passed (e.g. crash during
-      // shutdown).
+      if (final_results.empty())
+        return false;
+
+      bool has_non_success_test = false;
+      for (size_t i = 0; i < final_results.size(); i++) {
+        if (final_results[i].status != TestResult::TEST_SUCCESS) {
+          has_non_success_test = true;
+          break;
+        }
+      }
+
+      if (!has_non_success_test && exit_code != 0) {
+        // This is a bit surprising case: all tests are marked as successful,
+        // but the exit code was not zero. This can happen e.g. under memory
+        // tools that report leaks this way.
+
+        if (final_results.size() == 1) {
+          // Easy case. One test only so we know the non-zero exit code
+          // was caused by that one test.
+          final_results[0].status = TestResult::TEST_FAILURE_ON_EXIT;
+        } else {
+          // Harder case. Discard the results and request relaunching all
+          // tests without batching. This will trigger above branch on
+          // relaunch leading to more precise results.
+          LOG(WARNING) << "Not sure which test caused non-zero exit code, "
+                       << "relaunching all of them without batching.";
+
+          for (size_t i = 0; i < final_results.size(); i++)
+            tests_to_relaunch->push_back(final_results[i].full_name);
+
+          return false;
+        }
+      }
+
+      for (size_t i = 0; i < final_results.size(); i++) {
+        // Fix the output snippet after possible changes to the test result.
+        final_results[i].output_snippet =
+            GetTestOutputSnippet(final_results[i], output);
+        test_launcher->OnTestFinished(final_results[i]);
+        called_any_callback = true;
+      }
     } else {
       fprintf(stdout,
               "Failed to get out-of-band test success data, "
               "dumping full stdio below:\n%s\n",
               output.c_str());
       fflush(stdout);
 
       // We do not have reliable details about test results (parsing test
       // stdout is known to be unreliable), apply the executable exit code
       // to all tests.
@@ skipping 77 matching lines @@
 
   fprintf(stdout,
           "Tests took %" PRId64 " seconds.\n",
           (base::TimeTicks::Now() - start_time).InSeconds());
   fflush(stdout);
 
   return (success ? 0 : 1);
 }
 
 }  // namespace base