Bug 37845 - [chromium] new-run-webkit-tests --use-drt should run on Linux
Summary: [chromium] new-run-webkit-tests --use-drt should run on Linux
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-20 01:56 PDT by Tony Chang
Modified: 2010-05-18 22:12 PDT (History)
1 user (show)

See Also:

Attachments
Patch (3.40 KB, patch)
2010-04-20 01:56 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Patch (6.45 KB, patch)
2010-04-20 02:16 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Patch (4.58 KB, patch)
2010-04-22 17:57 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Patch (7.17 KB, patch)
2010-05-09 22:19 PDT, Tony Chang 		tkent: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2010-04-20 01:56:30 PDT 
[chromium] new-run-webkit-tests --use-drt should run on Linux
Comment 1 Tony Chang 2010-04-20 01:56:50 PDT 
Created attachment 53783 [details]
Patch
Comment 2 Tony Chang 2010-04-20 02:16:31 PDT 
Created attachment 53788 [details]
Patch
Comment 3 Tony Chang 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.
Comment 4 Adam Barth 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.
Comment 5 Tony Chang 2010-04-22 17:57:35 PDT 
Created attachment 54115 [details]
Patch
Comment 6 Tony Chang 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.
Comment 7 Eric Seidel (no email) 2010-05-08 23:26:40 PDT 
Comment on attachment 54115 [details]
Patch

Looks sane, but where are the tests?
Comment 8 Tony Chang 2010-05-09 22:19:12 PDT 
Created attachment 55519 [details]
Patch
Comment 9 Tony Chang 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.
Comment 10 Tony Chang 2010-05-17 23:40:07 PDT 
Maybe Kent-san can review this now.  Eric just wanted some tests added earlier.
Comment 11 Kent Tamura 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.
Comment 12 Tony Chang 2010-05-18 22:12:28 PDT 
Committed r59743: <http://trac.webkit.org/changeset/59743>