fixing drt-trampoline

fixing drt-trampoline

BUG=http://dartbug.com/3917

Committed: https://code.google.com/p/dart/source/detail?r=9230

[Siggi Cherem (dart-lang), 2012-06-28 21:23:57]

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
File tools/testing/drt-trampoline.py (left):

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
tools/testing/drt-trampoline.py:52: p.wait()
I believe the issue is related to pipeing output and using wait.

The failing test had a very long output. I believe that piping output uses a
limited buffer that gets filled up. If we don't read from it, 'wait' will block
forever.  'communicate' seems to be better suited for reading data out of the
subprocess.

[Emily Fortuna, 2012-06-28 21:44:39]

lgtm, but with comments, first please! :-)

https://chromiumcodereview.appspot.com/10701025/diff/1/tests/html/html.status
File tests/html/html.status (left):

https://chromiumcodereview.appspot.com/10701025/diff/1/tests/html/html.status...
tests/html/html.status:138: # Strange timeout happening (Issue 3917)
Will you update Issue 3917 with your current theory of why this was failing?

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
File tools/testing/drt-trampoline.py (left):

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
tools/testing/drt-trampoline.py:52: p.wait()
On 2012/06/28 21:23:57, sigmund wrote:
> I believe the issue is related to pipeing output and using wait.
> 
> The failing test had a very long output. I believe that piping output uses a
> limited buffer that gets filled up. If we don't read from it, 'wait' will
block
> forever.  'communicate' seems to be better suited for reading data out of the
> subprocess.

I think you're right. I seem to remember reading in Python's documentation some
warning about p.wait if the buffer gets filled.

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
File tools/testing/drt-trampoline.py (right):

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
tools/testing/drt-trampoline.py:64: output = output[output.find('\n') + 1: -5]
Perhaps I'm misunderstanding how this code works, but why do we replace the
first three newlines and then start with the 4th? How about:
output = output.replace('\n', '_', 4)
 output = output[output.find('\n') + 1: -len('#EOF\n')] (this makes the -5 more
clear, too)

[Siggi Cherem (dart-lang), 2012-06-28 21:55:06]

Thanks!

https://chromiumcodereview.appspot.com/10701025/diff/1/tests/html/html.status
File tests/html/html.status (left):

https://chromiumcodereview.appspot.com/10701025/diff/1/tests/html/html.status...
tests/html/html.status:138: # Strange timeout happening (Issue 3917)
On 2012/06/28 21:44:39, Emily Fortuna wrote:
> Will you update Issue 3917 with your current theory of why this was failing?

yes, I was planning to :)

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
File tools/testing/drt-trampoline.py (right):

https://chromiumcodereview.appspot.com/10701025/diff/1/tools/testing/drt-tram...
tools/testing/drt-trampoline.py:64: output = output[output.find('\n') + 1: -5]
On 2012/06/28 21:44:39, Emily Fortuna wrote:
> Perhaps I'm misunderstanding how this code works, but why do we replace the
> first three newlines and then start with the 4th? How about:
> output = output.replace('\n', '_', 4)
>  output = output[output.find('\n') + 1: -len('#EOF\n')] (this makes the -5
more
> clear, too)

Done. Turns out that it was 4 header lines, not 5...