Add integration test: hot reload + incremental compiler

pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart

Index: pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart
diff --git a/pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart b/pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart
new file mode 100644
index 0000000000000000000000000000000000000000..6b7dfdc9188c6b82f48cb0a6ccc75752f22e1d96
--- /dev/null
+++ b/pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart
@@ -0,0 +1,225 @@
+// Copyright (c) 2017, 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.
+
+/// Integration test that runs the incremental compiler, runs the compiled
+/// program, incrementally rebuild portions of the app, and triggers a hot
+/// reload on the running program.
+library front_end.incremental.hot_reload_e2e_test;
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:front_end/compiler_options.dart';
+import 'package:front_end/file_system.dart';
+import 'package:front_end/incremental_kernel_generator.dart';
+import 'package:front_end/memory_file_system.dart';
+import 'package:front_end/src/incremental/byte_store.dart';
+import 'package:front_end/src/testing/hybrid_file_system.dart';
+import 'package:front_end/src/vm/reload.dart';
+import 'package:kernel/ast.dart';
+import 'package:kernel/binary/limited_ast_to_binary.dart';
+import 'package:test/test.dart';
+
+main() {
+  IncrementalKernelGenerator compiler;
+  MemoryFileSystem fs;
+  Directory outDir;
+  Uri outputUri;
+  List<Future<String>> lines;
+  Future programIsDone;
+
+  setUp(() async {
+    outDir = Directory.systemTemp.createTempSync('hotreload_test');
+    outputUri = outDir.uri.resolve('test.dill');
+    fs = new MemoryFileSystem(Uri.parse('file:///'));
+    writeFile(fs, 'a.dart', sourceA);
+    writeFile(fs, 'b.dart', sourceB);
+    writeFile(fs, '.packages', '');
+    compiler = await createIncrementalCompiler(
+        'file:///a.dart', new HybridFileSystem(fs));
+    await rebuild(compiler, outputUri); // this is a full compile.
+  });
+
+  tearDown(() async {
+    outDir.deleteSync(recursive: true);
+    lines = null;
+  });
+
+  /// Start the VM with the first version of the program compiled by the
+  /// incremental compiler.
+  startProgram(args) async {

[scheglov, 2017/06/07 00:27:23]

Please type "args".

+    var vmArgs = [
+      '--enable-vm-service=0', // Note: use 0 to avoid port collsions.

[scheglov, 2017/06/07 00:27:23]

collisions

+      '--platform=${platformFile.toFilePath()}',
+      outputUri.toFilePath()
+    ];
+    vmArgs.addAll(args);
+    var vm = await Process.start(Platform.executable, vmArgs);
+    var splitter = new LineSplitter();
+
+    /// All tests below expect at most 3 events: reading the observatory port,
+    /// the output before waiting for a reload, and the output after the reload.
+    /// Each is represented by a single line in the output.
+    int i = 0;
+    var completers = new List.generate(3, (_) => new Completer<String>());
+    lines = completers.map((c) => c.future).toList();
+    vm.stdout.transform(UTF8.decoder).transform(splitter).listen((line) {
+      expect(i, lessThan(3));
+      completers[i++].complete(line);
+    });
+
+    vm.stderr.transform(UTF8.decoder).transform(splitter).toList().then((err) {
+      expect(err, isEmpty, reason: err.join('\n'));
+    });
+
+    programIsDone = vm.exitCode;
+  }
+
+  /// Request a hot reload on the running program.
+  hotReload() async {
+    var portLine = await lines[0];
+    expect(observatoryPortRegExp.hasMatch(portLine), isTrue);
+    var match = observatoryPortRegExp.firstMatch(portLine);
+    var port = int.parse(match.group(1));
+    var reloader = new VmReloader(port);
+    var reloadResult = await reloader.reload(outputUri);
+    expect(reloadResult['success'], isTrue);
+    await reloader?.disconnect();
+  }
+
+  test('initial program is valid', () async {
+    await startProgram(["2"]);

[scheglov, 2017/06/07 00:27:23]

What means ["2"] arguments here?
Just a signal to pass some arguments to the test script in order to avoid
waiting for hot reload?
Would it be better to use a boolean flag instead?

+    await programIsDone;
+    expect(await lines.skip(1).first, "part1 part2");
+  });
+
+  test('reload after leaf library modification', () async {
+    await startProgram([]);
+    expect(await lines[1], "part1 part2");
+
+    writeFile(fs, 'b.dart', sourceB.replaceAll("part1", "part3"));
+    await rebuild(compiler, outputUri);
+    await hotReload();
+    await programIsDone;
+    expect(await lines[2], "part3 part2");
+  });
+
+  test('reload after non-leaf library modification', () async {
+    await startProgram([]);
+    expect(await lines[1], "part1 part2");
+
+    writeFile(fs, 'a.dart', sourceA.replaceAll("part2", "part4"));
+    await rebuild(compiler, outputUri);
+    await hotReload();
+    await programIsDone;
+    expect(await lines[2], "part1 part4");
+  }, skip: true /* VM crashes on reload */);

[Siggi Cherem (dart-lang), 2017/06/06 22:44:07]

Siva: I forgot to mention, to reproduce the failure you need to:
 - change this to false or remove this argument.
 - run

    out/ReleaseX64/dart
pkg/front_end/test/src/incremental/hot_reload_e2e_test.dart

+
+  test('reload after whole program modification', () async {
+    await startProgram([]);
+    expect(await lines[1], "part1 part2");
+
+    await hotReload();
+
+    writeFile(fs, 'b.dart', sourceB.replaceAll("part1", "part5"));
+    writeFile(fs, 'a.dart', sourceA.replaceAll("part2", "part6"));
+    await rebuild(compiler, outputUri);
+    await hotReload();
+    await programIsDone;
+    expect(await lines[2], "part5 part6");
+  });

[scheglov, 2017/06/07 00:27:23]

It might be interesting to have a test with more than one reload.

+}
+
+var dartVm = Uri.base.resolve(Platform.resolvedExecutable);
+var sdkRoot = dartVm.resolve("patched_sdk/");
+var platformFile = sdkRoot.resolve('platform.dill');
+
+Future<IncrementalKernelGenerator> createIncrementalCompiler(
+    String entry, FileSystem fs) {
+  var entryUri = Uri.base.resolve(entry);
+  var options = new CompilerOptions()
+    ..sdkRoot = sdkRoot
+    ..sdkSummary = sdkRoot.resolve('outline.dill')
+    ..packagesFileUri = Uri.parse('file:///.packages')
+    ..strongMode = false
+    ..dartLibraries = loadDartLibraries()
+    ..fileSystem = fs
+    ..byteStore = new MemoryByteStore();
+  return IncrementalKernelGenerator.newInstance(options, entryUri);
+}
+
+Map<String, Uri> loadDartLibraries() {
+  var libraries = sdkRoot.resolve('lib/libraries.json');
+  var map =
+      JSON.decode(new File.fromUri(libraries).readAsStringSync())['libraries'];
+  var dartLibraries = <String, Uri>{};
+  map.forEach((k, v) => dartLibraries[k] = libraries.resolve(v));
+  return dartLibraries;
+}
+
+Future<bool> rebuild(IncrementalKernelGenerator compiler, Uri outputUri) async {
+  compiler.invalidate(Uri.parse("file:///a.dart"));
+  compiler.invalidate(Uri.parse("file:///b.dart"));
+  var program = (await compiler.computeDelta()).newProgram;
+  if (program != null && !program.libraries.isEmpty) {
+    await writeProgram(program, outputUri);
+    return true;
+  }
+  return false;
+}
+
+writeProgram(Program program, Uri outputUri) async {
+  // TODO(sigmund): the incremental generator should set the main method
+  // directly.
+  var mainLib = program.libraries.last;
+  program.mainMethod =
+      mainLib.procedures.firstWhere((p) => p.name.name == 'main');
+  var sink = new File.fromUri(outputUri).openWrite();

[scheglov, 2017/06/07 00:27:23]

I can do this.

+
+  // TODO(sigmund): the incremental generator should always filter these
+  // libraries instead.

[scheglov, 2017/06/07 00:27:23]

Is it a requirement for any kernel generation or for this test?

Do we expect that any Dart VM installation will have platform.dill and only this
file has to be used as the source of SDK classes?

Do we prohibit presence of dart:xyz libraries in applications?

Or is it just useless because VM will ignore these libraries?

As you understand, it is a very simple change, I just would like to know the
design we are working toward.

+  new LimitedBinaryPrinter(
+          sink, (library) => library.importUri.scheme != 'dart')
+      .writeProgramFile(program);
+  await sink.close();
+}
+
+writeFile(MemoryFileSystem fs, String fileName, String contents) {

[scheglov, 2017/06/07 00:27:23]

void?

+  fs.entityForUri(Uri.parse('file:///$fileName')).writeAsStringSync(contents);
+}
+
+/// This program calls a function periodically and exits when the function
+/// returns a different value than before (which only happens after a
+/// hot-reload).
+const sourceA = r'''
+import 'dart:async';
+import 'b.dart';
+
+main(args) {
+  var last = f();
+  print(last);
+
+  // Use any arguments to indicate that we don't want to wait for a
+  // hot-reload request.
+  if (args.length > 0) return;
+
+  new Timer.periodic(new Duration(milliseconds: 100), (timer) {
+    var result = f();
+    if (last != result) {
+      print(result);
+      timer.cancel();
+    }
+  });
+}
+
+f() => "$line part2";
+''';
+
+const sourceB = r'''
+get line => "part1";
+''';
+
+RegExp observatoryPortRegExp =
+    new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)/");