Bug 75648

Summary: webkitpy: clean up test/uri conversion routines
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, eric, ojan, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
add more explicit, clearer unit tests, remove unused import eric: review+

Dirk Pranke
Reported 2012-01-05 14:15:33 PST
webkitpy: clean up test/uri conversion routines
Attachments
Patch (16.94 KB, patch)
2012-01-05 14:17 PST, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
add more explicit, clearer unit tests, remove unused import (20.63 KB, patch)
2012-01-05 15:06 PST, Dirk Pranke 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-01-05 14:17:01 PST
Created attachment 121329 [details] Patch
Dirk Pranke
Comment 2 2012-01-05 14:18:20 PST
(haven't actually tested this on any real ports, just ran the unit tests)
Eric Seidel (no email)
Comment 3 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?
Dirk Pranke
Comment 4 2012-01-05 15:06:03 PST
Created attachment 121340 [details] add more explicit, clearer unit tests, remove unused import
Dirk Pranke
Comment 5 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.
Eric Seidel (no email)
Comment 6 2012-01-05 15:09:16 PST
Comment on attachment 121340 [details] add more explicit, clearer unit tests, remove unused import OK. Thanks.
Dirk Pranke
Comment 7 2012-01-06 14:27:31 PST
Committed r104340: <http://trac.webkit.org/changeset/104340>
Ryosuke Niwa
Comment 8 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
Dirk Pranke
Comment 9 2012-01-09 12:34:32 PST
looking into it.
Dirk Pranke
Comment 10 2012-01-09 12:58:27 PST
Bug 75884 filed.
Note You need to log in before you can comment on or make changes to this bug.