Bug 75648 - webkitpy: clean up test/uri conversion routines
Summary: webkitpy: clean up test/uri conversion routines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-05 14:15 PST by Dirk Pranke
Modified: 2012-01-09 12:58 PST (History)
6 users (show)

See Also:


Attachments
Patch (16.94 KB, patch)
2012-01-05 14:17 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
add more explicit, clearer unit tests, remove unused import (20.63 KB, patch)
2012-01-05 15:06 PST, Dirk Pranke
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2012-01-05 14:15:33 PST
webkitpy: clean up test/uri conversion routines
Comment 1 Dirk Pranke 2012-01-05 14:17:01 PST
Created attachment 121329 [details]
Patch
Comment 2 Dirk Pranke 2012-01-05 14:18:20 PST
(haven't actually tested this on any real ports, just ran the unit tests)
Comment 3 Eric Seidel (no email) 2012-01-05 14:24:12 PST
Comment on attachment 121329 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121329&action=review

I'm not sure I 100% follow the change, but it looks reasonable.  Mostly looks like you're moving code.  Looks like these arent' very tested.  Can we have more tests of these moved functions?

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:46
> +from webkitpy.common.system.path import cygpath

Seems unused?
Comment 4 Dirk Pranke 2012-01-05 15:06:03 PST
Created attachment 121340 [details]
add more explicit, clearer unit tests, remove unused import
Comment 5 Dirk Pranke 2012-01-05 15:07:42 PST
(In reply to comment #3)
> (From update of attachment 121329 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121329&action=review
> 
> I'm not sure I 100% follow the change, but it looks reasonable.  Mostly looks like you're moving code.  Looks like these arent' very tested.  Can we have more tests of these moved functions?
>

Yup, mostly moving code. The routines were actually well-covered by other tests, but I've added some explicit tests just to be clearer. Disclaimer: I haven't actually run the tests (yet?) on win32 or under cygwin to ensure that they pass there as well.
 
> > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:46
> > +from webkitpy.common.system.path import cygpath
> 
> Seems unused?

Yup. removed.
Comment 6 Eric Seidel (no email) 2012-01-05 15:09:16 PST
Comment on attachment 121340 [details]
add more explicit, clearer unit tests, remove unused import

OK.  Thanks.
Comment 7 Dirk Pranke 2012-01-06 14:27:31 PST
Committed r104340: <http://trac.webkit.org/changeset/104340>
Comment 8 Ryosuke Niwa 2012-01-08 17:17:15 PST
webkitpy.layout_tests.port.mock_drt_unittest.MockChromiumDRTTest has been failing on cr-win since this patch was landed: http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/22570/steps/webkitpy-test/logs/stdio
Comment 9 Dirk Pranke 2012-01-09 12:34:32 PST
looking into it.
Comment 10 Dirk Pranke 2012-01-09 12:58:27 PST
Bug 75884 filed.