Bug 46135 - new-run-webkit-tests: refactor command line args getting passed to DRT
Summary: new-run-webkit-tests: refactor command line args getting passed to DRT
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: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-20 16:43 PDT by Dirk Pranke
Modified: 2010-09-21 17:08 PDT (History)
5 users (show)

See Also:

Attachments
Patch (23.39 KB, patch)
2010-09-20 16:46 PDT, Dirk Pranke 		no flags Details | Formatted Diff | Diff
Patch (24.32 KB, patch)
2010-09-20 17:04 PDT, Dirk Pranke 		ojan: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 Dirk Pranke 2010-09-20 16:43:56 PDT 
new-run-webkit-tests: refactor command line args getting passed to DRT
Comment 1 Dirk Pranke 2010-09-20 16:46:02 PDT 
Created attachment 68155 [details]
Patch
Comment 2 Dirk Pranke 2010-09-20 17:04:41 PDT 
Created attachment 68159 [details]
Patch
Comment 3 Ojan Vafai 2010-09-20 17:28:02 PDT 
Comment on attachment 68159 [details]
Patch

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

Looks like an improvement. Please fix the style nit.

> WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py:353
> +        if self._image_path:
> +            driver_args.append("--pixel-tests=" + self._image_path)
> +        if self._options.use_drt:
> +            driver_args.append('--test-shell')
> +        else:
> +            driver_args.append('--layout-tests')
> +        if self._options.startup_dialog:
> +            driver_args.append('--testshell-startup-dialog')
> +        if self._options.gp_fault_error_box:
> +            driver_args.append('--gp-fault-error-box')

style nit: this needs some line breaks to be more readable. I'd personally put a line break after each if and if-else clause, but just a line break before and after the if-else clause would help.
Comment 4 Dirk Pranke 2010-09-20 17:35:30 PDT 
(In reply to comment #3)
> (From update of attachment 68159 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68159&action=review
> 
> Looks like an improvement. Please fix the style nit.
> 
> > WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py:353
> > +        if self._image_path:
> > +            driver_args.append("--pixel-tests=" + self._image_path)
> > +        if self._options.use_drt:
> > +            driver_args.append('--test-shell')
> > +        else:
> > +            driver_args.append('--layout-tests')
> > +        if self._options.startup_dialog:
> > +            driver_args.append('--testshell-startup-dialog')
> > +        if self._options.gp_fault_error_box:
> > +            driver_args.append('--gp-fault-error-box')
> 
> style nit: this needs some line breaks to be more readable. I'd personally put a line break after each if and if-else clause, but just a line break before and after the if-else clause would help.

done. Thanks!
Comment 5 Dirk Pranke 2010-09-20 18:26:24 PDT 
Committed r67905: <http://trac.webkit.org/changeset/67905>
Comment 6 Tony Chang 2010-09-21 16:44:32 PDT 
This is causing all tests on DRT to fail on Mac:
https://bugs.webkit.org/show_bug.cgi?id=46230
Comment 7 Dirk Pranke 2010-09-21 17:08:03 PDT 
This should be fixed as of r67993 (hopefully). See bug 46230 for more.