Make browser_tests shardable.

chrome/test/out_of_proc_test_runner.cc

 // 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.
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/environment.h"
 #include "base/file_util.h"
 #include "base/hash_tables.h"
 #include "base/linked_ptr.h"
 #include "base/logging.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/process_util.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
@@ skipping 40 matching lines @@
 const char kSingleProcessTestsFlag[]   = "single_process";
 const char kSingleProcessTestsAndChromeFlag[]   = "single-process";
 // The following is kept for historical reasons (so people that are used to
 // using it don't get surprised).
 const char kChildProcessFlag[]   = "child";
 
 const char kHelpFlag[]   = "help";
 
 const char kTestTerminateTimeoutFlag[] = "test-terminate-timeout";
 
+// The environment variable name for the test shard index.
+static const char kTestShardIndex[] = "GTEST_SHARD_INDEX";
+// The environment variable name for the total number of test shards.
+static const char kTestTotalShards[] = "GTEST_TOTAL_SHARDS";
+
 // How long we wait for the subprocess to exit (with a success/failure code).
 // See http://crbug.com/43862 for some discussion of the value.
 const int kDefaultTestTimeoutMs = 20000;
 
 // The default output file for XML output.
 static const FilePath::CharType kDefaultOutputFile[] = FILE_PATH_LITERAL(
     "test_detail.xml");
 
+// Parses the environment variable var as an Int32. If it is unset,
+// returns default_val. If it is not an Int32, prints an error

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

This comment is lying. We print error and abort when we can't unset the
variable, but when it's not an int the code just returns the default value.

[chase, 2011/01/14 23:39:35]

Done.

+// and aborts.
+int32 Int32FromEnvOrDie(const char* const var, int32 default_val) {
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+  std::string str_val;
+  int32 result;
+  if (!env->GetVar(var, &str_val))
+    return default_val;
+  if (!env->UnSetVar(var)) {
+    LOG(ERROR) << "Invalid environment: we could not unset " << var << ".\n";
+    exit(EXIT_FAILURE);
+  }
+  if (!base::StringToInt(str_val, &result))
+    return default_val;
+  return result;
+}
+
+// Checks whether sharding is enabled by examining the relevant
+// environment variable values. If the variables are present,
+// but inconsistent (i.e., shard_index >= total_shards), prints
+// an error and exits.
+bool ShouldShard(int32* total_shards, int32* shard_index) {
+  *total_shards = Int32FromEnvOrDie(kTestTotalShards, -1);
+  *shard_index = Int32FromEnvOrDie(kTestShardIndex, -1);
+
+  if (*total_shards == -1 && *shard_index == -1) {
+    return false;
+  } else if (*total_shards == -1 && *shard_index != -1) {
+    LOG(ERROR) << "Invalid environment variables: you have "
+               << kTestShardIndex << " = " << *shard_index
+               << ", but have left " << kTestTotalShards << " unset.\n";
+    exit(EXIT_FAILURE);
+  } else if (*total_shards != -1 && *shard_index == -1) {
+    LOG(ERROR) << "Invalid environment variables: you have "
+               << kTestTotalShards << " = " << *total_shards
+               << ", but have left " << kTestShardIndex << " unset.\n";
+    exit(EXIT_FAILURE);
+  } else if (*shard_index < 0 || *shard_index >= *total_shards) {
+    LOG(ERROR) << "Invalid environment variables: we require 0 <= "
+               << kTestShardIndex << " < " << kTestTotalShards
+               << ", but you have " << kTestShardIndex << "=" << *shard_index
+               << ", " << kTestTotalShards << "=" << *total_shards << ".\n";
+    exit(EXIT_FAILURE);
+  }
+
+  return *total_shards > 1;
+}
+
+// Given the total number of shards, the shard index, and the test id,
+// returns true iff the test should be run on this shard. The test id is
+// some arbitrary but unique non-negative integer assigned to each test

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: "assigned" -> "assigned by this launcher", to avoid confusion with gtest
which might also assign its own numbers.

[chase, 2011/01/14 23:39:35]

Done.

+// method. Assumes that 0 <= shard_index < total_shards.

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: Can we check that assumption?

