Call DumpRenderTree directly from test.dart instead of using the drt-trampoline.py indirection

tools/testing/dart/test_runner.dart

Index: tools/testing/dart/test_runner.dart
diff --git a/tools/testing/dart/test_runner.dart b/tools/testing/dart/test_runner.dart
index 335766a1bc63f1f59a1bd9726241f09026991a86..910170cf70b8295716b286ed4496baa360bb1109 100644
--- a/tools/testing/dart/test_runner.dart
+++ b/tools/testing/dart/test_runner.dart
@@ -32,11 +32,14 @@ class Command {
 
   /** Command line arguments to the executable. */
   List<String> arguments;
+  
+  /** Environment for the command */
+  Map<String,String> environment;
 
   /** The actual command line that will be executed. */
   String commandLine;
 
-  Command(this.executable, this.arguments) {
+  Command(this.executable, this.arguments, [this.environment = null]) {
     if (Platform.operatingSystem == 'windows') {
       // Windows can't handle the first command if it is a .bat file or the like
       // with the slashes going the other direction.
@@ -49,6 +52,8 @@ class Command {
   String toString() => commandLine;
 
   Future<bool> get outputIsUpToDate => new Future.immediate(false);
+  Path get expectedOutputFile => null;
+  bool get isPixelTest => false;

[ricow1, 2012/12/17 13:45:57]

file a bug for us to make this nicer by using polymorphic calls to get the right
output (it is not currently possible with the structure we have for commands and
commandoutput)

[kustermann, 2012/12/17 14:00:47]

Done. Issue 7453

 }
 
 class Dart2JsCommand extends Command {
@@ -105,6 +110,55 @@ class Dart2JsCommand extends Command {
   }
 }
 
+class DumpRenderTreeCommand extends Command {
+  /**
+   * If [expectedOutputPath] is set, the output of DumpRenderTree is compared
+   * with the content of [expectedOutputPath].
+   * This is used for example for pixel tests, where [expectedOutputPath] points
+   * to a *png file.
+   */
+  Path expectedOutputPath;
+
+  DumpRenderTreeCommand(String executable,
+                        String htmlFile,
+                        List<String> options,
+                        List<String> dartFlags,
+                        Uri packageRootUri,
+                        Path this.expectedOutputPath)
+      : super(executable,
+              _getArguments(options, htmlFile),
+              _getEnvironment(dartFlags, packageRootUri));
+
+  static Map _getEnvironment(List<String> dartFlags, Uri packageRootUri) {
+    var needDartFlags = dartFlags != null && dartFlags.length > 0;
+    var needDartPackageRoot = packageRootUri != null;
+
+    var env = null;
+    if (needDartFlags || needDartPackageRoot) {
+      var env = new Map.from(Platform.environment);
+      if (needDartFlags) {
+        env['DART_FLAGS'] = Strings.join(dartFlags, " ");
+      }
+      if (needDartPackageRoot) {
+        env['DART_PACKAGE_ROOT'] = packageRootUri.toString();
+      }
+    }
+
+    return env;
+  }
+
+  static List<String> _getArguments(List<String> options, String htmlFile) {
+    var arguments = new List.from(options);
+    arguments.add(htmlFile);
+    return arguments;
+  }
+  
+  Path get expectedOutputFile => expectedOutputPath;
+  bool get isPixelTest => (expectedOutputFile != null &&
+                           expectedOutputFile.filename.endsWith(".png"));
+}
+
+
 /**
  * TestCase contains all the information needed to run a test and evaluate
  * its output.  Running a test involves starting a separate process, with
@@ -317,8 +371,8 @@ abstract class CommandOutput {
                                  int exitCode,
                                  bool incomplete,
                                  bool timedOut,
-                                 List<String> stdout,
-                                 List<String> stderr,
+                                 List<int> stdout,
+                                 List<int> stderr,
                                  Duration time,
                                  bool compilationSkipped) {
     return new CommandOutputImpl.fromCase(testCase,
@@ -332,6 +386,8 @@ abstract class CommandOutput {
                                           compilationSkipped);
   }
 
+  Command get command;
+
   bool get incomplete;
 
   String get result;
@@ -350,9 +406,9 @@ abstract class CommandOutput {
 
   int get exitCode;
 
-  List<String> get stdout;
+  List<int> get stdout;
 
-  List<String> get stderr;
+  List<int> get stderr;
 
   List<String> get diagnostics;
 
@@ -360,6 +416,7 @@ abstract class CommandOutput {
 }
 
 class CommandOutputImpl implements CommandOutput {
+  Command command;
   TestCase testCase;
   int exitCode;
 
@@ -368,8 +425,8 @@ class CommandOutputImpl implements CommandOutput {
 
   bool timedOut;
   bool failed = false;
-  List<String> stdout;
-  List<String> stderr;
+  List<int> stdout;
+  List<int> stderr;
   Duration time;
   List<String> diagnostics;
   bool compilationSkipped;
@@ -389,12 +446,12 @@ class CommandOutputImpl implements CommandOutput {
   // Don't call this constructor, call CommandOutput.fromCase() to
   // get a new TestOutput instance.
   CommandOutputImpl(TestCase this.testCase,
-                    Command command,
+                    Command this.command,
                     int this.exitCode,
                     bool this.incomplete,
                     bool this.timedOut,
-                    List<String> this.stdout,
-                    List<String> this.stderr,
+                    List<int> this.stdout,
+                    List<int> this.stderr,
                     Duration this.time,
                     bool this.compilationSkipped) {
     testCase.commandOutputs[command] = this;
@@ -405,8 +462,8 @@ class CommandOutputImpl implements CommandOutput {
                                      int exitCode,
                                      bool incomplete,
                                      bool timedOut,
-                                     List<String> stdout,
-                                     List<String> stderr,
+                                     List<int> stdout,
+                                     List<int> stderr,
                                      Duration time,
                                      bool compilationSkipped) {
     if (testCase is BrowserTestCase) {
@@ -504,9 +561,10 @@ class BrowserCommandOutputImpl extends CommandOutputImpl {
     // and the virtual framebuffer X server didn't hook up, or DRT crashed with
     // a core dump. Sometimes DRT crashes after it has set the stdout to PASS,
     // so we have to do this check first.
-    for (String line in super.stderr) {
+    var stderrLines = new String.fromCharCodes(super.stderr).split("\n");
+    for (String line in stderrLines) {
       if (line.contains('Gtk-WARNING **: cannot open display: :99') ||
-        line.contains('Failed to run command. return code=1')) {
+          line.contains('Failed to run command. return code=1')) {
         // If we get the X server error, or DRT crashes with a core dump, retry
         // the test.
         if ((testCase as BrowserTestCase).numRetries > 0) {
@@ -516,23 +574,114 @@ class BrowserCommandOutputImpl extends CommandOutputImpl {
       }
     }
 
-    // Browser tests fail unless stdout contains
-    // 'Content-Type: text/plain' followed by 'PASS'.
-    bool has_content_type = false;
-    for (String line in super.stdout) {
-      switch (line) {
-        case 'Content-Type: text/plain':
-          has_content_type = true;
-          break;
-
-        case 'PASS':
-          if (has_content_type) {
-            return (exitCode != 0 && !hasCrashed);
+    if (command.expectedOutputFile != null) {
+      // We are either doing a pixel test or a layout test with DumpRenderTree
+
+      int findBytes(List<int> data, List<int> pattern, [int startPos=0]) {
+        // TODO(kustermann): Use one of the fast string-matching algorithms!
+        for (int i=startPos; i < (data.length-pattern.length); i++) {
+          bool found = true;
+          for (int j=0; j<pattern.length; j++) {
+            if (data[i+j] != pattern[j]) {
+              found = false;
+            }
           }
-          break;
+          if (found) {
+            return i;
+          }
+        }
+        return -1;
       }
+
+      bool areByteArraysEqual(List<int> buffer1,
+                              int buffer1Offset,
+                              List<int> buffer2,
+                              int buffer2Offset,
+                              int count) {
+        if ((buffer1Offset + count) > buffer1.length ||
+            (buffer2Offset + count) > buffer2.length) {
+          return false;
+        }
+
+        for (var i=0; i<count; i++) {
+          if (buffer1[buffer1Offset + i] != buffer2[buffer2Offset + i]) {
+            return false;
+          }
+        }
+        return true;
+      }
+
+      var stdout = testCase.commandOutputs[command].stdout;
+      var file = new File.fromPath(command.expectedOutputFile);
+      if (file.existsSync()) {
+        var bytesContentLength = "Content-Length:".charCodes;
+        var bytesNewLine = "\n".charCodes;
+        var bytesEOF = "#EOF\n".charCodes;
+
+        var expectedContent = file.readAsBytesSync();
+
+        /*
+         * The output of DumpRenderTree is different for pixel tests than for
+         * layout tests.
+         *
+         * On a pixel test, the DRT output has the following format
+         *     ......
+         *     ......
+         *     Content-Length: ...\n
+         *     <*png data>
+         *     #EOF\n
+         * So we need to get the byte-range of the png data first before
+         * comparing it with the content of the expected output file.
+         *
+         * On a layout tests, the DRT output is directly compared with the
+         * content of the expected output directly.
+         */
+        if (command.expectedOutputFile.filename.endsWith(".png")) {
+          var startOfContentLength = findBytes(stdout, bytesContentLength);
+          if (startOfContentLength >= 0) {
+            var newLineAfterContentLength = findBytes(stdout,
+                                                      bytesNewLine,
+                                                      startOfContentLength);
+            if (newLineAfterContentLength > 0) {
+              var startPosition = newLineAfterContentLength + 
+                                  bytesNewLine.length;
+              var endPosition = stdout.length - bytesEOF.length;
+
+              return !areByteArraysEqual(expectedContent,
+                                         0,
+                                         stdout,
+                                         startPosition,
+                                         endPosition - startPosition);
+            }
+          }
+          return true;
+        } else {
+          return !areByteArraysEqual(expectedContent, 0,
+                                     stdout, 0,
+                                     stdout.length);
+        }
+      }
+      return true;
+    } else {
+      // Browser tests fail unless stdout contains
+      // 'Content-Type: text/plain' followed by 'PASS'.
+      bool has_content_type = false;
+      var stdoutLines = new String.fromCharCodes(super.stdout).split("\n");
+      for (String line in stdoutLines) {
+        switch (line) {
+          case 'Content-Type: text/plain':
+            has_content_type = true;
+            break;
+  
+          case 'PASS':
+            if (has_content_type) {
+              return (exitCode != 0 && !hasCrashed);
+            }
+            break;
+        }
+      }
+      return true;
     }
-    return true;
   }
 }
 
