Bug 65743 – rebaseline-chromium-webkit-tests -p argument not working

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Tools / Tests

[noel gordon]

Comma separated list of rebaseline platform names results in a list of characters, none of which match a platform name.

[noel gordon]

Created attachment 103031 [details]
Patch

[Adam Barth]

Comment on attachment 103031 [details]
Patch

Looks reasonable.  How do we test this patch?

[noel gordon]

My current test is that the tool successfully produces rebaselines.  If no platforms are given, the tool currently fails for me.  If I provide
a list of rebaseline platforms via -p (--paltforms), the tools fails for me.  I'd just like something that works.

[Adam Barth]

@dpranke: Thoughts on how to test this change / would you be interested in reviewing this patch?

[Dirk Pranke]

Comment on attachment 103031 [details]
Patch

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).

[Adam Barth]

> I don't know if we expect this tool to live much longer; Adam?

Yeah, this tool is going to be deprecated later today.  I discussed this with Noel a bunch over chat.  We found a work around for him, so this patch isn't overly urgent.  I'm inclined to mark this bug WONTFIX and to address this use case with the new tool, if it's still needed.  (Please feel free to re-open the bug if you disagree.)

[Dirk Pranke]

(In reply to comment #6)
> > I don't know if we expect this tool to live much longer; Adam?
> 
> Yeah, this tool is going to be deprecated later today.  I discussed this with Noel a bunch over chat.  We found a work around for him, so this patch isn't overly urgent.  I'm inclined to mark this bug WONTFIX and to address this use case with the new tool, if it's still needed.  (Please feel free to re-open the bug if you disagree.)

WONTFIX is fine by me.

[noel gordon]

Saw http://trac.webkit.org/changeset/92637, so I'm fine with WONTFIX and the new fancy.