[webkit-reviews] review denied: [Bug 65743] rebaseline-chromium-webkit-tests -p argument not working : [Attachment 103031] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied noel gordon
<noel.gordon at gmail.com>'s request for review:
Bug 65743: rebaseline-chromium-webkit-tests -p argument not working
https://bugs.webkit.org/show_bug.cgi?id=65743

Attachment 103031: Patch
https://bugs.webkit.org/attachment.cgi?id=103031&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103031&action=review


> Tools/Scripts/webkitpy/to_be_moved/rebaseline_chromium_webkit_tests.py:845
> +

It would be nice if we could fold this list into the default --help message,
but I don't think there's an easy way to do that without assuming a given port.
We could hard-code the chromium port for this, but that would be something of a
hack, and it's not clear to me if it's worth it one way or another (I don't
know if we expect this tool to live much longer; Adam?).

> Tools/Scripts/webkitpy/to_be_moved/rebaseline_chromium_webkit_tests.py:1010
> +	   rebaseline_platforms = user_specified_platforms

This change (the actual bug fix) looks fine.

R-'ing because you should add some unit tests.

To test this, you should add a unit test to
rebaseline_chromium_webkit_tests_unittest.py, to the TestRealMain class. You
can probably clone test_all_platforms, just pass in a subset of the expected
platforms (much like the test_one_platform unit test does, earlier on in the
test file).

To test the -s flag, you should use outputcapture and make sure that -s prints
something (but don't worry about what).