Bug 92805 - nrwt: move sharding logic into layout_test_runner.py
Summary: nrwt: move sharding logic into layout_test_runner.py
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on: 92804
Blocks: 89267 92806
  Show dependency treegraph
 
Reported: 2012-07-31 16:07 PDT by Dirk Pranke
Modified: 2012-07-31 18:48 PDT (History)
5 users (show)

See Also:


Attachments
Patch (57.04 KB, patch)
2012-07-31 16:08 PDT, Dirk Pranke
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2012-07-31 16:07:36 PDT
nrwt: move sharding logic into layout_test_runner.py
Comment 1 Dirk Pranke 2012-07-31 16:08:45 PDT
Created attachment 155661 [details]
Patch
Comment 2 Ryosuke Niwa 2012-07-31 18:31:00 PDT
Comment on attachment 155661 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=155661&action=review

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:370
> +                                    'locked_shard'),

Nit: wrong indentation.

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:402
> +                                        extract_and_flatten(some_shards)))

Nit: wrong indentation.

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:407
> +    def _dir_for_test_input(self, test_input):
> +        """Returns the highest-level directory by which to shard the given
> +        test file."""

I would rename the function to _highest_dir_to_shard_for_test_input and get rid of the comment.

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:411
> +        # TODO(ojan): Make the http server on Windows be apache so we can

Nit: TODO -> FIXME.

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:433
> +                http://www.codinghorror.com/blog/2007/12/sorting-for-humans-natural-sort-order.html
> +                http://nedbatchelder.com/blog/200712.html#e20071211T054956

Why are these URLs indented?

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner_unittest.py:2
> +# Copyright (C) 2010 Google Inc. All rights reserved.

Maybe update the date here?
Comment 3 Dirk Pranke 2012-07-31 18:39:21 PDT
Comment on attachment 155661 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=155661&action=review

I will fix all the comments and changes. Thanks for the review!

>> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:433
>> +                http://nedbatchelder.com/blog/200712.html#e20071211T054956
> 
> Why are these URLs indented?

It made things easier to read?
Comment 4 Ryosuke Niwa 2012-07-31 18:40:16 PDT
Comment on attachment 155661 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=155661&action=review

>>> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:433
>>> +                http://nedbatchelder.com/blog/200712.html#e20071211T054956
>> 
>> Why are these URLs indented?
> 
> It made things easier to read?

I think they're easy to read without indentations :)
Comment 5 Dirk Pranke 2012-07-31 18:48:18 PDT
Committed r124280: <http://trac.webkit.org/changeset/124280>