Remove a TODO to convert bool failed, bool failure_ignored to an enum.

chrome/test/out_of_proc_test_runner.cc

Index: chrome/test/out_of_proc_test_runner.cc
diff --git a/chrome/test/out_of_proc_test_runner.cc b/chrome/test/out_of_proc_test_runner.cc
index 1d2bb10e4d9c5492400b6357a33ebd5b764c6a91..76deb0b0fea1d78d844a1ec87d82f2cce58161e2 100644
--- a/chrome/test/out_of_proc_test_runner.cc
+++ b/chrome/test/out_of_proc_test_runner.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -85,14 +85,19 @@ static const FilePath::CharType kDefaultOutputFile[] = FILE_PATH_LITERAL(
 // detailed failure messages either.
 class ResultsPrinter {
  public:
+  enum ResultMask {

[Paweł Hajdan Jr., 2011/01/11 16:33:11]

Why do we need a mask? It should be possible to just use an enum:

RESULT_SUCCESS,
RESULT_FAILURE,
RESULT_IGNORED_FAILURE.

We don't need to OR together RESULT_FAILURE and RESULT_IGNORED_FAILURE.

In fact, I think we should add |run| to the mix, resulting in

RESULT_NOT_RUN,
RESULT_SUCCESS,
RESULT_FAILURE,
RESULT_IGNORED_FAILURE

[tfarina, 2011/01/11 16:36:49]

See line 419.
Initially I went to this way, but because of the possibility of having true and
false, I had to use a mask.
 
I needed the OR because of the condition at line 419.

+    RESULT_NONE            = 0,
+    RESULT_FAILED          = 1 << 0,
+    RESULT_FAILURE_IGNORED = 1 << 1,
+  };
+
   explicit ResultsPrinter(const CommandLine& command_line);
   ~ResultsPrinter();
   void OnTestCaseStart(const char* name, int test_count) const;
   void OnTestCaseEnd() const;
 
-  // TODO(phajdan.jr): Convert bool failed, bool failure_ignored to an enum.
   void OnTestEnd(const char* name, const char* case_name, bool run,
-                 bool failed, bool failure_ignored, double elapsed_time) const;
+                 int result_mask, double elapsed_time) const;
  private:
   FILE* out_;
 
@@ -161,21 +166,20 @@ void ResultsPrinter::OnTestCaseEnd() const {
 void ResultsPrinter::OnTestEnd(const char* name,
                                const char* case_name,
                                bool run,
-                               bool failed,
-                               bool failure_ignored,
+                               int result_mask,
                                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 (!failed) {
+  if (!(result_mask & FAILED)) {
     fprintf(out_, " />\n");
     return;
   }
   fprintf(out_, ">\n");
   fprintf(out_, "      <failure message=\"\" type=\"\"%s></failure>\n",
-          failure_ignored ? " ignored=\"true\"" : "");
+          (result_mask & FAILURE_IGNORED) ? " ignored=\"true\"" : "");
   fprintf(out_, "    </testcase>\n");
 }
 
@@ -379,8 +383,8 @@ bool RunTests() {
       // 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, false, 0);
+        printer.OnTestEnd(test_info->name(), test_case->name(), false,
+                          ResultsPrinter::RESULT_NONE, 0);
         continue;
       }
       std::string test_name = test_info->test_case_name();
@@ -390,8 +394,8 @@ bool RunTests() {
       if ((!positive_filter.empty() &&
            !MatchesFilter(test_name, positive_filter)) ||
           MatchesFilter(test_name, negative_filter)) {
-        printer.OnTestEnd(test_info->name(), test_case->name(),
-                          false, false, false, 0);
+        printer.OnTestEnd(test_info->name(), test_case->name(), false,
+                          ResultsPrinter::RESULT_NONE, 0);
         continue;
       }
       base::Time start_time = base::Time::Now();
@@ -399,8 +403,8 @@ bool RunTests() {
       int exit_code = RunTest(test_name);
       if (exit_code == 0) {
         // Test passed.
-        printer.OnTestEnd(test_info->name(), test_case->name(), true, false,
-                          false,
+        printer.OnTestEnd(test_info->name(), test_case->name(), true,
+                          ResultsPrinter::RESULT_NONE,
                           (base::Time::Now() - start_time).InMillisecondsF());
       } else {
         failed_tests.push_back(test_name);
@@ -412,8 +416,11 @@ bool RunTests() {
         if (exit_code != -1)
           ignore_failure = base::TestSuite::ShouldIgnoreFailure(*test_info);
 
-        printer.OnTestEnd(test_info->name(), test_case->name(), true, true,
-                          ignore_failure,
+        int results = ResultsPrinter::RESULT_FAILED;
+        if (ignore_failure)
+          results |= ResultsPrinter::RESULT_FAILURE_IGNORED;
+
+        printer.OnTestEnd(test_info->name(), test_case->name(), true, results,

[tfarina, 2011/01/11 16:54:04]

If it was an enum, how you would write this line?

[Paweł Hajdan Jr., 2011/01/11 16:55:18]

Exactly the same.

                           (base::Time::Now() - start_time).InMillisecondsF());
         if (ignore_failure)
           ++ignored_failure_count;