Bug 38284 - Audit all uses of subprocess in webkitpy
Summary: Audit all uses of subprocess in webkitpy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-28 14:24 PDT by Eric Seidel (no email)
Modified: 2010-04-28 14:52 PDT (History)
4 users (show)

See Also:

Attachments
Patch (10.50 KB, patch)
2010-04-28 14:28 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Now with a unittest (12.81 KB, patch)
2010-04-28 14:46 PDT, Eric Seidel (no email) 		levin: review+
levin: commit-queue+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-04-28 14:24:15 PDT 
Audit all uses of subprocess in webkitpy
Comment 1 Eric Seidel (no email) 2010-04-28 14:28:25 PDT 
Created attachment 54621 [details]
Patch
Comment 2 David Levin 2010-04-28 14:38:53 PDT 
Comment on attachment 54621 [details]
Patch

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.
Comment 3 Eric Seidel (no email) 2010-04-28 14:46:09 PDT 
Created attachment 54624 [details]
Now with a unittest
Comment 4 Eric Seidel (no email) 2010-04-28 14:52:21 PDT 
Committed r58434: <http://trac.webkit.org/changeset/58434>