Fix coverage test

tests/standalone/coverage_test.dart

 // Copyright (c) 2013, 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.
 
 // This test forks a second vm process that runs the script tools/coverage.dart
 // and verifies that the coverage tool produces its expected output.
 // This test is mainly here to ensure that the coverage tool compiles and
 // runs.
 
 import "dart:io";
 import "dart:utf";
 
 // Coverage tool script relative to the path of this test.
 var coverageToolScript = "../../tools/coverage.dart";
 
 // Coverage target script relative to this test.
 var coverageTargetScript = "../language/hello_dart_test.dart";
 var targPath;
 
 Process coverageToolProcess;
 List sourceLines;
 int nextLineToMatch = 0;
 
 void onCoverageOutput(String line) {
   print("COV: $line");
   if (nextLineToMatch < sourceLines.length) {

[ricow1, 2013/06/03 19:05:57]

don't we want an exact match? what should happen if nextLineToMatch >
sourceLines.length?

[hausner, 2013/06/03 20:48:08]

A simple empty line from the coverage tool output could accidentally match
an empty line in the source text. I made the match a bit more stringent
and expect an exact match. It's still not completely accurate, but good enough
for what i had in mind. I just want to test that the coverage tool compiles and
runs so it does not bit-rot.

     if (line.endsWith(sourceLines[nextLineToMatch])) {
       nextLineToMatch++;
     }
   }
 }
 
 void onCoverageExit(exitCode) {
   var pid = coverageToolProcess.pid;
-  print("process $pid terminated with exit code $exitCode.");
+  print("Coverage tool process (pid $pid) terminated with exit code $exitCode.");

[ricow1, 2013/06/03 19:05:57]

long line and I assume that this is for debugging purposes only?

[hausner, 2013/06/03 20:48:08]

Yes, for debugging. But it doesn't hurt to leave it, right? Shortened line.

[ricow1, 2013/06/04 05:49:43]

Nope, that is perfectly fine, just wanted to make sure :-)

+}
+
+void checkSuccess() {
   if (nextLineToMatch < sourceLines.length) {
     print("Error: could not match all source code lines of '$targPath'");
     exit(-1);
   } else {
     print("Successfully matched all lines of '$targPath'");
   }
 }
 
+void onStdoutError(err) {
+  print("Error on coverage tool stdout: err");
+  checkSuccess();
+}
+
 void main() {
   var options = new Options();
 
   // Compute paths for coverage tool and coverage target relative
   // the the path of this script.
   var scriptPath = new Path(options.script).directoryPath;
   var toolPath = scriptPath.join(new Path(coverageToolScript)).canonicalize();
   targPath = scriptPath.join(new Path(coverageTargetScript)).canonicalize();
 
   sourceLines = new File(targPath.toNativePath()).readAsLinesSync();
   assert(sourceLines != null);
 
   var processOpts = [ "--compile_all",
                       toolPath.toNativePath(),
                       targPath.toNativePath() ];
 
   Process.start(options.executable, processOpts).then((Process process) {
     coverageToolProcess = process;
     coverageToolProcess.stdin.close();
     var stdoutStringStream = coverageToolProcess.stdout
         .transform(new StringDecoder())
         .transform(new LineTransformer());
-    stdoutStringStream.listen(onCoverageOutput);
+    stdoutStringStream.listen(onCoverageOutput,
+                              onError: onStdoutError,
+                              onDone: checkSuccess);
 
     var stderrStringStream = coverageToolProcess.stderr
         .transform(new StringDecoder())
         .transform(new LineTransformer());
     stderrStringStream.listen(onCoverageOutput);

[ricow1, 2013/06/03 19:05:57]

you can potentially get output on stderr after you have "determined" that you
where successful, i.e., the expected output is on stdout and you got it all, so
checkSuccess is called from the callback above. Now you get the bad output here,
which will not be considered since we just ignore it in onCoverageOutput

[hausner, 2013/06/03 20:48:08]

Fair point. I changed this to use 3 futures, checking the match after all 3
futures complete.

 
     coverageToolProcess.exitCode.then(onCoverageExit);
   });
 }