Bug 81949 - nrwt: crash while stopping layout test helper on apple mac lion
Summary: nrwt: crash while stopping layout test helper on apple mac lion
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-03-22 12:40 PDT by Dirk Pranke
Modified: 2012-03-22 13:29 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.20 KB, patch)
2012-03-22 13:02 PDT, 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 Dirk Pranke 2012-03-22 12:40:23 PDT
See: http://build.webkit.org/builders/Lion%20Intel%20Release%20%28Tests%29/builds/6721/steps/layout-test/logs/stdio

The fact that the stop call itself fails is probably a bug as well, but NRWT should deal with it.
Comment 1 Dirk Pranke 2012-03-22 13:02:03 PDT
Created attachment 133326 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-03-22 13:18:06 PDT
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.
Comment 3 Dirk Pranke 2012-03-22 13:21:18 PDT
(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.
Comment 4 Dirk Pranke 2012-03-22 13:29:31 PDT
Committed r111750: <http://trac.webkit.org/changeset/111750>