Add tests for process_manager.dart (#7178)

packages/flutter_tools/lib/src/base/process.dart

@@ -15,13 +15,23 @@ typedef Future<dynamic> ShutdownHook();
 // TODO(ianh): We have way too many ways to run subprocesses in this project.
 
 List<ShutdownHook> _shutdownHooks = <ShutdownHook>[];
+bool _shutdownHooksRunning = false;
 void addShutdownHook(ShutdownHook shutdownHook) {
+  assert(!_shutdownHooksRunning);
   _shutdownHooks.add(shutdownHook);
 }
 
 Future<Null> runShutdownHooks() async {
-  for (ShutdownHook shutdownHook in _shutdownHooks)
-    await shutdownHook();
+  List<ShutdownHook> hooks = new List<ShutdownHook>.from(_shutdownHooks);
+  _shutdownHooks.clear();
+  _shutdownHooksRunning = true;
+  try {
+    for (ShutdownHook shutdownHook in hooks)
+      await shutdownHook();
+  } finally {
+    _shutdownHooksRunning = false;
+  }
+  assert(_shutdownHooks.isEmpty);
 }
 
 Map<String, String> _environment(bool allowReentrantFlutter, [Map<String, String> environment]) {

packages/flutter_tools/lib/src/base/process_manager.dart

@@ -423,7 +423,7 @@ class RecordingProcessManager implements ProcessManager {
 /// A lightweight class that provides a builder pattern for building a
 /// manifest entry.
 class _ManifestEntryBuilder {
-  Map<String, dynamic> entry;
+  Map<String, dynamic> entry = <String, dynamic>{};
 
   /// Adds the specified key/value pair to the manifest entry iff the value
   /// is non-null. If [jsonValue] is specified, its value will be used instead
@@ -602,8 +602,8 @@ class ReplayProcessManager implements ProcessManager {
     try {
       List<Map<String, dynamic>> manifest = new JsonDecoder().convert(content);
       return new ReplayProcessManager._(manifest, dir);
-    } on FormatException {
-      throw new ArgumentError('$_kManifestName is not a valid JSON file.');
+    } on FormatException catch (e) {
+      throw new ArgumentError('$_kManifestName is not a valid JSON file: $e');
     }
   }
 
@@ -835,6 +835,8 @@ class _ReplayProcess implements Process {
   @override
   bool kill([ProcessSignal signal = ProcessSignal.SIGTERM]) {
     if (!_exitCodeCompleter.isCompleted) {
+      _stdoutController.close();
+      _stderrController.close();
       _exitCodeCompleter.complete(_exitCode);
       return true;
     }

packages/flutter_tools/test/all.dart

@@ -33,6 +33,7 @@ import 'install_test.dart' as install_test;
 import 'logs_test.dart' as logs_test;
 import 'os_utils_test.dart' as os_utils_test;
 import 'packages_test.dart' as packages_test;
+import 'process_manager_test.dart' as process_manager_test;
 import 'protocol_discovery_test.dart' as protocol_discovery_test;
 import 'run_test.dart' as run_test;
 import 'stop_test.dart' as stop_test;
@@ -68,6 +69,7 @@ void main() {
   logs_test.main();
   os_utils_test.main();
   packages_test.main();
+  process_manager_test.main();
   protocol_discovery_test.main();
   run_test.main();
   stop_test.main();

packages/flutter_tools/test/data/process_manager/replay/001.sing.100.stderr

@@ -0,0 +1 @@
+Uh, pineapple pen

packages/flutter_tools/test/data/process_manager/replay/001.sing.100.stdout

@@ -0,0 +1,2 @@
+I have a pen
+I have a pineapple

packages/flutter_tools/test/data/process_manager/replay/002.dance.101.stderr

@@ -0,0 +1 @@
+No one can dance like Psy

packages/flutter_tools/test/data/process_manager/replay/MANIFEST.txt

@@ -0,0 +1,23 @@
+[
+  {
+    "pid": 100,
+    "basename": "001.sing.100",
+    "executable": "sing",
+    "arguments": [
+      "ppap"
+    ],
+    "mode": "ProcessStartMode.NORMAL",
+    "exitCode": 0
+  },
+  {
+    "pid": 101,
+    "basename": "002.dance.101",
+    "executable": "dance",
+    "arguments": [
+      "gangnam-style"
+    ],
+    "stdoutEncoding": "system",
+    "stderrEncoding": "system",
+    "exitCode": 2
+  }
+]

packages/flutter_tools/test/process_manager_test.dart

@@ -0,0 +1,206 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:archive/archive.dart';
+import 'package:flutter_tools/src/base/context.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/os.dart';
+import 'package:flutter_tools/src/base/process.dart';
+import 'package:flutter_tools/src/base/process_manager.dart';
+import 'package:path/path.dart' as path;
+import 'package:test/test.dart';
+
+typedef bool Predicate<T>(T item);
+
+/// Decodes a UTF8-encoded byte array into a list of Strings, where each list
+/// entry represents a line of text.
+List<String> _decode(List<int> data) =>
+    const LineSplitter().convert(UTF8.decode(data));
+
+/// Consumes and returns an entire stream of bytes.
+Future<List<int>> _consume(Stream<List<int>> stream) =>
+    stream.expand((List<int> data) => data).toList();
+
+void main() {
+  group('RecordingProcessManager', () {
+    Directory tmp;
+    ProcessManager manager;
+
+    setUp(() {
+      tmp = Directory.systemTemp.createTempSync('flutter_tools_');
+      manager = new RecordingProcessManager(tmp.path);
+    });
+
+    tearDown(() {
+      tmp.deleteSync(recursive: true);
+    });
+
+    test('start', () async {
+      Process process = await manager.start('echo', <String>['foo']);
+      int pid = process.pid;
+      int exitCode = await process.exitCode;
+      List<int> stdout = await _consume(process.stdout);
+      List<int> stderr = await _consume(process.stderr);
+      expect(exitCode, 0);
+      expect(_decode(stdout), <String>['foo']);
+      expect(stderr, isEmpty);
+
+      // Force the recording to be written to disk.
+      await runShutdownHooks();
+
+      _Recording recording = _Recording.readFrom(tmp);
+      expect(recording.manifest, hasLength(1));
+      Map<String, dynamic> entry = recording.manifest.first;
+      expect(entry['pid'], pid);
+      expect(entry['exitCode'], exitCode);
+      expect(recording.stdoutForEntryAt(0), stdout);
+      expect(recording.stderrForEntryAt(0), stderr);
+    });
+
+    test('run', () async {
+      ProcessResult result = await manager.run('echo', <String>['bar']);
+      int pid = result.pid;
+      int exitCode = result.exitCode;
+      String stdout = result.stdout;
+      String stderr = result.stderr;
+      expect(exitCode, 0);
+      expect(stdout, 'bar\n');
+      expect(stderr, isEmpty);
+
+      // Force the recording to be written to disk.
+      await runShutdownHooks();
+
+      _Recording recording = _Recording.readFrom(tmp);
+      expect(recording.manifest, hasLength(1));
+      Map<String, dynamic> entry = recording.manifest.first;
+      expect(entry['pid'], pid);
+      expect(entry['exitCode'], exitCode);
+      expect(recording.stdoutForEntryAt(0), stdout);
+      expect(recording.stderrForEntryAt(0), stderr);
+    });
+
+    test('runSync', () async {
+      ProcessResult result = manager.runSync('echo', <String>['baz']);
+      int pid = result.pid;
+      int exitCode = result.exitCode;
+      String stdout = result.stdout;
+      String stderr = result.stderr;
+      expect(exitCode, 0);
+      expect(stdout, 'baz\n');
+      expect(stderr, isEmpty);
+
+      // Force the recording to be written to disk.
+      await runShutdownHooks();
+
+      _Recording recording = _Recording.readFrom(tmp);
+      expect(recording.manifest, hasLength(1));
+      Map<String, dynamic> entry = recording.manifest.first;
+      expect(entry['pid'], pid);
+      expect(entry['exitCode'], exitCode);
+      expect(recording.stdoutForEntryAt(0), stdout);
+      expect(recording.stderrForEntryAt(0), stderr);
+    });
+  });
+
+  group('ReplayProcessManager', () {
+    ProcessManager manager;
+
+    setUp(() async {
+      await runInMinimalContext(() async {
+        Directory dir = new Directory('test/data/process_manager/replay');
+        manager = await ReplayProcessManager.create(dir.path);
+      });
+    });
+
+    tearDown(() async {
+      // Allow the replay manager to clean up
+      await runShutdownHooks();
+    });
+
+    test('start', () async {
+      Process process = await manager.start('sing', <String>['ppap']);
+      int exitCode = await process.exitCode;
+      List<int> stdout = await _consume(process.stdout);
+      List<int> stderr = await _consume(process.stderr);
+      expect(process.pid, 100);
+      expect(exitCode, 0);
+      expect(_decode(stdout), <String>['I have a pen', 'I have a pineapple']);
+      expect(_decode(stderr), <String>['Uh, pineapple pen']);
+    });
+
+    test('run', () async {
+      ProcessResult result = await manager.run('dance', <String>['gangnam-style']);
+      expect(result.pid, 101);
+      expect(result.exitCode, 2);
+      expect(result.stdout, '');
+      expect(result.stderr, 'No one can dance like Psy\n');
+    });
+
+    test('runSync', () {
+      ProcessResult result = manager.runSync('dance', <String>['gangnam-style']);
+      expect(result.pid, 101);
+      expect(result.exitCode, 2);
+      expect(result.stdout, '');
+      expect(result.stderr, 'No one can dance like Psy\n');
+    });
+  });
+}
+
+Future<Null> runInMinimalContext(Future<dynamic> method()) async {
+  AppContext context = new AppContext();
+  context.putIfAbsent(ProcessManager, () => new ProcessManager());
+  context.putIfAbsent(Logger, () => new BufferLogger());
+  context.putIfAbsent(OperatingSystemUtils, () => new OperatingSystemUtils());
+  await context.runInZone(method);
+}
+
+/// A testing utility class that encapsulates a recording.
+class _Recording {
+  final File file;
+  final Archive _archive;
+
+  _Recording(this.file, this._archive);
+
+  static _Recording readFrom(Directory dir) {
+    File file = new File(path.join(
+        dir.path, RecordingProcessManager.kDefaultRecordTo));
+    Archive archive = new ZipDecoder().decodeBytes(file.readAsBytesSync());
+    return new _Recording(file, archive);
+  }
+
+  List<Map<String, dynamic>> get manifest {
+    return JSON.decoder.convert(_getFileContent('MANIFEST.txt', UTF8));
+  }
+
+  dynamic stdoutForEntryAt(int index) =>
+      _getStdioContent(manifest[index], 'stdout');
+
+  dynamic stderrForEntryAt(int index) =>
+      _getStdioContent(manifest[index], 'stderr');
+
+  dynamic _getFileContent(String name, Encoding encoding) {
+    List<int> bytes = _fileNamed(name).content;
+    return encoding == null ? bytes : encoding.decode(bytes);
+  }
+
+  dynamic _getStdioContent(Map<String, dynamic> entry, String type) {
+    String basename = entry['basename'];
+    String encodingName = entry['${type}Encoding'];
+    Encoding encoding;
+    if (encodingName != null)
+      encoding = encodingName == 'system'
+          ? const SystemEncoding()
+          : Encoding.getByName(encodingName);
+    return _getFileContent('$basename.$type', encoding);
+  }
+
+  ArchiveFile _fileNamed(String name) => _archive.firstWhere(_hasName(name));
+
+  Predicate<ArchiveFile> _hasName(String name) =>
+      (ArchiveFile file) => file.name == name;
+}