Bug 37845

Summary: [chromium] new-run-webkit-tests --use-drt should run on Linux
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch tkent: review+

Tony Chang
Reported 2010-04-20 01:56:30 PDT
[chromium] new-run-webkit-tests --use-drt should run on Linux
Attachments
Patch (3.40 KB, patch)
2010-04-20 01:56 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (6.45 KB, patch)
2010-04-20 02:16 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (4.58 KB, patch)
2010-04-22 17:57 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch (7.17 KB, patch)
2010-05-09 22:19 PDT, Tony Chang 		tkent: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2010-04-20 01:56:50 PDT
Created attachment 53783 [details] Patch
Tony Chang
Comment 2 2010-04-20 02:16:31 PDT
Created attachment 53788 [details] Patch
Tony Chang
Comment 3 2010-04-20 02:17:06 PDT
(In reply to comment #2) > Created an attachment (id=53788) [details] > Patch Updated to fix the output directory of the tests. It was writing the test results into xcodebuild.
Adam Barth
Comment 4 2010-04-22 12:51:17 PDT
Comment on attachment 53788 [details] Patch WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py:91 + def results_directory(self): Why is this code copy/pasted twice? It seems better to have it in the base class if its used by more than one subclass. WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py:167 + def _path_to_image_diff(self): This code looks copy/pasted too. WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py:101 + def results_directory(self): Here the code is again. We don't want any copy/paste code.
Tony Chang
Comment 5 2010-04-22 17:57:35 PDT
Created attachment 54115 [details] Patch
Tony Chang
Comment 6 2010-04-22 17:58:36 PDT
(In reply to comment #4) > (From update of attachment 53788 [details]) > WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py:91 > + def results_directory(self): > Why is this code copy/pasted twice? It seems better to have it in the base > class if its used by more than one subclass. I copied the functions because they call _build_path() which is only defined in the implementation classes. But I see other methods doing this, so I guess that's ok.
Eric Seidel (no email)
Comment 7 2010-05-08 23:26:40 PDT
Comment on attachment 54115 [details] Patch Looks sane, but where are the tests?
Tony Chang
Comment 8 2010-05-09 22:19:12 PDT
Created attachment 55519 [details] Patch
Tony Chang
Comment 9 2010-05-09 22:20:44 PDT
(In reply to comment #7) > (From update of attachment 54115 [details]) > Looks sane, but where are the tests? Good point, added some tests.
Tony Chang
Comment 10 2010-05-17 23:40:07 PDT
Maybe Kent-san can review this now. Eric just wanted some tests added earlier.
Kent Tamura
Comment 11 2010-05-17 23:50:47 PDT
Comment on attachment 55519 [details] Patch This looks OK. I'll set r+ tomorrow if no one adds review comments.
Tony Chang
Comment 12 2010-05-18 22:12:28 PDT
Committed r59743: <http://trac.webkit.org/changeset/59743>
Note You need to log in before you can comment on or make changes to this bug.