fixing drt-trampoline

tools/testing/drt-trampoline.py

 # Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 # for details. All rights reserved. Use of this source code is governed by a
 # BSD-style license that can be found in the LICENSE file.
 #
 # For now we have to use this trampoline to turn --dart-flags command line
 # switch into env variable DART_FLAGS.  Eventually, DumpRenderTree should
 # support --dart-flags and this hack may go away.
 #
 # Expected invocation: python drt-trampoline.py <path to DRT> <DRT command line>
 
@@ skipping 31 matching lines @@
             'Bad file expectation (%s), ' % out_expected_file
             + 'please specify either a .txt or a .png file')
     elif arg.endswith('.html'):
       test_file = arg
       cmd.append(arg)
     else:
       cmd.append(arg)
 
 
   p = subprocess.Popen(cmd, env=env, stdout=subprocess.PIPE)
-  p.wait()

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

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]

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

+  output, error = p.communicate()
   if p.returncode != 0:
     raise Exception('Failed to run command. return code=%s' % p.returncode)
 
   if out_expected_file:
     # Compare output to the given expectation file.
-    output = None
     expectation = None
-    with p.stdout as res:
-      if is_png:
-        # DRT prints the image to STDOUT, but includes 5 header lines.
-        for i in range(4): res.readline()
-      output = res.read()
+    if is_png:
+      # DRT prints the image to STDOUT, but includes extra text that we trim:
+      # - 5 header lines
+      # - a '#EOF\n' at the end
+      output = output.replace('\n', '_', 3)
+      output = output[output.find('\n') + 1: -5]

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

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]

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

     if os.path.exists(out_expected_file):
       with open(out_expected_file, 'r') as f:
         expectation = f.read()
     else:
       # Instructions on how to create the expectation will be printed below
       # (outout != expectation)
       print 'File %s was not found' % out_expected_file
       expectation = None
 
     # Report test status using the format test.dart expects to see from DRT.
     print 'Content-Type: text/plain'
     if expectation == output:
       print 'PASS'
       print 'Expectation matches'
     else:
       # Generate a temporary file in the same place as the .html file:
       out_file = test_file[:test_file.rfind('.html')] + out_expected_file[-4:]
       with open(out_file, 'w') as f:
         f.write(output)
       print 'FAIL'
       print 'Expectation didn\'t match. Update expectations by running:\n'
       print 'cp %s %s\n' % (out_file, out_expected_file)
     print '#EOF'
   else:
     # Pipe through the output for non-layout tests.
-    sys.stdout.write(p.stdout.read())
+    print output
 
 if __name__ == '__main__':
   try:
     sys.exit(main(sys.argv))
   except StandardError as e:
     print 'Fail: ' + str(e)
     sys.exit(1)