Summary: | nrwt: crash while stopping layout test helper on apple mac lion | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||
Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, eric, jberlin, ojan, thorton, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dirk Pranke
2012-03-22 12:40:23 PDT
Created attachment 133326 [details]
Patch
Comment on attachment 133326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133326&action=review Seems OK. > Tools/Scripts/webkitpy/common/system/executive_mock.py:102 > def popen(self, *args, **kwargs): > # FIXME: Implement logging when self._should_log is set. > - return MockProcess() > + if not self._proc: > + self._proc = MockProcess() > + return self._proc I'm confused why we'd save the MockProcess. popen() would normaly give you a new process every time you called it. :) > Tools/Scripts/webkitpy/layout_tests/port/mac.py:203 > + except IOError, e: > + pass Do we need to log anything here? > Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py:199 > + host.executive._proc = MockProcess('ready\n') I see, this is why you have _proc as a member. (In reply to comment #2) > (From update of attachment 133326 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133326&action=review > > Seems OK. > > > Tools/Scripts/webkitpy/common/system/executive_mock.py:102 > > def popen(self, *args, **kwargs): > > # FIXME: Implement logging when self._should_log is set. > > - return MockProcess() > > + if not self._proc: > > + self._proc = MockProcess() > > + return self._proc > > I'm confused why we'd save the MockProcess. popen() would normaly give you a new process every time you called it. :) > > > Tools/Scripts/webkitpy/layout_tests/port/mac.py:203 > > + except IOError, e: > > + pass > > Do we need to log anything here? > Probably a good idea to at least log a debug message ... > > Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py:199 > > + host.executive._proc = MockProcess('ready\n') > > I see, this is why you have _proc as a member. right :). It's crufty, but it's mock test code, so it's good enough for me for now. Feel free to suggest / add something cleaner. Committed r111750: <http://trac.webkit.org/changeset/111750> |