test.py: Sending JSON between test_controller.js <-> browser_controller

dart/tools/testing/dart/test_runner.dart

Index: dart/tools/testing/dart/test_runner.dart
diff --git a/dart/tools/testing/dart/test_runner.dart b/dart/tools/testing/dart/test_runner.dart
index b83d4a7711809b1a322628ef1633bbe71f0e8009..05a9f91c7af5fa609f5d8c3467c03c1260812a7f 100644
--- a/dart/tools/testing/dart/test_runner.dart
+++ b/dart/tools/testing/dart/test_runner.dart
@@ -13,7 +13,7 @@ library test_runner;
 
 import "dart:async";
 import "dart:collection" show Queue;
-import "dart:convert" show LineSplitter, UTF8;
+import "dart:convert" show LineSplitter, UTF8, JSON;
 // We need to use the 'io' prefix here, otherwise io.exitCode will shadow
 // CommandOutput.exitCode in subclasses of CommandOutput.
 import "dart:io" as io;
@@ -874,6 +874,165 @@ class HTMLBrowserCommandOutputImpl extends BrowserCommandOutputImpl {
   }
 }
 
+class BrowserControllerTestOutcome extends CommandOutputImpl
+                                   with UnittestSuiteMessagesMixin {
+  BrowserTestOutput _result;
+  String _outcomeStr;
+
+  factory BrowserControllerTestOutcome(

[ricow1, 2013/11/15 07:26:33]

120 line factory constructor, could we please extract some of this into separate
functions?

[kustermann, 2013/11/19 10:06:10]

Added BrowserTestJsonResult class + static method for parsing.

+      Command command, BrowserTestOutput result) {

[ricow1, 2013/11/14 14:42:27]

move up

[kustermann, 2013/11/19 10:06:10]

It doesn't fit.

+    void validate(String assertion, bool value) {
+      if (!value) {
+        throw "InvalidFormat sent from browser driving page: $assertion:\n\n"
+               "${result.lastKnownMessage}";
+      }
+    }
+
+    String indent(String string, int numSpaces) {

[ricow1, 2013/11/14 14:42:27]

there is something wrong with our language, this is so much nicer in python :-)

[kustermann, 2013/11/19 10:06:10]

Well, we could emulate that behaviour, but I don't want to do it in this CL,
it's already big enough.

But yes, ContextManagers rock :)

+      var SPACE = ' ';
+      var spaces = '';
+      for (var i = 0; i < numSpaces; i++) spaces += SPACE;

[Bill Hesse, 2013/11/15 11:24:03]

How about

var spaces = new List.filled(numSpaces, ' ').join();
var result = string.split('\r\n')
                   .expand((line) => line.split('\n'))
                   .expand((line) => ['\n',spaces, line])
                   .skip(1)
                   .join();
return result;

[kustermann, 2013/11/19 10:06:10]

How about:
      var spaces = new List.filled(numSpaces, ' ').join('');
      return string.replaceAll('\r\n', '\n')
          .split('\n')
          .map((line) => "$spaces$line")
          .join('\n');

+
+      var buffer = new StringBuffer();
+      var lines = string.replaceAll('\r', '').split('\n');
+      for (var i = 0; i < lines.length; i++) {

[ricow1, 2013/11/14 14:42:27]

how about something like
return lines.map((e) => "$spaces$e").join('\n')

+        buffer.write("$spaces${lines[i]}");
+        if (i != lines.length) {
+          buffer.write('\n');
+        }
+      }
+      return buffer.toString();
+    }
+
+
+    String stdout = "";
+    String stderr = "";
+    String outcomeStr;
+
+    var events;
+    try {
+      events = JSON.decode(result.lastKnownMessage);
+    } catch(error) {}

[ricow1, 2013/11/14 14:42:27]

we should do something if there is an error right?

[kustermann, 2013/11/19 10:06:10]

No. If there was an error, "events == null" and we will fallback to the "search
for FAIL/PASS in the message"-approach.

