Revert 47917 - Make outofprocess tests output correct XML file

 Disable XML output for children
 Output XML file with failed, disabled and timing information in the
  parent launcher process.

BUG=40473,42781
TEST=run the browser_tests (or other tests that uses outofproctest runner) with 1) gtest_output=xml 2) gtest_output=xml:test.xml 3) gtest_output=xml:/tmp/ and see if the output XML file (test_detail.xml for 1), test.xml for 2), /tmp/browser_tests.xml for 3)) contains correct information.

Review URL: http://codereview.chromium.org/2071001

TBR=kinuko@chromium.org
Review URL: http://codereview.chromium.org/2119016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47920 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 75 insertions, 202 deletions

diff --git a/chrome/test/test_launcher/out_of_proc_test_runner.cc b/chrome/test/test_launcher/out_of_proc_test_runner.cc
index b5e53570..443401b 100644
--- a/chrome/test/test_launcher/out_of_proc_test_runner.cc
+++ b/chrome/test/test_launcher/out_of_proc_test_runner.cc
@@ -16,7 +16,6 @@
 namespace {
 
 const char kGTestListTestsFlag[] = "gtest_list_tests";
-const char kGTestOutputFlag[] = "gtest_output";
 const char kGTestHelpFlag[]   = "gtest_help";
 const char kSingleProcessTestsFlag[]   = "single_process";
 const char kSingleProcessTestsAndChromeFlag[]   = "single-process";
@@ -43,17 +42,12 @@ class OutOfProcTestRunner : public tests::TestRunner {
   // Returns true if the test succeeded, false if it failed.
   bool RunTest(const std::string& test_name) {
     const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
-    // Construct the new command line.  Strip out gtest_output flag if
-    // it has been given because otherwise each test outputs the same file
-    // over and over overriding the previous one every time.
-    // We will generate the final output file later in RunTests().
-    CommandLine new_cmd_line(cmd_line->GetProgram());
-    CommandLine::SwitchMap switches = cmd_line->GetSwitches();
-    switches.erase(kGTestOutputFlag);
-    for (CommandLine::SwitchMap::const_iterator iter = switches.begin();
-         iter != switches.end(); ++iter) {
-      new_cmd_line.AppendSwitchWithValue((*iter).first, (*iter).second);
-    }
+#if defined(OS_WIN)
+    CommandLine new_cmd_line =
+        CommandLine::FromString(cmd_line->command_line_string());
+#else
+    CommandLine new_cmd_line(cmd_line->argv());
+#endif
 
     // Always enable disabled tests.  This method is not called with disabled
     // tests unless this flag was specified to the browser test executable.
diff --git a/chrome/test/test_launcher/test_runner.cc b/chrome/test/test_launcher/test_runner.cc
index ea496ed..5c3cf25 100644
--- a/chrome/test/test_launcher/test_runner.cc
+++ b/chrome/test/test_launcher/test_runner.cc
@@ -4,173 +4,71 @@
 
 #include "chrome/test/test_launcher/test_runner.h"
 
-#include <iostream>
 #include <vector>
 
 #include "base/command_line.h"
-#include "base/file_util.h"
-#include "base/linked_ptr.h"
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
-#include "base/time.h"
-#include "net/base/escape.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const char kGTestListTestsFlag[] = "gtest_list_tests";
 const char kGTestRunDisabledTestsFlag[] = "gtest_also_run_disabled_tests";
-const char kGTestOutputFlag[] = "gtest_output";
-const char kGTestFilterFlag[] = "gtest_filter";
-
-// The default output file for XML output.
-static const FilePath::StringType kDefaultOutputFile = "test_detail.xml";
-
-// A helper class to output results.
-// Note: as currently XML is the only supported format by gtest, we don't
-// check output format (e.g. "xml:" prefix) here and output an XML file
-// unconditionally.
-// Note: we don't output per-test-case or total summary info like
-// total failed_test_count, disabled_test_count, elapsed_time and so on.
-// Only each test (testcase element in the XML) will have the correct
-// failed/disabled/elapsed_time information. Each test won't include
-// detailed failure messages either.
-class ResultsPrinter {
- public:
-  ResultsPrinter(const CommandLine& command_line);
-  ~ResultsPrinter();
-  void OnTestCaseStart(const char* name, int test_count) const;
-  void OnTestCaseEnd() const;
-  void OnTestEnd(const char* name, const char* case_name, bool run,
-                 bool failure, double elapsed_time) const;
- private:
-  FILE* out_;
-};
-
-ResultsPrinter::ResultsPrinter(const CommandLine& command_line) : out_(NULL) {
-  if (!command_line.HasSwitch(kGTestOutputFlag))
-    return;
-  std::string flag = command_line.GetSwitchValueASCII(kGTestOutputFlag);
-  size_t colon_pos = flag.find(':');
-  FilePath path;
-  if (colon_pos != std::string::npos) {
-    FilePath flag_path = command_line.GetSwitchValuePath(kGTestOutputFlag);
-    FilePath::StringType path_string = flag_path.value();
-    path = FilePath(path_string.substr(colon_pos + 1));
-    // If the given path ends with '/', consider it is a directory.
-    // Note: This does NOT check that a directory (or file) actually exists
-    // (the behavior is same as what gtest does).
-    if (file_util::EndsWithSeparator(path)) {
-      FilePath executable = command_line.GetProgram().BaseName();
-      path = path.Append(executable.ReplaceExtension(
-          FilePath::StringType("xml")));
-    }
-  }
-  if (path.value().empty())
-    path = FilePath(kDefaultOutputFile);
-  out_ = file_util::OpenFile(path, "w");
-  if (!out_) {
-    LOG(ERROR) << "Cannot open output file: "
-               << path.value() << "." << std::endl;
-    return;
-  }
-  fprintf(out_, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
-  fprintf(out_, "<testsuites name=\"AllTests\" tests=\"\" failures=\"\""
-          " disabled=\"\" errors=\"\" time=\"\">\n");
-}
-
-ResultsPrinter::~ResultsPrinter() {
-  if (!out_)
-    return;
-  fprintf(out_, "</testsuites>\n");
-  fclose(out_);
-}
-
-void ResultsPrinter::OnTestCaseStart(const char* name, int test_count) const {
-  if (!out_)
-    return;
-  fprintf(out_, "  <testsuite name=\"%s\" tests=\"%d\" failures=\"\""
-          " disabled=\"\" errors=\"\" time=\"\">\n", name, test_count);
-}
-
-void ResultsPrinter::OnTestCaseEnd() const {
-  if (!out_)
-    return;
-  fprintf(out_, "  </testsuite>\n");
-}
 
-void ResultsPrinter::OnTestEnd(const char* name, const char* case_name,
-                               bool run, bool failure,
-                               double elapsed_time) const {
-  if (!out_)
-    return;
-  fprintf(out_, "    <testcase name=\"%s\" status=\"%s\" time=\"%.3f\""
-          " classname=\"%s\"",
-          name, run ? "run" : "notrun", elapsed_time / 1000.0, case_name);
-  if (!failure) {
-    fprintf(out_, " />\n");
-    return;
-  }
-  fprintf(out_, ">\n");
-  fprintf(out_, "      <failure message=\"\" type=\"\"></failure>\n");
-  fprintf(out_, "    </testcase>\n");
-}
-
-class TestCasePrinterHelper {
- public:
-  TestCasePrinterHelper(const ResultsPrinter& printer, const char* name,
-                        int total_test_count)
-      : printer_(printer) {
-    printer_.OnTestCaseStart(name, total_test_count);
-  }
-  ~TestCasePrinterHelper() {
-    printer_.OnTestCaseEnd();
-  }
- private:
-  const ResultsPrinter& printer_;
-};
-
-// For a basic pattern matching for gtest_filter options.  (Copied from
-// gtest.cc, see the comment below and http://crbug.com/44497)
-bool PatternMatchesString(const char *pattern, const char *str) {
-  switch (*pattern) {
-    case '\0':
-    case ':':  // Either ':' or '\0' marks the end of the pattern.
-      return *str == '\0';
-    case '?':  // Matches any single character.
-      return *str != '\0' && PatternMatchesString(pattern + 1, str + 1);
-    case '*':  // Matches any string (possibly empty) of characters.
-      return (*str != '\0' && PatternMatchesString(pattern, str + 1)) ||
-          PatternMatchesString(pattern + 1, str);
-    default:  // Non-special character.  Matches itself.
-      return *pattern == *str &&
-          PatternMatchesString(pattern + 1, str + 1);
-  }
-}
-
-// TODO(phajdan.jr): Avoid duplicating gtest code. (http://crbug.com/44497)
-// For basic pattern matching for gtest_filter options.  (Copied from
-// gtest.cc)
-bool MatchesFilter(const std::string& name, const std::string& filter) {
-  const char *cur_pattern = filter.c_str();
-  for (;;) {
-    if (PatternMatchesString(cur_pattern, name.c_str())) {
-      return true;
+// Retrieves the list of tests to run by running gtest with the
+// --gtest_list_tests flag in a forked process and parsing its output.
+// |command_line| should contain the command line used to start the browser
+// test launcher, it is expected that it does not contain the
+// --gtest_list_tests flag already.
+// Note: we cannot implement this in-process for InProcessTestRunner as GTest
+// prints to the stdout and there are no good way of temporarily redirecting
+// outputs.
+bool GetTestList(const CommandLine& command_line,
+                 std::vector<std::string>* test_list) {
+  DCHECK(!command_line.HasSwitch(kGTestListTestsFlag));
+
+  // Run ourselves with the --gtest_list_tests option and read the output.
+  CommandLine new_command_line(command_line);
+  new_command_line.AppendSwitch(kGTestListTestsFlag);
+  std::string output;
+  if (!base::GetAppOutput(new_command_line, &output))
+    return false;
+
+  // The output looks like:
+  // TestCase.
+  //   Test1
+  //   Test2
+  // OtherTestCase.
+  //   FooTest
+  // ...
+  std::vector<std::string> lines;
+  SplitString(output, '\n', &lines);
+
+  std::string test_case;
+  for (std::vector<std::string>::const_iterator iter = lines.begin();
+       iter != lines.end(); ++iter) {
+    std::string line = *iter;
+
+    // Ignore empty and metadata lines (like "YOU HAVE x FLAKY TESTS").
+    if (line.empty() || line.find(' ') != std::string::npos)
+      continue;
+
+    if (line[line.size() - 1] == '.') {
+      // This is a new test case.
+      test_case = line;
+      continue;
     }
 
-    // Finds the next pattern in the filter.
-    cur_pattern = strchr(cur_pattern, ':');
-
-    // Returns if no more pattern can be found.
-    if (cur_pattern == NULL) {
-      return false;
-    }
+    if (!command_line.HasSwitch(kGTestRunDisabledTestsFlag) &&
+        line.find("DISABLED") != std::string::npos)
+      continue;  // Skip disabled tests.
 
-    // Skips the pattern separater (the ':' character).
-    cur_pattern++;
+    // We are dealing with a test.
+    test_list->push_back(test_case + line);
   }
+  return true;
 }
 
 }  // namespace
@@ -188,50 +86,31 @@ bool RunTests(const TestRunnerFactory& test_runner_factory) {
 
   DCHECK(!command_line->HasSwitch(kGTestListTestsFlag));
 
-  testing::UnitTest* const unit_test = testing::UnitTest::GetInstance();
+  // First let's get the list of tests we need to run.
+  std::vector<std::string> test_list;
+  if (!GetTestList(*command_line, &test_list)) {
+    printf("Failed to retrieve the tests to run.\n");
+    return false;
+  }
 
-  std::string filter_string = command_line->GetSwitchValueASCII(
-      kGTestFilterFlag);
+  if (test_list.empty()) {
+    printf("No tests to run.\n");
+    return false;
+  }
 
   int test_run_count = 0;
   std::vector<std::string> failed_tests;
-
-  ResultsPrinter printer(*command_line);
-  for (int i = 0; i < unit_test->total_test_case_count(); ++i) {
-    const testing::TestCase* test_case = unit_test->GetTestCase(i);
-    TestCasePrinterHelper helper(printer, test_case->name(),
-                                 test_case->total_test_count());
-    for (int j = 0; j < test_case->total_test_count(); ++j) {
-      const testing::TestInfo* test_info = test_case->GetTestInfo(j);
-      // Skip disabled tests.
-      if (std::string(test_info->name()).find("DISABLED") == 0 &&
-          !command_line->HasSwitch(kGTestRunDisabledTestsFlag)) {
-        printer.OnTestEnd(test_info->name(), test_case->name(),
-                          false, false, 0);
-        continue;
-      }
-      std::string test_name = test_info->test_case_name();
-      test_name.append(".");
-      test_name.append(test_info->name());
-      // Skip the test that doesn't match the filter string (if given).
-      if (filter_string.size() && !MatchesFilter(test_name, filter_string)) {
-        printer.OnTestEnd(test_info->name(), test_case->name(),
-                          false, false, 0);
-        continue;
-      }
-      base::Time start_time = base::Time::Now();
-      scoped_ptr<TestRunner> test_runner(
-          test_runner_factory.CreateTestRunner());
-      if (!test_runner.get() || !test_runner->Init())
-        return false;
-      ++test_run_count;
-      if (!test_runner->RunTest(test_name.c_str())) {
-        failed_tests.push_back(test_name);
-        printer.OnTestEnd(test_info->name(), test_case->name(), true, true,
-                          (base::Time::Now() - start_time).InMillisecondsF());
-      } else {
-        printer.OnTestEnd(test_info->name(), test_case->name(), true, false,
-                          (base::Time::Now() - start_time).InMillisecondsF());
+  for (std::vector<std::string>::const_iterator iter = test_list.begin();
+       iter != test_list.end(); ++iter) {
+    std::string test_name = *iter;
+    scoped_ptr<TestRunner> test_runner(test_runner_factory.CreateTestRunner());
+    if (!test_runner.get() || !test_runner->Init())
+      return false;
+    test_run_count++;
+    if (!test_runner->RunTest(test_name.c_str())) {
+      if (std::find(failed_tests.begin(), failed_tests.end(), test_name) ==
+          failed_tests.end()) {
+        failed_tests.push_back(*iter);
       }
     }
   }