webkitpy: Reworking test chunking for sharding.

webkitpy: Reworking test chunking for sharding.

Tests which are not being run as part of this shard should not appear in
the output. This means the chunking needs to occur *before* the test
expectations are loaded as any test with an expectation will appear in
the output.

After doing this;

 * It is possible to merge the test shard output without having to
   understand the different test states.

 * It significantly reduces the size of the output on each shard (as
   they only contain the details about tests which are actually assigned
   to the shard).

BUG=524758
Review-Url: https://codereview.chromium.org/2693533003
Cr-Commit-Position: refs/heads/master@{#450623}
Committed: https://chromium.googlesource.com/chromium/src/+/238f6ccccaef8bff5a9f2be96ce71bdf2faebde6

[mithro, 2017-02-11 00:47:55]

tansell@chromium.org changed reviewers:
+ jeffcarp@chromium.org, qyearsley@chromium.org

[jeffcarp, 2017-02-13 20:03:49]

lgtm

[qyearsley, 2017-02-13 21:31:49]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-13 21:32:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qyearsley, 2017-02-13 21:49:00]

LGTM, although if it's simple to add I'd also like to see unit tests for
Printer.print_found and TestExpectations.remove_tests :-)

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:95:
start_time = time.time()
Not related to this CL, but it's worth noting that in other places we replaced
time.time() with host.time() (which would be self._port.host.time() here) to
make it more unit-testable.

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:1179:
if test.name and test.name in tests_to_remove:
I guess that self._expectations may contain TestExpectationLine objects for
blank lines and comments, which is why we want to check name?

Also, I think `test.name` is almost always the same as `test.path`, but it
theoretically might be different; probably doesn't matter here.

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing.py:108:
found_str += '; running %d' % num_to_run
So this might say something like "Found 2090 (total 41012); running 2045" or
something like that? Might the message  be clearer if it said something like
"Found 41012 (2090 on this shard); running 2045" or something like that?

[mcgreevy, 2017-02-13 22:45:12]

mcgreevy@chromium.org changed reviewers:
+ mcgreevy@chromium.org

[mcgreevy, 2017-02-13 22:45:13]

lgtm

[commit-bot: I haz the power, 2017-02-13 22:51:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-13 22:51:59]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mithro, 2017-02-13 23:37:00]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-13 23:37:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mithro, 2017-02-14 00:40:03]

Turns out there are unittests for this code that I hadn't updated. I've updated
them now.

I'm still trying to track down how this change causes the following test failure
to occur;
------------------------
$ ./third_party/WebKit/Tools/Scripts/test-webkitpy --verbose
webkitpy.layout_tests.run_webkit_tests_unittest.RunTest.test_run_singly         
                                   
[1/1] webkitpy.layout_tests.run_webkit_tests_unittest.RunTest.test_run_singly
failed unexpectedly:
  Traceback (most recent call last):
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py",
line 450, in test_run_singly
      batch_tests_run = get_test_batches(['--run-singly'])
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py",
line 113, in get_test_batches
      results = get_test_results(args, host)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py",
line 136, in get_test_results
      logging_stream=logging_stream, stdout=stdout)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py",
line 563, in run
      run_details = _run_tests(port, options, args, printer)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py",
line 554, in _run_tests
      return manager.run(args)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py",
line 148, in run
      num_workers)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py",
line 360, in _run_tests
      tests_to_skip, num_workers, retry_attempt)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py",
line 122, in run_tests
      pool.run(('test_list', shard.name, shard.test_inputs) for shard in
all_shards)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/common/message_pool.py",
line 95, in run
      self.wait()
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/common/message_pool.py",
line 124, in wait
      self._workers[0].run()
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/common/message_pool.py",
line 268, in run
      self._raise(sys.exc_info())
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/common/message_pool.py",
line 256, in run
      worker.handle(message.name, message.src, *message.args)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py",
line 259, in handle
      device_failed = self._run_test(test_input, test_list_name)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py",
line 295, in _run_test
      stop_when_done)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py",
