[webkit-reviews] review granted: [Bug 38284] Audit all uses of subprocess in webkitpy : [Attachment 54621] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 38284: Audit all uses of subprocess in webkitpy
https://bugs.webkit.org/show_bug.cgi?id=38284

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

------- Additional Comments from David Levin <levin at chromium.org>
Two things to fix up before committing.

> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> +	   in webkitpy.  I did not find any unsafe calls
> +	   in my audit, but I did remove numerousunneeded

numerousunneeded


>
> diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py

> -	       rcode = subprocess.call(['wdiff'], stderr=f)
> +	       # We're ignoring the return and always returning True
> +	       self._executive.run_command(['wdiff'],
error_handler=Excutive.ignore_error)
Excutive

You mentioned possibly adding a test for this? It may be good if you could
because it would have caught this issue.