[webkit-reviews] review granted: [Bug 84762] webkit-patch rebaseline-test is retrieving stale expectations : [Attachment 138680] Now with updated tests :)

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 84762: webkit-patch rebaseline-test is retrieving stale expectations
https://bugs.webkit.org/show_bug.cgi?id=84762

Attachment 138680: Now with updated tests :)
https://bugs.webkit.org/attachment.cgi?id=138680&action=review

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


Thanks for fixing up the tests.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:144
> +	   # correct results. Also, this fetch_from_zip test is really sleazy
because it
> +	   # is bypassing all of the unit tests ...

Nit: comment outdated.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:146
> +	   zip_url =
self._results_url(builder_name).replace('results/layout-test-results',
'layout-test-results.zip')

Now that we don't have the else clause, this is the only use of this method.
Maybe make that method do the right thing? If you hvae it call results_url
instead of accumulated_results_url and then append layout-test-results.zip, I
think it'll do what you want.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:152
> +	       print "	Found " + member_name

In the spirit of keeping the log output minimal, I'm not sure this is really
useful. If the rebaseline continues without error, we can assume it's found.
The error-case below is good though.