Allow seeding the random layout test order and write out seed.

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py

 # Copyright (C) 2010 Google Inc. All rights reserved.
 # Copyright (C) 2010 Gabor Rapcsanyi (rgabor@inf.u-szeged.hu), University of Szeged
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
@@ skipping 71 matching lines @@
         self.HTTP_SUBDIR = 'http' + port.TEST_PATH_SEPARATOR
         self.INSPECTOR_SUBDIR = 'inspector' + port.TEST_PATH_SEPARATOR
         self.PERF_SUBDIR = 'perf'
         self.WEBSOCKET_SUBDIR = 'websocket' + port.TEST_PATH_SEPARATOR
         self.LAYOUT_TESTS_DIRECTORY = 'LayoutTests'
         self.ARCHIVED_RESULTS_LIMIT = 25
         self._http_server_started = False
         self._wptserve_started = False
         self._websockets_server_started = False
 
+        self._random_seed = None
         self._results_directory = self._port.results_directory()
         self._finder = LayoutTestFinder(self._port, self._options)
         self._runner = LayoutTestRunner(self._options, self._port, self._printer, self._results_directory, self._test_is_slow)
 
     def run(self, args):
         """Run the tests and return a RunDetails object with the results."""
         start_time = time.time()
         self._printer.write_update("Collecting tests ...")
         running_all_tests = False
         try:
@@ skipping 83 matching lines @@
             enabled_pixel_tests_in_retry, only_include_failing=True)
 
         exit_code = summarized_failing_results['num_regressions']
         if exit_code > test_run_results.MAX_FAILURES_EXIT_STATUS:
             _log.warning('num regressions (%d) exceeds max exit status (%d)',
                          exit_code, test_run_results.MAX_FAILURES_EXIT_STATUS)
             exit_code = test_run_results.MAX_FAILURES_EXIT_STATUS
 
         if not self._options.dry_run:
             self._write_json_files(summarized_full_results, summarized_failing_results, initial_results, running_all_tests)
+            if self._random_seed:
+                self._write_random_seed()
 
             if self._options.write_full_results_to:
                 self._filesystem.copyfile(self._filesystem.join(self._results_directory, "full_results.json"),
                                           self._options.write_full_results_to)
 
             self._upload_json_files()
 
             results_path = self._filesystem.join(self._results_directory, "results.html")
             self._copy_results_html_file(results_path)
             if initial_results.keyboard_interrupted:
@@ skipping 41 matching lines @@
         tests_to_run = [test for test in test_names if test not in tests_to_skip]
 
         if not tests_to_run:
             return tests_to_run, tests_to_skip
 
         # Create a sorted list of test files so the subset chunk,
         # if used, contains alphabetically consecutive tests.
         if self._options.order == 'natural':
             tests_to_run.sort(key=self._port.test_key)
         elif self._options.order == 'random':
-            random.shuffle(tests_to_run)
-        elif self._options.order == 'random-seeded':
-            rnd = random.Random()
-            rnd.seed(4)  # http://xkcd.com/221/
-            rnd.shuffle(tests_to_run)
+            if self._options.seed is not None:
+                self._random_seed = self._options.seed
+            else:
+                self._random_seed = int(time.time())

[ojan, 2016/09/06 16:53:29]

I think we should do a fixed value. Using current time is almost always a
mistake because it's then hard to reproduce flakiness you encounter.

[qyearsley, 2016/09/06 17:22:48]

If the seed is output with the results, it should then be possible to reproduce
flakiness that is encountered by passing --order=random --seed=<seed from prior
run>.

One possible advantage of a random seed based on current time by default is that
more time-dependent flakiness would be revealed over the course of multiple test
runs. Although, the order should also change over time as the number of tests
changes, so an order-dependent flaky test shouldn't be able to "hide" for too
long.

Given that, does it still seem like a fixed seed would be a good idea?