@@ -582,7 +731,8 @@ class AnalysisCommandOutputImpl extends CommandOutputImpl {
     List<String> staticWarnings = [];
 
     // Read the returned list of errors and stuff them away.
-    for (String line in super.stderr) {
+    var stderrLines = new String.fromCharCodes(super.stderr).split("\n");
+    for (String line in stderrLines) {
       if (line.length == 0) continue;
       List<String> fields = splitMachineError(line);
       if (fields[ERROR_LEVEL] == 'ERROR') {
@@ -731,8 +881,9 @@ class RunningProcess {
   bool timedOut = false;
   Date startTime;
   Timer timeoutTimer;
-  List<String> stdout;
-  List<String> stderr;
+  List<int> stdout;
+  List<int> stderr;
+  List<String> notifications;
   bool compilationSkipped;
   bool allowRetries;
 
@@ -746,6 +897,8 @@ class RunningProcess {
    * Called when all commands are executed.
    */
   void testComplete(CommandOutput lastCommandOutput) {
+    var command = lastCommandOutput.command;
+
     if (timeoutTimer != null) {
       timeoutTimer.cancel();
     }
@@ -753,8 +906,14 @@ class RunningProcess {
         && testCase.configuration['verbose'] != null
         && testCase.configuration['verbose']) {
       print(testCase.displayName);
-      for (var line in lastCommandOutput.stderr) print(line);
-      for (var line in lastCommandOutput.stdout) print(line);
+      print('');
+      if (notifications.length > 0) {
+        print("Notifications:");
+        for (var line in notifications) {
+          print(notifications);
+        }
+        print('');
+      }
     }
     if (allowRetries && testCase.usesWebDriver
         && lastCommandOutput.unexpectedOutput
@@ -796,14 +955,14 @@ class RunningProcess {
       testComplete(createCommandOutput(command, exitCode, false));
     } else if (exitCode != 0) {
       // One of the steps failed.
-      stderr.add('test.dart: Compilation failed$suffix, exit code $exitCode\n');
+      notifications.add('test.dart: Compilation failed$suffix, '
+                        'exit code $exitCode\n');
       testComplete(createCommandOutput(command, exitCode, true));
     } else {
       createCommandOutput(command, exitCode, true);
       // One compilation step successfully completed, move on to the
       // next step.
-      stderr.add('test.dart: Compilation finished $suffix\n');
-      stdout.add('test.dart: Compilation finished $suffix\n');
+      notifications.add('test.dart: Compilation finished $suffix\n\n');
       if (currentStep == totalSteps - 1 && testCase.usesWebDriver &&
           !testCase.configuration['noBatch']) {
         // Note: processQueue will always be non-null for runtime == ie9, ie10,
@@ -826,6 +985,7 @@ class RunningProcess {
   CommandOutput createCommandOutput(Command command,
                                     int exitCode,
                                     bool incomplete) {
+    // FIXME(kustermann): should we also include this.notifications ??
     var commandOutput = new CommandOutput.fromCase(
         testCase,
         command,
@@ -841,22 +1001,25 @@ class RunningProcess {
   }
 
   void resetLocalOutputInformation() {
-    stdout = new List<String>();
-    stderr = new List<String>();
+    stdout = new List<int>();
+    stderr = new List<int>();
+    notifications = new List<String>();
     compilationSkipped = false;
   }
 
-  VoidFunction makeReadHandler(StringInputStream source,
-                               List<String> destination) {
-    void handler () {
-      if (source.closed) return;  // TODO(whesse): Remove when bug is fixed.
-      var line = source.readLine();
-      while (null != line) {
-        destination.add(line);
-        line = source.readLine();
+  void drainStream(InputStream source, List<int> destination) {
+    void onDataHandler () {
+      if (source.closed) {
+        return;  // TODO(whesse): Remove when bug is fixed.
+      }
+      var data = source.read();
+      while (data != null) {
+        destination.addAll(data);
+        data = source.read();
       }
     }
-    return handler;
+    source.onData = onDataHandler;
+    source.onClosed = onDataHandler;
   }
 
   void start() {
@@ -874,14 +1037,20 @@ class RunningProcess {
 
     command.outputIsUpToDate.then((bool isUpToDate) {
       if (isUpToDate) {
-        stdout.add("Skipped compilation because the old output is "
-                   "still up to date!");
+        notifications.add("Skipped compilation because the old output is "
+                          "still up to date!");
         compilationSkipped = true;
         commandComplete(command, 0);
       } else {
         ProcessOptions options = new ProcessOptions();
-        options.environment =
-            new Map<String, String>.from(Platform.environment);
+        if (command.environment != null) {
+          options.environment =
+              new Map<String, String>.from(command.environment);
+        } else {
+          options.environment =
+              new Map<String, String>.from(Platform.environment);
+        }
+
         options.environment['DART_CONFIGURATION'] =
             TestUtils.configurationDir(testCase.configuration);
         Future processFuture = Process.start(command.executable,
@@ -890,12 +1059,8 @@ class RunningProcess {
         processFuture.then((Process p) {
           process = p;
           process.onExit = processExitHandler;
-          var stdoutStringStream = new StringInputStream(process.stdout);
-          var stderrStringStream = new StringInputStream(process.stderr);
-          stdoutStringStream.onLine =
-              makeReadHandler(stdoutStringStream, stdout);
-          stderrStringStream.onLine =
-              makeReadHandler(stderrStringStream, stderr);
+          drainStream(process.stdout, stdout);
+          drainStream(process.stderr, stderr);
           if (timeoutTimer == null) {
             // Create one timeout timer when starting test case, remove it at
             // the end.
@@ -951,8 +1116,8 @@ class BatchRunnerProcess {
   StringInputStream _stderrStream;
 
   TestCase _currentTest;
-  List<String> _testStdout;
-  List<String> _testStderr;
+  List<int> _testStdout;
+  List<int> _testStderr;
   String _status;
   bool _stdoutDrained = false;
   bool _stderrDrained = false;
@@ -1029,9 +1194,15 @@ class BatchRunnerProcess {
     _stdoutDrained = false;
     _stderrDrained = false;
     _ignoreStreams = new MutableValue<bool>(false);  // Captured by closures.
-    _stdoutStream.onLine = _readStdout(_stdoutStream, _testStdout);
-    _stderrStream.onLine = _readStderr(_stderrStream, _testStderr);
+    _readStdout(_stdoutStream, _testStdout);
+    _readStderr(_stderrStream, _testStderr);
     _timer = new Timer(testCase.timeout * 1000, _timeoutHandler);
+
+    if (testCase.commands.last.environment != null) {
+      print("Warning: command.environment != null, but we don't support custom "
+            "environments for batch runner tests!");
+    }
+
     var line = _createArgumentsLine(testCase.batchTestArguments);
     _process.stdin.onError = (err) {
       print('Error on batch runner input stream stdin');
@@ -1081,9 +1252,9 @@ class BatchRunnerProcess {
     if (_stderrDrained) _reportResult();
   }
 
-  VoidFunction _readStdout(StringInputStream stream, List<String> buffer) {
+  void _readStdout(StringInputStream stream, List<int> buffer) {
     var ignoreStreams = _ignoreStreams;  // Capture this mutable object.
-    void reader() {
+    void onLineHandler() {
       if (ignoreStreams.value) {
          while (stream.readLine() != null) {
           // Do nothing.
@@ -1100,7 +1271,7 @@ class BatchRunnerProcess {
         } else if (line.startsWith('>>> ')) {
           throw new Exception('Unexpected command from dartc batch runner.');
         } else {
-          buffer.add(line);
+          buffer.addAll("$line\n".charCodes);
         }
         line = stream.readLine();
       }
@@ -1109,12 +1280,12 @@ class BatchRunnerProcess {
         _stdoutDone();
       }
     }
-    return reader;
+    stream.onLine = onLineHandler;
   }
 
-  VoidFunction _readStderr(StringInputStream stream, List<String> buffer) {
+  void _readStderr(StringInputStream stream, List<int> buffer) {
     var ignoreStreams = _ignoreStreams;  // Capture this mutable object.
-    void reader() {
+    void onLineHandler() {
       if (ignoreStreams.value) {
         while (stream.readLine() != null) {
           // Do nothing.
@@ -1127,12 +1298,12 @@ class BatchRunnerProcess {
         if (line.startsWith('>>> EOF STDERR')) {
           _stderrDone();
         } else {
-          buffer.add(line);
+          buffer.addAll("$line\n".charCodes);
         }
         line = stream.readLine();
       }
     }
-    return reader;
+    stream.onLine = onLineHandler;
   }
 
   ExitCodeEvent makeExitHandler(String status) {