Bug 103661 - webkitpy's run_webkit_tests should initialize the printer earlier on
Summary: webkitpy's run_webkit_tests should initialize the printer earlier on
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-29 12:36 PST by Peter Beverloo
Modified: 2012-12-04 04:12 PST (History)
6 users (show)

See Also:


Attachments
Patch (12.12 KB, patch)
2012-12-03 14:28 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 Peter Beverloo 2012-11-29 12:36:58 PST
Before creating the Printer, which creates the MeteredStream that handles the log messages run_webkit_tests.py's run() sets up derived options. _set_up_derived_options() calls various methods on the Port objects. As this is done without a printer, they aren't able to use normal loggers and have to resort to writing to stderr directly.

There are FIXMEs in at least the Mac and Chromium-Android Ports, and the Chromium-Android Port imports "sys" for this reason alone.
Comment 1 Dirk Pranke 2012-12-03 14:28:58 PST
Created attachment 177332 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-12-03 15:03:18 PST
Comment on attachment 177332 [details]
Patch

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

FANTASTIC.

> Tools/Scripts/webkitpy/layout_tests/port/mac.py:241
> -        except ScriptError, e:
> +        except ScriptError:
>              _log.warning('Unable to sample process.')

We could log the actual error. :)
Comment 3 Dirk Pranke 2012-12-03 15:05:26 PST
(In reply to comment #2)
> (From update of attachment 177332 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177332&action=review
> 
> FANTASTIC.
> 
> > Tools/Scripts/webkitpy/layout_tests/port/mac.py:241
> > -        except ScriptError, e:
> > +        except ScriptError:
> >              _log.warning('Unable to sample process.')
> 
> We could log the actual error. :)

Sure :).
Comment 4 Dirk Pranke 2012-12-03 15:08:57 PST
Committed r136443: <http://trac.webkit.org/changeset/136443>
Comment 5 Peter Beverloo 2012-12-04 04:12:14 PST
Awesome, thanks for following up!!