line 52, in run_single_test
      return runner.run()
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py",
line 146, in run
      return self._run_compare_test()
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py",
line 163, in _run_compare_test
      driver_output = self._driver.run_test(self._driver_input(),
self._stop_when_done)
    File
"/usr/local/google/home/tansell/work/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py",
line 563, in run_test
      raise ValueError('exception from ' + test_name)
  ValueError: exception from failures/expected/exception.html
1 test run, 1 failure.
------------------------

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:95:
start_time = time.time()
On 2017/02/13 21:49:00, qyearsley wrote:
> Not related to this CL, but it's worth noting that in other places we replaced
> time.time() with host.time() (which would be self._port.host.time() here) to
> make it more unit-testable.

Acknowledged.

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:1179:
if test.name and test.name in tests_to_remove:
On 2017/02/13 21:49:00, qyearsley wrote:
> I guess that self._expectations may contain TestExpectationLine objects for
> blank lines and comments, which is why we want to check name?
> 
> Also, I think `test.name` is almost always the same as `test.path`, but it
> theoretically might be different; probably doesn't matter here.

I'm following the `add_extra_skipped_tests` code above.

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing.py
(right):

https://codereview.chromium.org/2693533003/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing.py:108:
found_str += '; running %d' % num_to_run
On 2017/02/13 21:49:00, qyearsley wrote:
> So this might say something like "Found 2090 (total 41012); running 2045" or
> something like that? Might the message  be clearer if it said something like
> "Found 41012 (2090 on this shard); running 2045" or something like that?

I bounced back and forth between putting the total in the brackets and shard in
brackets. I think it's more useful to have the actual number being run first.

[commit-bot: I haz the power, 2017-02-14 00:45:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-14 00:45:04]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mithro, 2017-02-14 18:58:32]

The CQ bit was checked by tansell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-14 18:58:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-14 20:21:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-14 20:21:50]

Dry run: This issue passed the CQ dry run.

[mithro, 2017-02-15 07:21:11]

The CQ bit was checked by tansell@chromium.org

[mithro, 2017-02-15 07:21:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
jeffcarp@chromium.org, qyearsley@chromium.org, mcgreevy@chromium.org
Link to the patchset: https://codereview.chromium.org/2693533003/#ps60001
(title: "Rebase onto revert-1491143003")

[commit-bot: I haz the power, 2017-02-15 07:21:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 07:24:52]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487143271870410,
"parent_rev": "e47e5f7e3c033378b37b1e39c0c236ed6a608d8f", "commit_rev":
"238f6ccccaef8bff5a9f2be96ce71bdf2faebde6"}

[commit-bot: I haz the power, 2017-02-15 07:25:27]

Description was changed from

==========
webkitpy: Reworking test chunking for sharding.

Tests which are not being run as part of this shard should not appear in
the output. This means the chunking needs to occur *before* the test
expectations are loaded as any test with an expectation will appear in
the output.

After doing this;

 * It is possible to merge the test shard output without having to
   understand the different test states.

 * It significantly reduces the size of the output on each shard (as
   they only contain the details about tests which are actually assigned
   to the shard).

BUG=524758
==========

to

==========
webkitpy: Reworking test chunking for sharding.

Tests which are not being run as part of this shard should not appear in
the output. This means the chunking needs to occur *before* the test
expectations are loaded as any test with an expectation will appear in
the output.

After doing this;

 * It is possible to merge the test shard output without having to
   understand the different test states.

 * It significantly reduces the size of the output on each shard (as
   they only contain the details about tests which are actually assigned
   to the shard).

BUG=524758

Review-Url: https://codereview.chromium.org/2693533003
Cr-Commit-Position: refs/heads/master@{#450623}
Committed:
https://chromium.googlesource.com/chromium/src/+/238f6ccccaef8bff5a9f2be96ce7...
==========

[commit-bot: I haz the power, 2017-02-15 07:25:28]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/238f6ccccaef8bff5a9f2be96ce7...