[webkit-reviews] review denied: [Bug 53040] nrwt: add more unit tests for rebaseline-chromium-webkit-tests : [Attachment 79987] hand-test non-mocked code paths, fix a few bugs

[bugzilla-daemon at webkit.org]

Mihai Parparita <mihaip at chromium.org> has denied Dirk Pranke
<dpranke at chromium.org>'s request for review:
Bug 53040: nrwt: add more unit tests for rebaseline-chromium-webkit-tests
https://bugs.webkit.org/show_bug.cgi?id=53040

Attachment 79987: hand-test non-mocked code paths, fix a few bugs
https://bugs.webkit.org/attachment.cgi?id=79987&action=review

------- Additional Comments from Mihai Parparita <mihaip at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=79987&action=review

> Tools/Scripts/webkitpy/common/fetcher.py:34
> +class UrlFetcher(object):

Can the file be called urlfetcher.py, to mirror the class name? fetcher.py is a
bit too generic.

> Tools/Scripts/webkitpy/common/zipper.py:34
> +class UnZipper(object):

I see that comparisons have already been made to the fileset classes that Koz
added (they're meant for the rebaselining new tool, thus they should satisfy
the needs of the old one too).

This class seems pretty similar to ZipFileSet (e.g. they both have namelist and
read). I'm not seeing any barriers to just using it directly.

> Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:894
> +    # We need to create three different Port objects over the life of this

I think this comment should stay in main(), since that's where we actually
create the port objects.