Skip to content

Commit

Permalink
Implement --gtest_shuffle in //base/test/launcher.
Browse files Browse the repository at this point in the history
We were previously passing --gtest_shuffle down to gtest. However,
in some cases (e.g. browser_tests, or other suites using
content::TestLauncherDelegate), gtest wasn't seeing the flag until
it was already in a test-specific worker process -- i.e., until
it only had one test to shuffle.

This moves the shuffling into the test launcher, before we hand
off to the delegate, ensuring that we shuffle across the entire shard.
The implementation is largely based on gtest's: reproducible given a
seed, w/ a seed range of [0, 100000).

This also uses --gtest_shuffle on browser_tests on Mac in an attempt
to collect more data on what's causing VM WindowServer deaths there.

Bug: 767397
Change-Id: I8aced17cdc5a574e3dc127143c0d0a553f62bf62
Reviewed-on: https://chromium-review.googlesource.com/1100966
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567763}
  • Loading branch information
jbudorick authored and Commit Bot committed Jun 15, 2018
1 parent 799b030 commit deda551
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 6 deletions.
58 changes: 56 additions & 2 deletions base/test/launcher/test_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <algorithm>
#include <map>
#include <random>
#include <utility>

#include "base/at_exit.h"
Expand Down Expand Up @@ -108,6 +109,10 @@ const size_t kOutputSnippetLinesLimit = 5000;
// results in truncating the output and failing the test.
const size_t kOutputSnippetBytesLimit = 300 * 1024;

// Limit of seed values for gtest shuffling. Arbitrary, but based on
// gtest's similarly arbitrary choice.
const uint32_t kRandomSeedUpperBound = 100000;

