Initial support to test.dart for running precompiler tests on android devices

tools/testing/dart/android.dart

Index: tools/testing/dart/android.dart
diff --git a/tools/testing/dart/android.dart b/tools/testing/dart/android.dart
index 1d390715733707bd716b615c676b5b17a7e6d3ee..cc9d6e026a919df6aba08af908ac849bb42aa83d 100644
--- a/tools/testing/dart/android.dart
+++ b/tools/testing/dart/android.dart
@@ -12,14 +12,37 @@ import "dart:io";
 import "path.dart";
 import "utils.dart";
 
+class AdbCommandResult {
+  final String command;
+  final String stdout;
+  final String stderr;
+  final int exitCode;
+
+  AdbCommandResult(this.command, this.stdout, this.stderr, this.exitCode);
+}
+
 Future _executeCommand(String executable, List<String> args,
     [String stdin = ""]) {
-  return _executeCommandRaw(executable, args, stdin).then((results) => null);
+  return _executeCommandRaw(executable, args, stdin).then((result) {
+    _throwIfFailedAdbCommand(result);
+  });
 }
 
-Future _executeCommandGetOutput(String executable, List<String> args,
-    [String stdin = ""]) {
-  return _executeCommandRaw(executable, args, stdin).then((output) => output);
+Future<String> _executeCommandGetOutput(String executable, List<String> args,

[Bill Hesse, 2016/04/27 12:22:31]

Do we really need two functions, especially since these are private?
Just one returning Future<AdbCommandResult> should be fine.  Getting the string
is simple, just .stdout.


Also,
If you make the throwIfFailed a method on AdbCommandResult:

return (await _executeCommandRaw(...))..throwIfFailed();

[kustermann, 2016/04/27 13:25:08]

Done.

+                                        [String stdin = ""]) async {
+  AdbCommandResult result = await _executeCommandRaw(executable, args, stdin);
+  _throwIfFailedAdbCommand(result);
+  return result.stdout;
+}
+
+void _throwIfFailedAdbCommand(AdbCommandResult result) {
+  if (result.exitCode != 0) {
+    var error = "Running: ${result.command} failed:"
+        "stdout: \n ${result.stdout}"
+        "stderr: \n ${result.stderr}"
+        "exitCode: \n ${result.exitCode}";
+    throw new Exception(error);
+  }
 }
 
 /**
@@ -29,8 +52,8 @@ Future _executeCommandGetOutput(String executable, List<String> args,
  * If the exit code of the process was nonzero it will complete with an error.
  * If starting the process failed, it will complete with an error as well.
  */
-Future _executeCommandRaw(String executable, List<String> args,
-    [String stdin = ""]) {
+Future<AdbCommandResult> _executeCommandRaw(
+    String executable, List<String> args, [String stdin = ""]) {
   Future<String> getOutput(Stream<List<int>> stream) {
     return stream
         .transform(UTF8.decoder)
@@ -38,7 +61,6 @@ Future _executeCommandRaw(String executable, List<String> args,
         .then((data) => data.join(""));
   }
 
-  DebugLogger.info("Running: '\$ $executable ${args.join(' ')}'");
   return Process.start(executable, args).then((Process process) {
     if (stdin != null && stdin != '') {
       process.stdin.write(stdin);
@@ -51,17 +73,8 @@ Future _executeCommandRaw(String executable, List<String> args,
       process.exitCode
     ];

[Bill Hesse, 2016/04/27 12:22:31]

I think this is much simpler with async-await.
We don't even need to run in parallel, but we can:
var stdput = getOutput(process.stdout);
..

return new AdbCommandResult(command, await stdout, await stderr, await
process.exitCode); 

[kustermann, 2016/04/27 13:25:08]

If we don't drain stdout/stderr, then the buffer runs full and it can lead to a
deadlock. We need therefore to drain them in parallel.

[Bill Hesse, 2016/04/27 13:54:40]

The draining in parallel does happen, if we do the functions calls, and then
just await on variables holding the results.

     return Future.wait(futures).then((results) {
-      bool success = results[2] == 0;
-      if (!success) {
-        var error = "Running: '\$ $executable ${args.join(' ')}' failed:"
-            "stdout: \n ${results[0]}"
-            "stderr: \n ${results[1]}"
-            "exitCode: \n ${results[2]}";
-        throw new Exception(error);
-      } else {
-        DebugLogger.info("Success: $executable finished");
-      }
-      return results[0];
+      String command = '$executable ${args.join(' ')}';
+      return new AdbCommandResult(command, results[0], results[1], results[2]);
     });
   });
 }
@@ -299,22 +312,50 @@ class AdbDevice {
     return _adbCommand(arguments);
   }
 
-  Future _adbCommand(List<String> adbArgs) {
-    if (_deviceId != null) {
-      var extendedAdbArgs = ['-s', _deviceId];
-      extendedAdbArgs.addAll(adbArgs);
-      adbArgs = extendedAdbArgs;
+  Future<AdbCommandResult> runAdbCommand(List<String> adbArgs) {
+    return _executeCommandRaw("adb", _deviceSpecificArgs(adbArgs));
+  }
+
+  Future<AdbCommandResult> runAdbShellCommand(List<String> shellArgs) async {
+    const MARKER = 'AdbShellExitCode: ';
+
+    // The exitcode of 'adb shell ...' can be 0 even though the command failed
+    // with a non-zero exit code. We therefore explicitly print it to stdout and
+    // search for it.
+
+    var args = ['shell',
+                "${shellArgs.join(' ')} ; echo $MARKER \$?"];
+    AdbCommandResult result = await _executeCommandRaw(
+        "adb", _deviceSpecificArgs(args));
+    int exitCode = result.exitCode;
+    var lines = result
+        .stdout.split('\n')
+        .where((line) => line.trim().length > 0)
+        .toList();
+    if (lines.length > 0) {
+      int index = lines.last.indexOf(MARKER);
+      assert(index >= 0);
+      exitCode = int.parse(lines.last.substring(index + MARKER.length).trim());
     }
-    return _executeCommand("adb", adbArgs);
+    return new AdbCommandResult(
+        result.command, result.stdout, result.stderr, exitCode);
+  }
+
+  Future _adbCommand(List<String> adbArgs) {

[Bill Hesse, 2016/04/27 12:22:31]

These two could also be united to one that returns Future<AdbCommandResult>. 
Then the factoring of deviceSpecificArgs isn't as necessary.

[kustermann, 2016/04/27 13:25:09]

Done.

+    return _executeCommand("adb", _deviceSpecificArgs(adbArgs));
   }
 
   Future<String> _adbCommandGetOutput(List<String> adbArgs) {
+    return _executeCommandGetOutput("adb", _deviceSpecificArgs(adbArgs));
+  }
+
+  List<String> _deviceSpecificArgs(List<String> adbArgs) {
     if (_deviceId != null) {
       var extendedAdbArgs = ['-s', _deviceId];
       extendedAdbArgs.addAll(adbArgs);
       adbArgs = extendedAdbArgs;
     }
-    return _executeCommandGetOutput("adb", adbArgs);
+    return adbArgs;
   }
 }
 
@@ -350,3 +391,43 @@ class Intent {
 
   Intent(this.action, this.package, this.activity, [this.dataUri]);
 }
+
+/**
+ * Discovers all available devices and supports aquire/release.

[Bill Hesse, 2016/04/27 12:22:31]

acquire

[kustermann, 2016/04/27 13:25:08]

Done.

+ */
+class AdbDevicePool {
+  List<AdbDevice> _idleDevices;
+  List<Completer> _waiter = [];
+
+  AdbDevicePool(this._idleDevices);
+
+  static Future<AdbDevicePool> create() async {
+    var names = await AdbHelper.listDevices();
+    var devices = names.map((id) => new AdbDevice(id)).toList();
+    if (devices.length == 0) {
+      throw new Exception(
+          'No android devices found. '
+          'Please make sure "adb devices" shows your device!');
+    }
+    return new AdbDevicePool(devices);
+  }
+
+  Future<AdbDevice> aquireDevice() async {

[Bill Hesse, 2016/04/27 12:22:31]

acquire

[kustermann, 2016/04/27 13:25:09]

Done.

+    if (_idleDevices.length > 0) {
+      return _idleDevices.removeLast();
+    } else {
+      var completer = new Completer();
+      _waiter.add(completer);
+      return completer.future;
+    }
+  }
+
+  void releaseDevice(AdbDevice device) {
+    if (_waiter.length > 0) {
+      Completer completer = _waiter.removeLast();

[Bill Hesse, 2016/04/27 12:22:31]

Both idle devices and waiting requests are LIFO.  This seems worse than FIFO. 
Using queues rather than list seems worth it.

[kustermann, 2016/04/27 13:25:08]

Fair enough. Though we have just about 4 devices and we don't care about LIFO vs
FIFO.

+      completer.complete(device);
+    } else {
+      _idleDevices.add(device);
+    }
+  }
+}