[chase, 2011/01/14 23:39:35]

This assumption is verified in ShouldShard().  I updated the comment to point
that out.

+bool ShouldRunTestOnShard(int total_shards, int shard_index, int test_id) {
+  return (test_id % total_shards) == shard_index;
+}
+
 // 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 {
@@ skipping 271 matching lines @@
   // positive filter and negative filter portions.
   std::string positive_filter = filter;
   std::string negative_filter = "";
   size_t dash_pos = filter.find('-');
   if (dash_pos != std::string::npos) {
     positive_filter = filter.substr(0, dash_pos);  // Everything up to the dash.
     negative_filter = filter.substr(dash_pos + 1); // Everything after the dash.
   }
 
   int test_run_count = 0;
+  int num_runnable_tests = 0;
   int ignored_failure_count = 0;
   std::vector<std::string> failed_tests;
 
+  int32 total_shards;
+  int32 shard_index;
+  bool should_shard = ShouldShard(&total_shards, &shard_index);
+  bool should_run;

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: Why is |should_run| declared here, so far from the place of use?

[chase, 2011/01/14 23:39:35]

Moved below.

+
   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, 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 ((!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);
         continue;
       }
+      // Decide if this test should be run.
+      should_run = true;

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: It should be possible to declare bool should_run here, or maybe add an
isolated function ShouldRun.

[chase, 2011/01/14 23:39:35]

Declared here.

+      if (should_shard) {
+        should_run = ShouldRunTestOnShard(total_shards, shard_index,
+                                          num_runnable_tests);
+      }
+      num_runnable_tests += 1;
+      // If sharding is enabled and the test should not be run, skip it.
+      if (should_shard && !should_run) {

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: Is should_shard needed in the condition?

If should_shard is false (and the loop body doesn't change it), should_run will
always be true.

[chase, 2011/01/14 23:39:35]

Done.

+        continue;
+      }
       base::Time start_time = base::Time::Now();
       ++test_run_count;
       int exit_code = RunTest(test_name);
       if (exit_code == 0) {
         // Test passed.
         printer.OnTestEnd(test_info->name(), test_case->name(), true, false,
                           false,
                           (base::Time::Now() - start_time).InMillisecondsF());
       } else {
         failed_tests.push_back(test_name);
 
         bool ignore_failure = false;
 
         // -1 exit code means a crash or hang. Never ignore those failures,
         // they are serious and should always be visible.
         if (exit_code != -1)
           ignore_failure = base::TestSuite::ShouldIgnoreFailure(*test_info);
 
         printer.OnTestEnd(test_info->name(), test_case->name(), true, true,
                           ignore_failure,
                           (base::Time::Now() - start_time).InMillisecondsF());
         if (ignore_failure)
           ++ignored_failure_count;
       }
     }
   }
 
-  printf("%d test%s run\n", test_run_count, test_run_count > 1 ? "s" : "");
+  printf("%d test%s run\n", test_run_count, test_run_count == 1 ? "" : "s");

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: Why? This change seems unrelated, I'd prefer not to put more things into
the diff unless necessary (you can TBR those later).

[chase, 2011/01/14 23:39:35]

Removed.

   printf("%d test%s failed (%d ignored)\n",
          static_cast<int>(failed_tests.size()),
-         failed_tests.size() > 1 ? "s" : "", ignored_failure_count);
+         failed_tests.size() == 1 ? "" : "s", ignored_failure_count);

[Paweł Hajdan Jr., 2011/01/14 09:32:45]

nit: Why? This change seems unrelated, I'd prefer not to put more things into
the diff unless necessary (you can TBR those later).

[chase, 2011/01/14 23:39:35]

Removed.

   if (failed_tests.size() - ignored_failure_count == 0)
     return true;
 
   printf("Failing tests:\n");
   for (std::vector<std::string>::const_iterator iter = failed_tests.begin();
        iter != failed_tests.end(); ++iter) {
     printf("%s\n", iter->c_str());
   }
 
   return false;
@@ skipping 99 matching lines @@
       exit_code = 1;
       break;
     }
 
     // Special value "-1" means "repeat indefinitely".
     if (cycles != -1)
       cycles--;
   }
   return exit_code;
 }