From 364bfa0e6afe35459b046b36b6403cf58bc350db Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Tue, 22 Aug 2017 11:19:24 -0700 Subject: [PATCH] Fix frontend_server problem with _filename not being set in `compile`. (#3998) * Fix problem with _filename not being set. Add tests for that(and add mockito to mock IKG). * Initialize KernelSerializer, Fix indentation * Style nits and reworded comment --- DEPS | 4 + frontend_server/.packages | 3 +- frontend_server/lib/server.dart | 65 +++++-- frontend_server/pubspec.yaml | 3 + frontend_server/test/server_test.dart | 267 ++++++++++++++------------ 5 files changed, 202 insertions(+), 140 deletions(-) diff --git a/DEPS b/DEPS index 964185874..546e22e8a 100644 --- a/DEPS +++ b/DEPS @@ -63,6 +63,7 @@ vars = { 'dart_markdown_tag': '0.11.3', 'dart_matcher_tag': '0.12.0+2', 'dart_mime_rev': '75890811d4af5af080351ba8a2853ad4c8df98dd', + 'dart_mockito_tag': '2.0.2', 'dart_mustache4dart_tag': 'v1.1.0', 'dart_oauth2_tag': '1.0.2', 'dart_observatory_pub_packages_rev': 'a4e392521c720d244cd63e067387195d78584b35', @@ -254,6 +255,9 @@ deps = { 'src/dart/third_party/pkg/mime': Var('chromium_git') + '/external/github.com/dart-lang/mime' + '@' + Var('dart_mime_rev'), + 'src/dart/third_party/pkg/mockito': + Var('chromium_git') + '/external/github.com/dart-lang/mockito' + '@' + Var('dart_mockito_tag'), + 'src/dart/third_party/pkg/mustache4dart': Var('chromium_git') + '/external/github.com/valotas/mustache4dart' + '@' + Var('dart_mustache4dart_tag'), diff --git a/frontend_server/.packages b/frontend_server/.packages index f7bb541de..fe9b8d657 100644 --- a/frontend_server/.packages +++ b/frontend_server/.packages @@ -1,4 +1,4 @@ -# Generated by pub on 2017-08-18 15:55:43.613975. +# Generated by pub on 2017-08-22 07:05:11.485224. analyzer:../../dart/pkg/analyzer/lib/ args:../../dart/third_party/pkg/args/lib/ async:../../dart/third_party/pkg/async/lib/ @@ -22,6 +22,7 @@ logging:../../dart/third_party/pkg/logging/lib/ matcher:../../dart/third_party/pkg/matcher/lib/ meta:../../dart/pkg/meta/lib/ mime:../../dart/third_party/pkg/mime/lib/ +mockito:../../dart/third_party/pkg/mockito/lib/ package_config:../../dart/third_party/pkg_tested/package_config/lib/ package_resolver:../../dart/third_party/pkg_tested/package_resolver/lib/ path:../../dart/third_party/pkg/path/lib/ diff --git a/frontend_server/lib/server.dart b/frontend_server/lib/server.dart index 22c574d57..65a0bb758 100644 --- a/frontend_server/lib/server.dart +++ b/frontend_server/lib/server.dart @@ -11,7 +11,7 @@ import 'package:front_end/incremental_kernel_generator.dart'; import 'package:front_end/compiler_options.dart'; import 'package:front_end/kernel_generator.dart'; import 'package:kernel/ast.dart'; -import 'package:kernel/kernel.dart'; +import 'package:kernel/kernel.dart' as kernel; import 'package:kernel/target/flutter.dart'; import 'package:kernel/target/targets.dart'; import 'package:usage/uuid/uuid.dart'; @@ -59,8 +59,12 @@ enum _State { READY_FOR_INSTRUCTION, RECOMPILE_LIST } /// Actions that every compiler should implement. abstract class CompilerInterface { /// Compile given Dart program identified by `filename` with given list of - /// `options`. - Future compile(String filename, ArgResults options); + /// `options`. When `generator` parameter is omitted, new instance of + /// `IncrementalKernelGenerator` is created by this method. Main use for this + /// parameter is for mocking in tests. + Future compile(String filename, ArgResults options, { + IncrementalKernelGenerator generator, + }); /// Assuming some Dart program was previously compiled, recompile it again /// taking into account some changed(invalidated) sources. @@ -78,17 +82,32 @@ abstract class CompilerInterface { void invalidate(Uri uri); } +/// Class that for test mocking purposes encapsulates interaction with +/// global kernel methods. +class KernelSerializer { + /// Writes given kernel `program` to `path`. + Future writeProgramToBinary(Program program, String path) { + return kernel.writeProgramToBinary(program, path); + } +} + class _FrontendCompiler implements CompilerInterface { - _FrontendCompiler(this._outputStream) { + _FrontendCompiler(this._outputStream, { this.kernelSerializer }) { _outputStream ??= stdout; + kernelSerializer ??= new KernelSerializer(); } + StringSink _outputStream; + KernelSerializer kernelSerializer; + IncrementalKernelGenerator _generator; String _filename; - StringSink _outputStream; @override - Future compile(String filename, ArgResults options) async { + Future compile(String filename, ArgResults options, { + IncrementalKernelGenerator generator, + }) async { + _filename = filename; final String boundaryKey = new Uuid().generateV4(); _outputStream.writeln("result $boundaryKey"); final Uri sdkRoot = _ensureFolderPath(options['sdk-root']); @@ -101,9 +120,11 @@ class _FrontendCompiler implements CompilerInterface { }; Program program; if (options['incremental']) { - _generator = await IncrementalKernelGenerator.newInstance( - compilerOptions, Uri.base.resolve(filename) - ); + _generator = generator != null + ? generator + : await IncrementalKernelGenerator.newInstance( + compilerOptions, Uri.base.resolve(_filename) + ); final DeltaProgram deltaProgram = await _generator.computeDelta(); program = deltaProgram.newProgram; } else { @@ -112,11 +133,11 @@ class _FrontendCompiler implements CompilerInterface { compilerOptions.linkedDependencies = [ sdkRoot.resolve('platform.dill') ]; - program = await kernelForProgram(Uri.base.resolve(filename), compilerOptions); + program = await kernelForProgram(Uri.base.resolve(_filename), compilerOptions); } if (program != null) { - final String kernelBinaryFilename = filename + ".dill"; - await writeProgramToBinary(program, kernelBinaryFilename); + final String kernelBinaryFilename = _filename + ".dill"; + await kernelSerializer.writeProgramToBinary(program, kernelBinaryFilename); _outputStream.writeln("$boundaryKey $kernelBinaryFilename"); } else _outputStream.writeln(boundaryKey); @@ -125,13 +146,12 @@ class _FrontendCompiler implements CompilerInterface { @override Future recompileDelta() async { - _generator.computeDelta(); final String boundaryKey = new Uuid().generateV4(); _outputStream.writeln("result $boundaryKey"); final DeltaProgram deltaProgram = await _generator.computeDelta(); final Program program = deltaProgram.newProgram; final String kernelBinaryFilename = _filename + ".dill"; - await writeProgramToBinary(program, kernelBinaryFilename); + await kernelSerializer.writeProgramToBinary(program, kernelBinaryFilename); _outputStream.writeln("$boundaryKey"); return null; } @@ -164,17 +184,21 @@ class _FrontendCompiler implements CompilerInterface { /// processes user input. /// `compiler` is an optional parameter so it can be replaced with mocked /// version for testing. -Future starter(List args, {CompilerInterface compiler, - Stream> input, StringSink output}) async { +Future starter(List args, { + CompilerInterface compiler, + Stream> input, StringSink output, + IncrementalKernelGenerator generator, + KernelSerializer kernelSerializer +}) async { final ArgResults options = _argParser.parse(args); if (options['train']) return 0; - compiler ??= new _FrontendCompiler(output); + compiler ??= new _FrontendCompiler(output, kernelSerializer: kernelSerializer); input ??= stdin; if (options.rest.isNotEmpty) { - await compiler.compile(options.rest[0], options); + await compiler.compile(options.rest[0], options, generator: generator); return 0; } @@ -189,9 +213,8 @@ Future starter(List args, {CompilerInterface compiler, const String COMPILE_INSTRUCTION_SPACE = 'compile '; const String RECOMPILE_INSTRUCTION_SPACE = 'recompile '; if (string.startsWith(COMPILE_INSTRUCTION_SPACE)) { - final String filename = - string.substring(COMPILE_INSTRUCTION_SPACE.length); - await compiler.compile(filename, options); + final String filename = string.substring(COMPILE_INSTRUCTION_SPACE.length); + await compiler.compile(filename, options, generator: generator); } else if (string.startsWith(RECOMPILE_INSTRUCTION_SPACE)) { boundaryKey = string.substring(RECOMPILE_INSTRUCTION_SPACE.length); state = _State.RECOMPILE_LIST; diff --git a/frontend_server/pubspec.yaml b/frontend_server/pubspec.yaml index fc79ae739..03e493fc8 100644 --- a/frontend_server/pubspec.yaml +++ b/frontend_server/pubspec.yaml @@ -36,6 +36,7 @@ dev_dependencies: isolate: any matcher: any mime: any + mockito: any package_resolver: any plugin: any pool: any @@ -116,6 +117,8 @@ dependency_overrides: path: ../../dart/third_party/pkg/matcher mime: path: ../../dart/third_party/pkg/mime + mockito: + path: ../../dart/third_party/pkg/mockito package_resolver: path: ../../dart/third_party/pkg_tested/package_resolver plugin: diff --git a/frontend_server/test/server_test.dart b/frontend_server/test/server_test.dart index dc4032072..8263ee106 100644 --- a/frontend_server/test/server_test.dart +++ b/frontend_server/test/server_test.dart @@ -1,57 +1,20 @@ import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; import 'dart:isolate'; import 'package:args/src/arg_results.dart'; import 'package:frontend_server/server.dart'; +import 'package:front_end/incremental_kernel_generator.dart'; +import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; -class _MockedCompiler implements CompilerInterface { - _MockedCompiler({ - this.compileCallback, - this.recompileDeltaCallback, - this.acceptLastDeltaCallback, - this.rejectLastDeltaCallback, - this.invalidateCallback - }); +class _MockedCompiler extends Mock implements CompilerInterface {} - Future Function(String, ArgResults) compileCallback; - Future Function() recompileDeltaCallback; - void Function() acceptLastDeltaCallback; - void Function() rejectLastDeltaCallback; - void Function(Uri) invalidateCallback; - - @override - Future compile(String filename, ArgResults options) { - return compileCallback != null - ? compileCallback(filename, options) - : new Future.value(null); - } - - @override - void invalidate(Uri uri) { - if (invalidateCallback != null) - invalidateCallback(uri); - } - - @override - Future recompileDelta() { - return recompileDeltaCallback != null - ? recompileDeltaCallback() - : new Future.value(null); - } - - @override - void acceptLastDelta() { - if (acceptLastDeltaCallback != null) - acceptLastDeltaCallback(); - } - - @override - void rejectLastDelta() { - if (rejectLastDeltaCallback != null) - rejectLastDeltaCallback(); - } -} +class _MockedIncrementalKernelGenerator extends Mock + implements IncrementalKernelGenerator {} + +class _MockedKernelSerializer extends Mock implements KernelSerializer {} Future main() async { group('basic', () { @@ -60,39 +23,50 @@ Future main() async { }); }); - group('batch compile', () { + group('batch compile with mocked compiler', () { + final CompilerInterface compiler = new _MockedCompiler(); + test('compile from command line', () async { final List args = [ 'server.dart', '--sdk-root', - 'sdkroot' + 'sdkroot', ]; - final int exitcode = await starter(args, compiler: new _MockedCompiler( - compileCallback: (String filename, ArgResults options) { - expect(filename, equals('server.dart')); - expect(options['sdk-root'], equals('sdkroot')); - } - )); + final int exitcode = await starter(args, compiler: compiler); expect(exitcode, equals(0)); + final List capturedArgs = + verify( + compiler.compile( + argThat(equals('server.dart')), + captureAny, + generator: any, + ) + ).captured; + expect(capturedArgs.single['sdk-root'], equals('sdkroot')); }); }); - group('interactive compile', () { + group('interactive compile with mocked compiler', () { + final CompilerInterface compiler = new _MockedCompiler(); + final List args = [ '--sdk-root', - 'sdkroot' + 'sdkroot', ]; test('compile one file', () async { final StreamController> inputStreamController = - new StreamController>(); + new StreamController>(); final ReceivePort compileCalled = new ReceivePort(); - final int exitcode = await starter(args, compiler: new _MockedCompiler( - compileCallback: (String filename, ArgResults options) { - expect(filename, equals('server.dart')); - expect(options['sdk-root'], equals('sdkroot')); + when(compiler.compile(any, any, generator: any)).thenAnswer( + (Invocation invocation) { + expect(invocation.positionalArguments[0], equals('server.dart')); + expect(invocation.positionalArguments[1]['sdk-root'], equals('sdkroot')); compileCalled.sendPort.send(true); - }), + } + ); + + final int exitcode = await starter(args, compiler: compiler, input: inputStreamController.stream, ); expect(exitcode, equals(0)); @@ -103,15 +77,18 @@ Future main() async { test('compile few files', () async { final StreamController> streamController = - new StreamController>(); + new StreamController>(); final ReceivePort compileCalled = new ReceivePort(); int counter = 1; - final int exitcode = await starter(args, compiler: new _MockedCompiler( - compileCallback: (String filename, ArgResults options) { - expect(filename, equals('server${counter++}.dart')); - expect(options['sdk-root'], equals('sdkroot')); + when(compiler.compile(any, any, generator: any)).thenAnswer( + (Invocation invocation) { + expect(invocation.positionalArguments[0], equals('server${counter++}.dart')); + expect(invocation.positionalArguments[1]['sdk-root'], equals('sdkroot')); compileCalled.sendPort.send(true); - }), + } + ); + + final int exitcode = await starter(args, compiler: compiler, input: streamController.stream, ); expect(exitcode, equals(0)); @@ -120,41 +97,50 @@ Future main() async { await compileCalled.first; streamController.close(); }); + }); + + group('interactive incremental compile with mocked compiler', () { + final CompilerInterface compiler = new _MockedCompiler(); + + final List args = [ + '--sdk-root', + 'sdkroot', + '--incremental' + ]; test('recompile few files', () async { final StreamController> streamController = - new StreamController>(); + new StreamController>(); final ReceivePort recompileCalled = new ReceivePort(); - int counter = 0; - final List expectedInvalidatedFiles = [ - Uri.base.resolve('file1.dart'), - Uri.base.resolve('file2.dart'), - ]; - final int exitcode = await starter(args, compiler: new _MockedCompiler( - invalidateCallback: (Uri uri) { - expect(uri, equals(expectedInvalidatedFiles[counter++])); - }, - recompileDeltaCallback: () { - expect(counter, equals(2)); - recompileCalled.sendPort.send(true); - }), + when(compiler.recompileDelta()).thenAnswer((Invocation invocation) { + recompileCalled.sendPort.send(true); + }); + final int exitcode = await starter(args, compiler: compiler, input: streamController.stream, ); expect(exitcode, equals(0)); streamController.add('recompile abc\nfile1.dart\nfile2.dart\nabc\n'.codeUnits); await recompileCalled.first; + + verifyInOrder( + [ + compiler.invalidate(Uri.base.resolve('file1.dart')), + compiler.invalidate(Uri.base.resolve('file2.dart')), + compiler.recompileDelta(), + ] + ); streamController.close(); }); test('accept', () async { final StreamController> inputStreamController = - new StreamController>(); + new StreamController>(); final ReceivePort acceptCalled = new ReceivePort(); - final int exitcode = await starter(args, compiler: new _MockedCompiler( - acceptLastDeltaCallback: () { - acceptCalled.sendPort.send(true); - }), + when(compiler.acceptLastDelta()).thenAnswer((Invocation invocation) { + acceptCalled.sendPort.send(true); + }); + final int exitcode = await starter(args, compiler: compiler, input: inputStreamController.stream, ); expect(exitcode, equals(0)); @@ -167,10 +153,10 @@ Future main() async { final StreamController> inputStreamController = new StreamController>(); final ReceivePort rejectCalled = new ReceivePort(); - final int exitcode = await starter(args, compiler: new _MockedCompiler( - rejectLastDeltaCallback: () { - rejectCalled.sendPort.send(true); - }), + when(compiler.rejectLastDelta()).thenAnswer((Invocation invocation) { + rejectCalled.sendPort.send(true); + }); + final int exitcode = await starter(args, compiler: compiler, input: inputStreamController.stream, ); expect(exitcode, equals(0)); @@ -179,45 +165,90 @@ Future main() async { inputStreamController.close(); }); - test('recompile few files', () async { + test('compile then recompile', () async { final StreamController> streamController = new StreamController>(); final ReceivePort recompileCalled = new ReceivePort(); - int counter = 0; - final List expectedInvalidatedFiles = [ - Uri.base.resolve('file1.dart'), - Uri.base.resolve('file2.dart'), - Uri.base.resolve('file2.dart'), - Uri.base.resolve('file3.dart'), - ]; - bool acceptCalled = false; - final int exitcode = await starter(args, compiler: new _MockedCompiler( - invalidateCallback: (Uri uri) { - expect(uri, equals(expectedInvalidatedFiles[counter++])); - }, - recompileDeltaCallback: () { - if (counter == 2) { - expect(acceptCalled, equals(false)); - } else { - expect(counter, equals(4)); - expect(acceptCalled, equals(true)); - } - recompileCalled.sendPort.send(true); - }, - acceptLastDeltaCallback: () { - expect(acceptCalled, equals(false)); - acceptCalled = true; - }), + when(compiler.recompileDelta()).thenAnswer((Invocation invocation) { + recompileCalled.sendPort.send(true); + }); + final int exitcode = await starter(args, compiler: compiler, input: streamController.stream, ); expect(exitcode, equals(0)); - streamController.add('recompile abc\nfile1.dart\nfile2.dart\nabc\n'.codeUnits); + streamController.add('compile file1.dart\n'.codeUnits); streamController.add('accept\n'.codeUnits); streamController.add('recompile def\nfile2.dart\nfile3.dart\ndef\n'.codeUnits); await recompileCalled.first; + + verifyInOrder([ + compiler.compile('file1.dart', any, generator: any), + compiler.acceptLastDelta(), + compiler.invalidate(Uri.base.resolve('file2.dart')), + compiler.invalidate(Uri.base.resolve('file3.dart')), + compiler.recompileDelta(), + ]); streamController.close(); }); }); + + group('interactive incremental compile with mocked IKG', () { + final List args = [ + '--sdk-root', + 'sdkroot', + '--incremental', + ]; + + test('compile then accept', () async { + final StreamController> streamController = + new StreamController>(); + final StreamController> stdoutStreamController = + new StreamController>(); + final IOSink ioSink = new IOSink(stdoutStreamController.sink); + ReceivePort receivedResult = new ReceivePort(); + + String boundaryKey; + stdoutStreamController.stream + .transform(UTF8.decoder) + .transform(new LineSplitter()) + .listen((String s) { + const String RESULT_OUTPUT_SPACE = 'result '; + if (boundaryKey == null) { + if (s.startsWith(RESULT_OUTPUT_SPACE)) { + boundaryKey = s.substring(RESULT_OUTPUT_SPACE.length); + } + } else { + if (s == boundaryKey) { + boundaryKey = null; + receivedResult.sendPort.send(true); + } + } + }); + + final _MockedIncrementalKernelGenerator generator = + new _MockedIncrementalKernelGenerator(); + when(generator.computeDelta()).thenReturn(new Future.value( + new DeltaProgram(null /* program stub */) + )); + final int exitcode = await starter(args, compiler: null, + input: streamController.stream, + output: ioSink, + generator: generator, + kernelSerializer: new _MockedKernelSerializer() + ); + expect(exitcode, equals(0)); + + streamController.add('compile file1.dart\n'.codeUnits); + await receivedResult.first; + streamController.add('accept\n'.codeUnits); + receivedResult = new ReceivePort(); + streamController.add('recompile def\nfile1.dart\ndef\n'.codeUnits); + await receivedResult.first; + + streamController.close(); + }); + + }); return 0; } -- GitLab