// Set of live launch test processes with corresponding lock (it is allowed
// for callers to launch processes on different threads).
Lock* GetLiveProcessesLock() {
Expand Down Expand Up @@ -246,8 +251,10 @@ CommandLine PrepareCommandLineForGTest(const CommandLine& command_line,
CommandLine new_command_line(command_line.GetProgram());
CommandLine::SwitchMap switches = command_line.GetSwitches();

// Strip out gtest_repeat flag - this is handled by the launcher process.
// Handled by the launcher process.
switches.erase(kGTestRepeatFlag);
switches.erase(kGTestShuffleFlag);
switches.erase(kGTestRandomSeedFlag);

// Don't try to write the final XML report in child processes.
switches.erase(kGTestOutputFlag);
Expand Down Expand Up @@ -555,6 +562,8 @@ const char kGTestListTestsFlag[] = "gtest_list_tests";
const char kGTestRepeatFlag[] = "gtest_repeat";
const char kGTestRunDisabledTestsFlag[] = "gtest_also_run_disabled_tests";
const char kGTestOutputFlag[] = "gtest_output";
const char kGTestShuffleFlag[] = "gtest_shuffle";
const char kGTestRandomSeedFlag[] = "gtest_random_seed";

TestLauncherDelegate::~TestLauncherDelegate() = default;

Expand All @@ -578,14 +587,18 @@ TestLauncher::TestLauncher(TestLauncherDelegate* launcher_delegate,
retry_limit_(0),
force_run_broken_tests_(false),
run_result_(true),
shuffle_(false),
shuffle_seed_(0),
watchdog_timer_(FROM_HERE,
kOutputTimeout,
this,
&TestLauncher::OnOutputTimeout),
parallel_jobs_(parallel_jobs) {}

TestLauncher::~TestLauncher() {
base::TaskScheduler::GetInstance()->Shutdown();
if (base::TaskScheduler::GetInstance()) {
base::TaskScheduler::GetInstance()->Shutdown();
}
}

bool TestLauncher::Run() {
Expand Down Expand Up @@ -939,6 +952,38 @@ bool TestLauncher::Init() {
if (command_line->HasSwitch(switches::kTestLauncherForceRunBrokenTests))
force_run_broken_tests_ = true;

// Some of the TestLauncherDelegate implementations don't call into gtest
// until they've already split into test-specific processes. This results
// in gtest's native shuffle implementation attempting to shuffle one test.
// Shuffling the list of tests in the test launcher (before the delegate
// gets involved) ensures that the entire shard is shuffled.
if (command_line->HasSwitch(kGTestShuffleFlag)) {
shuffle_ = true;

if (command_line->HasSwitch(kGTestRandomSeedFlag)) {
const std::string custom_seed_str =
command_line->GetSwitchValueASCII(kGTestRandomSeedFlag);
uint32_t custom_seed = 0;
if (!StringToUint(custom_seed_str, &custom_seed)) {
LOG(ERROR) << "Unable to parse seed \"" << custom_seed_str << "\".";
return false;
}
if (custom_seed >= kRandomSeedUpperBound) {
LOG(ERROR) << "Seed " << custom_seed << " outside of expected range "
<< "[0, " << kRandomSeedUpperBound << ")";
return false;
}
shuffle_seed_ = custom_seed;
} else {
std::uniform_int_distribution<uint32_t> dist(0, kRandomSeedUpperBound);
std::random_device random_dev;
shuffle_seed_ = dist(random_dev);
}
} else if (command_line->HasSwitch(kGTestRandomSeedFlag)) {
LOG(ERROR) << kGTestRandomSeedFlag << " requires " << kGTestShuffleFlag;
return false;
}

fprintf(stdout, "Using %" PRIuS " parallel jobs.\n", parallel_jobs_);
fflush(stdout);

Expand Down Expand Up @@ -1156,6 +1201,15 @@ void TestLauncher::RunTests() {
test_names.push_back(test_name);
}

if (shuffle_) {
std::mt19937 randomizer;
randomizer.seed(shuffle_seed_);
std::shuffle(test_names.begin(), test_names.end(), randomizer);

fprintf(stdout, "Randomizing with seed %u\n", shuffle_seed_);
fflush(stdout);
}

// Save an early test summary in case the launcher crashes or gets killed.
MaybeSaveSummaryAsJSON({"EARLY_SUMMARY", kUnreliableResultsTag});

Expand Down
8 changes: 7 additions & 1 deletion base/test/launcher/test_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ extern const char kGTestListTestsFlag[];
extern const char kGTestRepeatFlag[];
extern const char kGTestRunDisabledTestsFlag[];
extern const char kGTestOutputFlag[];
extern const char kGTestShuffleFlag[];
extern const char kGTestRandomSeedFlag[];

// Interface for use with LaunchTests that abstracts away exact details
// which tests and how are run.
Expand Down Expand Up @@ -197,7 +199,7 @@ class TestLauncher {
int32_t total_shards_; // Total number of outer shards, at least one.
int32_t shard_index_; // Index of shard the launcher is to run.

int cycles_; // Number of remaining test itreations, or -1 for infinite.
int cycles_; // Number of remaining test iterations, or -1 for infinite.

// Test filters (empty means no filter).
bool has_at_least_one_positive_filter_;
Expand Down Expand Up @@ -239,6 +241,10 @@ class TestLauncher {
// Result to be returned from Run.
bool run_result_;

// Support for test shuffling, just like gtest does.
bool shuffle_;
uint32_t shuffle_seed_;

TestResultsTracker results_tracker_;

// Watchdog timer to make sure we do not go without output for too long.
Expand Down
9 changes: 6 additions & 3 deletions testing/buildbot/chromium.mac.json
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@
},
{
"args": [
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter"
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter",
"--gtest_shuffle"
],
"swarming": {
"can_use_on_swarming_builders": true,
Expand Down Expand Up @@ -1167,7 +1168,8 @@
},
{
"args": [
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter"
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter",
"--gtest_shuffle"
],
"swarming": {
"can_use_on_swarming_builders": true,
Expand Down Expand Up @@ -1724,7 +1726,8 @@
},
{
"args": [
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter"
"--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter",
"--gtest_shuffle"
],
"experiment_percentage": 100,
"swarming": {
Expand Down
9 changes: 9 additions & 0 deletions testing/buildbot/test_suite_exceptions.pyl
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,27 @@
},
# chromium.mac
'Mac10.11 Tests': {
# A subset of tests seem to cause WindowServer deaths on VMs.
# crbug.com/828031 et al.
'args': [
'--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter',
'--gtest_shuffle',
],
},
'Mac10.12 Tests': {
# A subset of tests seem to cause WindowServer deaths on VMs.
# crbug.com/828031 et al.
'args': [
'--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter',
'--gtest_shuffle',
],
},
'Mac10.13 Tests': {
# A subset of tests seem to cause WindowServer deaths on VMs.
# crbug.com/828031 et al.
'args': [
'--test-launcher-filter-file=../../testing/buildbot/filters/mac_window_server_killers.browser_tests.filter',
'--gtest_shuffle',
],
'experiment_percentage': 100,
},
Expand Down

0 comments on commit deda551

Please sign in to comment.