[webkit-reviews] review granted: [Bug 81949] nrwt: crash while stopping layout test helper on apple mac lion : [Attachment 133326] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 81949: nrwt: crash while stopping layout test helper on apple mac lion
https://bugs.webkit.org/show_bug.cgi?id=81949

Attachment 133326: Patch
https://bugs.webkit.org/attachment.cgi?id=133326&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.