[Android] Switch all subprocess.Popen calls to use a temporary file instead of PIPE.

[Android] Switch all subprocess.Popen calls to use a temporary file and other Python bug workarounds.

BUG=224382
TEST=Ran tests.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193273

[Isaac (away), 2013-04-09 20:53:54]

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/androi...
build/android/pylib/android_commands.py:957: logcat_command = 'adb %s logcat -v
threadtime %s' % (self._adb._target_arg,
yikes -- why is this a shell command?

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
File build/android/pylib/cmd_helper.py (right):

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
build/android/pylib/cmd_helper.py:80: stdout = tmpout.read()
I think this will have performance cost; possibly significant on some platforms
(which might not just be ubuntu) -- I have reservations about this change.

[frankf, 2013-04-09 21:14:05]

Thanks craig! If this works, we also need to change
thirdparty/android_testrunner modules.

Some nits.

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
File build/android/pylib/cmd_helper.py (right):

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
build/android/pylib/cmd_helper.py:16: def _call(args, stdout=None, stderr=None,
shell=None, cwd=None):
-> _Call

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
build/android/pylib/cmd_helper.py:80: stdout = tmpout.read()
I'm not concerned with performance at this point. This is a temp solution to get
the bots stable until we roll out Python2.7 with possibly backported Python3
subprocess module.

On 2013/04/09 20:53:54, Isaac wrote:
> I think this will have performance cost; possibly significant on some
platforms
> (which might not just be ubuntu) -- I have reservations about this change.

[craigdh, 2013-04-09 21:25:00]

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/androi...
build/android/pylib/android_commands.py:957: logcat_command = 'adb %s logcat -v
threadtime %s' % (self._adb._target_arg,
On 2013/04/09 20:53:54, Isaac wrote:
> yikes -- why is this a shell command?

We need to go through and remove most of the shell=True, but that needs to wait
for another cl.

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
File build/android/pylib/cmd_helper.py (right):

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
build/android/pylib/cmd_helper.py:16: def _call(args, stdout=None, stderr=None,
shell=None, cwd=None):
On 2013/04/09 21:14:05, frankf wrote:
> -> _Call

Done.

https://codereview.chromium.org/13861026/diff/2001/build/android/pylib/cmd_he...
build/android/pylib/cmd_helper.py:80: stdout = tmpout.read()
On 2013/04/09 21:14:05, frankf wrote:
> I'm not concerned with performance at this point. This is a temp solution to
get
> the bots stable until we roll out Python2.7 with possibly backported Python3
> subprocess module.
> 
> On 2013/04/09 20:53:54, Isaac wrote:
> > I think this will have performance cost; possibly significant on some
> platforms
> > (which might not just be ubuntu) -- I have reservations about this change.
> 

Running it locally I haven't observed any noticeable performance degradation.
The OS should be smart enough to keep it in RAM. Anyway, this is the only
solution I know of to the PIPE buffer issue, a StringIO or something won't work.
Chromite does the same.

[Isaac (away), 2013-04-09 21:41:44]

You also have an SSD.  I don't think we should replace pipes with files for all
calls to cmd_helper...

[craigdh, 2013-04-09 22:56:40]

On 2013/04/09 21:41:44, Isaac wrote:
> You also have an SSD.  I don't think we should replace pipes with files for
all
> calls to cmd_helper...

My /tmp isn't on the SSD. Anyway, looking at the tryjob the tests are all
completing within a few seconds of tryjobs run earlier today without the
temporary files change. And that minor difference appears to be noise as it goes
both ways.

[frankf, 2013-04-09 23:16:11]

lgtm

[Isaac (away), 2013-04-09 23:40:48]

+yfriedman, nileshagrawal

[nilesh, 2013-04-09 23:54:52]

Please test for test timeout and test crash as well.

[craigdh, 2013-04-10 00:14:22]

android_dbg_triggered_tests hit unrelated known issue http://crbug.com/229685.

[craigdh, 2013-04-10 00:38:31]

Committed patchset #4 manually as r193273 (presubmit successful).