+
+    // We got the new data format if we get JSON data.
+    if (events != null) {
+      const ALLOWED_TYPES =
+          const ['debug', 'print', 'dom', 'test_outcome', 'message_received'];
+      const ALLOWED_OUTCOMES = const ['PASS', 'FAIL', 'NOT_STARTED'];
+      const REQUIRED_TYPES = const ['test_outcome', 'dom'];
+
+      validate("Message must be a List", events is List);
+
+      var testOutcome, htmlDom;
+      var debugMessages = [], dartPrints = [], receivedMessages = [];
+      for (var entry in events) {
+        validate("An entry must be a Map", entry is Map);
+        validate("'type' of an entry must be a String", entry['type'] is String);

[ricow1, 2013/11/15 07:26:33]

long line

[kustermann, 2013/11/19 10:06:10]

Done.

+        validate("'type' as to be in $ALLOWED_TYPES.",

[Bill Hesse, 2013/11/15 11:24:03]

has to be

[kustermann, 2013/11/19 10:06:10]

Done.

+                 ALLOWED_TYPES.contains(entry['type']));
+        validate("'timestamp' of an entry must be a number",
+            entry['timestamp'] is int || entry['timestamp'] is double);
+        switch(entry['type']) {
+          case 'debug':
+            debugMessages.add(entry['value']);
+            break;
+          case 'print':
+            dartPrints.add(entry['value']);

[Bill Hesse, 2013/11/15 11:24:03]

dartPrints is a little weird.  "printedByDart", "dartPrintedMessages",
"dartPrinted"?

[kustermann, 2013/11/19 10:06:10]

Done.

+            break;
+          case 'message_received':
+            receivedMessages.add(entry['value']);
+            break;
+          case 'dom':
+            validate("The message can only contain one 'dom' type.",
+                     htmlDom == null);
+            htmlDom = entry['value'];
+            break;
+          case 'test_outcome':
+            validate("Test outcome has to be one of $ALLOWED_OUTCOMES.",
+                     ALLOWED_OUTCOMES.contains(entry['value']));
+            // We require that we get only one test_outcome
+            validate("The message can only contain one 'test_outcome'.",
+                     testOutcome == null);
+            testOutcome = entry['value'];
+            break;
+        }
+      }
+      validate("The message must have a 'test_outcome' and a 'dom'.",
+               testOutcome != null && htmlDom != null);
+      outcomeStr = testOutcome;

[Bill Hesse, 2013/11/15 11:24:03]

outcome or outcomeString?

+    } else {
+      // Old determining of failures/passes
+      if (result.lastKnownMessage.contains("FAIL")) {
+        outcomeStr = "FAIL";
+      } else if (result.lastKnownMessage.contains("PASS")) {
+        outcomeStr = "PASS";
+      } else {
+        outcomeStr = "FAIL";
+      }
+    }
+
+    if (result.didTimeout) {
+      if (result.delayUntilTestStarted != null) {
+        stderr = "This test timed out. The delay until the test actually "
+                 "started was: ${result.delayUntilTestStarted}.";
+      } else {
+        stderr = "This test has not notified test.py that it started running. "
+                 "This could be a bug in test.py! "

[Bill Hesse, 2013/11/15 11:24:03]

Perhaps a TODO to change this after this a while?

[kustermann, 2013/11/19 10:06:10]

Done.

+                 "Please contact ricow/kustermann";
+      }
+    }
+
+    if (events != null) {
+      stdout = "events:\n${indent(prettifyJson(events), 2)}\n\n";
+    } else {
+      stdout = "message:\n${indent(result.lastKnownMessage, 2)}\n\n";
+    }
+
+    stderr =
+        '$stderr\n\n'
+        'BrowserOutput while running the test (* EXPERIMENTAL *):\n'
+        'BrowserOutput.stdout:\n'
+        '${indent(result.browserOutput.stdout.toString(), 2)}\n'

[Bill Hesse, 2013/11/15 11:24:03]

indent would be easier if it returned the lines ending with \n, and you always
want a \n.

+        'BrowserOutput.stderr:\n'
+        '${indent(result.browserOutput.stderr.toString(), 2)}\n'
+        '\n';
+    return new BrowserControllerTestOutcome._internal(
+      command, result, outcomeStr, encodeUtf8(stdout), encodeUtf8(stderr));
+  }
+
+  BrowserControllerTestOutcome._internal(
+      Command command, BrowserTestOutput result, String outcomeStr,
+      List<int> stdout, List<int> stderr)
+  : super(command, 0, result.didTimeout, stdout, stderr, result.duration,
+          false) {
+    _result = result;
+    _outcomeStr = outcomeStr;
+  }
+
+  Expectation result(TestCase testCase) {
+    // Handle timeouts first
+    if (_result.didTimeout)  return Expectation.TIMEOUT;
+
+    // If the test didn't start, we've a problem. Report it as a failure.
+    if (_outcomeStr == 'NOT_STARTED') return Expectation.FAIL;
+
+    Expectation outcome;
+    if (_outcomeStr == 'FAIL') outcome = Expectation.RUNTIME_ERROR;
+    else if (_outcomeStr == 'PASS') outcome = Expectation.PASS;
+    else throw "error";
+
+    // Multitests are handled specially
+    if (testCase.info != null) {
+      if (testCase.info.hasRuntimeError) {
+        if (outcome == Expectation.RUNTIME_ERROR) return Expectation.PASS;
+        return Expectation.MISSING_RUNTIME_ERROR;
+      }
+    }
+
+    return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
+  }
+}
+
 
 class AnalysisCommandOutputImpl extends CommandOutputImpl {
   // An error line has 8 fields that look like:
@@ -1061,7 +1220,7 @@ class CompilationCommandOutputImpl extends CommandOutputImpl {
 
     // Multitests are handled specially
     if (testCase.info != null) {
-      if (testCase.info.hasCompileError) {
+    if (testCase.info.hasCompileError) {

[Bill Hesse, 2013/11/15 11:24:03]

indentation

         // Nonzero exit code of the compiler means compilation failed
         // TODO(kustermann): Do we have a special exit code in that case???
         if (exitCode != 0) {
@@ -1853,31 +2012,8 @@ class CommandExecutorImpl implements CommandExecutor {
     var completer = new Completer<CommandOutput>();
 
     var callback = (BrowserTestOutput output) {
-      bool timedOut = output.didTimeout;
-      String stderr = "";
-      if (timedOut) {
-        if (output.delayUntilTestStarted != null) {
-          stderr = "This test timed out. The delay until the test actually "
-                   "started was: ${output.delayUntilTestStarted}.";
-        } else {
-          stderr = "This test has not notified test.py that it started running."
-                   " This could be a bug in test.py! "
-                   "Please contact ricow/kustermann";
-        }
-      }
-      stderr =
-          '$stderr\n\n'
-          'BrowserOutput while running the test (* EXPERIMENTAL *):\n'
-          'BrowserOutput.stdout:\n${output.browserOutput.stdout.toString()}\n'
-          'BrowserOutput.stderr:\n${output.browserOutput.stderr.toString()}\n';
-      var commandOutput = createCommandOutput(browserCommand,
-                          0,
-                          timedOut,
-                          encodeUtf8(output.dom),
-                          encodeUtf8(stderr),
-                          output.duration,
-                          false);
-      completer.complete(commandOutput);
+      completer.complete(
+          new BrowserControllerTestOutcome(browserCommand, output));
     };
     BrowserTest browserTest = new BrowserTest(browserCommand.url,
                                               callback,