[jeffcarp, 2016/09/06 17:22:51]

Is that still true if we log the seed in results.json?

[ojan, 2016/09/06 21:27:11]

Think of this from the perspective of a sheriff trying to figure out why a
failure happened. It will be hard to reason about current time. Admittedly, a
fixed seed is hard to reason about as well, but at least that only changes when
the list of test to run changes.

[jeffcarp, 2016/09/06 22:57:30]

If this would make it hard to tell whether a failing test is flaky or
order-dependent, should we investigate other methods to make sure tests are
order-independent? Or what if we started with a small set of tests that are
known to be order-independent, run those in random order, and slowly move more
tests into that bucket?

[ojan, 2016/09/06 23:07:43]

I think this is a good idea. You probably want it to be a blacklist so that when
you're done moving things you can get rid of the concept of order-dependent
tests. Concretely, we could have an OrderDependentTests file that lets you list
directories or individual tests and the order they should run in. Then you can
remove one directory at a time from that list.

Theoretically, we could even run the order-independent tests on swarming while
we're working on emptying the OrderDependentTest file. I don't know how
complicated that would be to do. If it's easy, I think it'd be a good idea. If
complicated, it might not be worth it.

+            _log.info("Test random order seed: %d", self._random_seed)
+            tests_to_run.sort()
+            rand = random.Random()
+            rand.seed(self._random_seed)
+            rand.shuffle(tests_to_run)
 
         tests_to_run, tests_in_other_chunks = self._finder.split_into_chunks(tests_to_run)
         self._expectations.add_extra_skipped_tests(tests_in_other_chunks)
         tests_to_skip.update(tests_in_other_chunks)
 
         return tests_to_run, tests_to_skip
 
+    def _write_random_seed(self):
+        """Writes the random seed used for randomized test order."""
+        assert self._options.order == 'random' and self._random_seed
+        path = self._filesystem.join(self._results_directory, 'random-seed.txt')
+        contents = '%d\n' % self._random_seed
+        _log.info('%s %s', path, contents)
+        self._filesystem.write_text_file(path, contents)

[qyearsley, 2016/09/05 00:15:28]

We could also potentially:
 - Not write the seed to the results directory; to reproduce a test order would
then involve getting the seed that's logged when the test was run.
 - Also write the test order. This might sometimes be helpful, but may be
unnecessary.

[Dirk Pranke, 2016/09/06 01:19:57]

I would include the seed as a field in the results.json file, along with all of
the other records of the test run. I don't think we should write a separate file
just for this.

I would probably also log the seed that was used at the beginning of the run, as
part of the printer.print_config() function called at run_webkit_tests.py:544,
rather than logging it here.

[mithro, 2016/09/06 02:02:25]

I'm happy with Dirk's suggestions. 

The aim is to be able to find the random seed uses for a given run so it can be
reproduced. This includes both when run on the bots and when run locally.

[qyearsley, 2016/09/06 17:22:48]

Aye, SGTM

+
     def _test_input_for_file(self, test_file):
         return TestInput(test_file,
                          self._options.slow_time_out_ms if self._test_is_slow(test_file) else self._options.time_out_ms,
                          self._test_requires_lock(test_file),
                          should_add_missing_baselines=(self._options.new_test_results and not self._test_is_expected_missing(test_file)))
 
     def _test_requires_lock(self, test_file):
         """Return True if the test needs to be locked when running multiple
         instances of this test runner.
 
@@ skipping 275 matching lines @@
 
         stats = {}
         for result in initial_results.results_by_name.values():
             if result.type != test_expectations.SKIP:
                 stats[result.test_name] = {'results': (_worker_number(result.worker_name), result.test_number, result.pid, int(
                     result.test_run_time * 1000), int(result.total_run_time * 1000))}
         stats_trie = {}
         for name, value in stats.iteritems():
             json_results_generator.add_path_to_trie(name, value, stats_trie)
         return stats_trie