[webkit-reviews] review granted: [Bug 87802] webkitpy: add support for an ordered dict of test expectations : [Attachment 144667] actually test on python2.6, fix names

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 87802: webkitpy: add support for an ordered dict of test expectations
https://bugs.webkit.org/show_bug.cgi?id=87802

Attachment 144667: actually test on python2.6, fix names
https://bugs.webkit.org/attachment.cgi?id=144667&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=144667&action=review


> Tools/Scripts/webkitpy/layout_tests/port/base.py:895
> +	   """Returns an OrderedDict of name -> expectations strings. The names

> +	   are expected to be (but not required to be) paths in the filesystem.

> +	   If the name is a path, the file can be considered updatable for
things
> +	   like rebaselining, so don't use names that are paths if they're not
paths.
> +	   Generally speaking the ordering should be files in the filesystem in

> +	   cascade order (test_expectations.txt followed by Skipped, if the
port
> +	   honors both formats), then any built-in expectations (e.g., from
compile-time
> +	   exclusions), then --additional-expectations options."""

This has more information than is useful I think. Something like the following
would be better IMO:
"""Returns an OrderedDict of name -> expectations strings. If that set of
expectations corresponds to a file, name is the path to the file. Files are
ordered in cascade order test_expectations.txt, then cascade order Skipped,
then compile-time exclusions, then --additional-expectations."""

Up to you, but all the business about rebaselining seems extraneous to me. I'm
not really sure we need any of this comment actually. It's all "what" and no
"why".

> Tools/Scripts/webkitpy/layout_tests/port/base.py:897
> +	   overrides = OrderedDict()

overrides is a weird name for this.