Fix flakiness in pyauto test on ChromeOS: ntp.NTPTest.testThumbnailPersistence.

Fix flakiness in pyauto test on ChromeOS: ntp.NTPTest.testThumbnailPersistence.

The test previously failed to wait for the 'pkill chrome' command to finish
killing the relevant chrome processes.

BUG=chromium-os:16297
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88411

[dennis_jeffrey, 2011-06-08 18:22:24]

With this change, I'm seeing consistent passing on ChromeoS with "--repeat=20",
whereas previously more than half of the runs failed.

However, this solution feels a little "hacky" to me.  If you can think of a
better approach to handle this situation, please let me know.  Thanks!

[Nirnimesh, 2011-06-08 18:32:22]

Doesn't feel hacky to me. In fact this might explain the flakiness in some other
persistence tests as well. We'll see.

Thanks

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:184: proc = subprocess.Popen(['ps', '-C',
'chrome'], stdout=subprocess.PIPE)
'prep chrome' will be easier to parse

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:185: output, _ = proc.communicate()
output = proc.communicate()[0]

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:196: def _DoesOldChromeProcessStillExist():
take orig_pids as arg

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:204: for new_pid in new_pids:
merge with previous line

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:209: self.WaitUntil(lambda: not
_DoesOldChromeProcessStillExist())
pass orig_pids as arg

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:209: self.WaitUntil(lambda: not
_DoesOldChromeProcessStillExist())
Why the negation? Just invert the function --
lambda: _ArePIDsDead(orig_pids)

[dennis_jeffrey, 2011-06-08 21:50:30]

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:184: proc = subprocess.Popen(['ps', '-C',
'chrome'], stdout=subprocess.PIPE)
On 2011/06/08 18:32:22, Nirnimesh wrote:
> 'prep chrome' will be easier to parse

Awesome!  Thanks!

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:185: output, _ = proc.communicate()
On 2011/06/08 18:32:22, Nirnimesh wrote:
> output = proc.communicate()[0]

This line has since been removed.

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:196: def _DoesOldChromeProcessStillExist():
On 2011/06/08 18:32:22, Nirnimesh wrote:
> take orig_pids as arg

Done.

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:204: for new_pid in new_pids:
On 2011/06/08 18:32:22, Nirnimesh wrote:
> merge with previous line

Done.

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:209: self.WaitUntil(lambda: not
_DoesOldChromeProcessStillExist())
On 2011/06/08 18:32:22, Nirnimesh wrote:
> pass orig_pids as arg

Done.

http://codereview.chromium.org/7108023/diff/1/chrome/test/pyautolib/pyauto.py...
chrome/test/pyautolib/pyauto.py:209: self.WaitUntil(lambda: not
_DoesOldChromeProcessStillExist())
On 2011/06/08 18:32:22, Nirnimesh wrote:
> Why the negation? Just invert the function --
> lambda: _ArePIDsDead(orig_pids)

Done.

[Nirnimesh, 2011-06-08 21:53:11]

LGTM