From b69b2a8c9eabc0973fd434cbe09439176d4d104f Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 17 Apr 2020 10:40:02 -0700 Subject: [PATCH] Convert expression evaluation exceptions to errors (#54916) --- .../lib/src/build_runner/devfs_web.dart | 3 +- packages/flutter_tools/lib/src/compile.dart | 2 +- .../lib/src/resident_runner.dart | 4 + .../web/web_expression_compiler_test.dart | 88 +++++++++++++++++++ .../expression_evaluation_test.dart | 38 ++++++-- .../expression_evaluation_web_test.dart | 69 +++++++-------- .../test/integration.shard/test_driver.dart | 6 +- 7 files changed, 158 insertions(+), 52 deletions(-) create mode 100644 packages/flutter_tools/test/general.shard/web/web_expression_compiler_test.dart diff --git a/packages/flutter_tools/lib/src/build_runner/devfs_web.dart b/packages/flutter_tools/lib/src/build_runner/devfs_web.dart index bd4094f8599..2e3a9d9257e 100644 --- a/packages/flutter_tools/lib/src/build_runner/devfs_web.dart +++ b/packages/flutter_tools/lib/src/build_runner/devfs_web.dart @@ -77,7 +77,8 @@ class WebExpressionCompiler implements ExpressionCompiler { content, compilerOutput.errorCount > 0); } - throw Exception('Failed to compile $expression'); + return ExpressionCompilationResult( + 'InternalError: frontend server failed to compile \'$expression\'', true); } } diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index cdfa2fc37fc..0dafd0b4fb2 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -856,7 +856,7 @@ class DefaultResidentCompiler implements ResidentCompiler { } Future _compileExpressionToJs(_CompileExpressionToJsRequest request) async { - _stdoutHandler.reset(suppressCompilerMessages: true, expectSources: false); + _stdoutHandler.reset(suppressCompilerMessages: false, expectSources: false); // 'compile-expression-to-js' should be invoked after compiler has been started, // program was compiled. diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 0f240d56398..50eac66d59b 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -17,6 +17,7 @@ import 'base/file_system.dart'; import 'base/io.dart' as io; import 'base/logger.dart'; import 'base/signals.dart'; +import 'base/terminal.dart'; import 'base/utils.dart'; import 'build_info.dart'; import 'codegen.dart'; @@ -92,6 +93,9 @@ class FlutterDevice { // Override the filesystem scheme so that the frontend_server can find // the generated entrypoint code. fileSystemScheme: 'org-dartlang-app', + compilerMessageConsumer: + (String message, {bool emphasis, TerminalColor color, }) => + globals.printTrace(message), initializeFromDill: globals.fs.path.join(getBuildDirectory(), 'cache.dill'), targetModel: TargetModel.dartdevc, experimentalFlags: experimentalFlags, diff --git a/packages/flutter_tools/test/general.shard/web/web_expression_compiler_test.dart b/packages/flutter_tools/test/general.shard/web/web_expression_compiler_test.dart new file mode 100644 index 00000000000..702abec6698 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/web/web_expression_compiler_test.dart @@ -0,0 +1,88 @@ +// Copyright 2014 The Flutter 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 'package:dwds/dwds.dart'; +import 'package:flutter_tools/src/compile.dart'; +import 'package:flutter_tools/src/build_runner/devfs_web.dart'; +import 'package:matcher/matcher.dart'; +import 'package:mockito/mockito.dart'; +import 'package:flutter_tools/src/globals.dart' as globals; + +import '../../src/common.dart'; +import '../../src/testbed.dart'; + +void main() { + Testbed testbed; + + setUp(() { + testbed = Testbed(); + }); + + test('WebExpressionCompiler handles successful expression compilation', () => testbed.run(() async { + globals.fs.file('compilerOutput').writeAsStringSync('a'); + + final ResidentCompiler residentCompiler = MockResidentCompiler(); + when(residentCompiler.compileExpressionToJs( + any, any, any, any, any, any, any + )).thenAnswer((Invocation invocation) async { + return const CompilerOutput('compilerOutput', 0, []); + }); + + final ExpressionCompiler expressionCompiler = + WebExpressionCompiler(residentCompiler); + + final ExpressionCompilationResult result = + await expressionCompiler.compileExpressionToJs( + null, null, 1, 1, null, null, null, null); + + expectResult(result, false, 'a'); + })); + + test('WebExpressionCompiler handles compilation error', () => testbed.run(() async { + globals.fs.file('compilerOutput').writeAsStringSync('Error: a'); + + final ResidentCompiler residentCompiler = MockResidentCompiler(); + when(residentCompiler.compileExpressionToJs( + any, any, any, any, any, any, any + )).thenAnswer((Invocation invocation) async { + return const CompilerOutput('compilerOutput', 1, []); + }); + + final ExpressionCompiler expressionCompiler = + WebExpressionCompiler(residentCompiler); + + final ExpressionCompilationResult result = + await expressionCompiler.compileExpressionToJs( + null, null, 1, 1, null, null, null, null); + + expectResult(result, true, 'Error: a'); + })); + + test('WebExpressionCompiler handles internal error', () => testbed.run(() async { + final ResidentCompiler residentCompiler = MockResidentCompiler(); + when(residentCompiler.compileExpressionToJs( + any, any, any, any, any, any, any + )).thenAnswer((Invocation invocation) async { + return null; + }); + + final ExpressionCompiler expressionCompiler = + WebExpressionCompiler(residentCompiler); + + final ExpressionCompilationResult result = + await expressionCompiler.compileExpressionToJs( + null, null, 1, 1, null, null, null, 'a'); + + expectResult(result, true, 'InternalError: frontend server failed to compile \'a\''); + })); +} + +void expectResult(ExpressionCompilationResult result, bool isError, String value) { + expect(result, + const TypeMatcher() + .having((ExpressionCompilationResult instance) => instance.isError, 'isError', isError) + .having((ExpressionCompilationResult instance) =>instance.result, 'result', value)); +} + +class MockResidentCompiler extends Mock implements ResidentCompiler {} diff --git a/packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart b/packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart index dba82bbcd3b..9eddb706689 100644 --- a/packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart +++ b/packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:matcher/matcher.dart'; import 'package:vm_service/vm_service.dart'; @@ -146,32 +147,51 @@ void batch2() { } Future evaluateTrivialExpressions(FlutterTestDriver flutter) async { - InstanceRef res; + ObjRef res; res = await flutter.evaluateInFrame('"test"'); - expect(res.kind == InstanceKind.kString && res.valueAsString == 'test', isTrue); + expectValueOfType(res, InstanceKind.kString, 'test'); res = await flutter.evaluateInFrame('1'); - expect(res.kind == InstanceKind.kInt && res.valueAsString == 1.toString(), isTrue); + expectValueOfType(res, InstanceKind.kInt, 1.toString()); res = await flutter.evaluateInFrame('true'); - expect(res.kind == InstanceKind.kBool && res.valueAsString == true.toString(), isTrue); + expectValueOfType(res, InstanceKind.kBool, true.toString()); } Future evaluateComplexExpressions(FlutterTestDriver flutter) async { - final InstanceRef res = await flutter.evaluateInFrame('new DateTime.now().year'); - expect(res.kind == InstanceKind.kInt && res.valueAsString == DateTime.now().year.toString(), isTrue); + final ObjRef res = await flutter.evaluateInFrame('new DateTime.now().year'); + expectValueOfType(res, InstanceKind.kInt, DateTime.now().year.toString()); } Future evaluateComplexReturningExpressions(FlutterTestDriver flutter) async { final DateTime now = DateTime.now(); - final InstanceRef resp = await flutter.evaluateInFrame('new DateTime.now()'); - expect(resp.classRef.name, equals('DateTime')); + final ObjRef resp = await flutter.evaluateInFrame('new DateTime.now()'); + expectInstanceOfClass(resp, 'DateTime'); // Ensure we got a reasonable approximation. The more accurate we try to // make this, the more likely it'll fail due to differences in the time // in the remote VM and the local VM at the time the code runs. final InstanceRef res = await flutter.evaluate(resp.id, r'"$year-$month-$day"'); - expect(res.valueAsString, equals('${now.year}-${now.month}-${now.day}')); + expectValue(res, '${now.year}-${now.month}-${now.day}'); +} + +void expectInstanceOfClass(ObjRef result, String name) { + expect(result, + const TypeMatcher() + .having((InstanceRef instance) => instance.classRef.name, 'resp.classRef.name', name)); +} + +void expectValueOfType(ObjRef result, String kind, String message) { + expect(result, + const TypeMatcher() + .having((InstanceRef instance) => instance.kind, 'kind', kind) + .having((InstanceRef instance) => instance.valueAsString, 'valueAsString', message)); +} + +void expectValue(ObjRef result, String message) { + expect(result, + const TypeMatcher() + .having((InstanceRef instance) => instance.valueAsString, 'valueAsString', message)); } void main() { diff --git a/packages/flutter_tools/test/integration.shard/expression_evaluation_web_test.dart b/packages/flutter_tools/test/integration.shard/expression_evaluation_web_test.dart index f36202681f2..437de37e070 100644 --- a/packages/flutter_tools/test/integration.shard/expression_evaluation_web_test.dart +++ b/packages/flutter_tools/test/integration.shard/expression_evaluation_web_test.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:file/file.dart'; import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:matcher/matcher.dart'; import 'package:vm_service/vm_service.dart'; @@ -46,6 +47,14 @@ void batch1() { ); } + test('flutter run expression evaluation - can handle compilation errors', () async { + await initProject(); + await _flutter.run(withDebugger: true, chrome: true); + await breakInTopLevelFunction(_flutter); + await evaluateErrorExpressions(_flutter); + await cleanProject(); + }, skip: 'CI not setup for web tests'); // https://github.com/flutter/flutter/issues/53779 + test('flutter run expression evaluation - can evaluate trivial expressions in top level function', () async { await initProject(); await _flutter.run(withDebugger: true, chrome: true); @@ -77,22 +86,6 @@ void batch1() { await evaluateComplexExpressions(_flutter); await cleanProject(); }, skip: 'CI not setup for web tests'); // https://github.com/flutter/flutter/issues/53779 - - test('flutter run expression evaluation - can evaluate expressions returning complex objects in top level function', () async { - await initProject(); - await _flutter.run(withDebugger: true, chrome: true); - await breakInTopLevelFunction(_flutter); - await evaluateComplexReturningExpressions(_flutter); - await cleanProject(); - }, skip: 'Evaluate on objects is not supported for web yet'); - - test('flutter run expression evaluation - can evaluate expressions returning complex objects in build method', () async { - await initProject(); - await _flutter.run(withDebugger: true, chrome: true); - await breakInBuildMethod(_flutter); - await evaluateComplexReturningExpressions(_flutter); - await cleanProject(); - }, skip: 'Evaluate on objects is not supported for web yet'); } void batch2() { @@ -133,43 +126,43 @@ void batch2() { await evaluateComplexExpressions(_flutter); await cleanProject(); }, skip: 'CI not setup for web tests'); // https://github.com/flutter/flutter/issues/53779 +} - test('flutter test expression evaluation - can evaluate expressions returning complex objects in a test', () async { - await initProject(); - await _flutter.run(withDebugger: true, chrome: true, script: _project.testFilePath); - await breakInMethod(_flutter); - await evaluateComplexReturningExpressions(_flutter); - await cleanProject(); - }, skip: 'Evaluate on objects is not supported for web yet'); + +Future evaluateErrorExpressions(FlutterTestDriver flutter) async { + final ObjRef res = await flutter.evaluateInFrame('typo'); + expectError(res, 'CompilationError: Getter not found: \'typo\'.\ntypo\n^^^^'); } Future evaluateTrivialExpressions(FlutterTestDriver flutter) async { - InstanceRef res; + ObjRef res; res = await flutter.evaluateInFrame('"test"'); - expect(res.kind == InstanceKind.kString && res.valueAsString == 'test', isTrue); + expectInstance(res, InstanceKind.kString, 'test'); res = await flutter.evaluateInFrame('1'); - expect(res.kind == InstanceKind.kDouble && res.valueAsString == 1.toString(), isTrue); + expectInstance(res, InstanceKind.kDouble, 1.toString()); res = await flutter.evaluateInFrame('true'); - expect(res.kind == InstanceKind.kBool && res.valueAsString == true.toString(), isTrue); + expectInstance(res, InstanceKind.kBool, true.toString()); } Future evaluateComplexExpressions(FlutterTestDriver flutter) async { - final InstanceRef res = await flutter.evaluateInFrame('new DateTime.now().year'); - expect(res.kind == InstanceKind.kDouble && res.valueAsString == DateTime.now().year.toString(), isTrue); + final ObjRef res = await flutter.evaluateInFrame('new DateTime.now().year'); + expectInstance(res, InstanceKind.kDouble, DateTime.now().year.toString()); } -Future evaluateComplexReturningExpressions(FlutterTestDriver flutter) async { - final DateTime now = DateTime.now(); - final InstanceRef resp = await flutter.evaluateInFrame('new DateTime.now()'); - expect(resp.classRef.name, equals('DateTime')); - // Ensure we got a reasonable approximation. The more accurate we try to - // make this, the more likely it'll fail due to differences in the time - // in the remote VM and the local VM at the time the code runs. - final InstanceRef res = await flutter.evaluate(resp.id, r'"$year-$month-$day"'); - expect(res.valueAsString, equals('${now.year}-${now.month}-${now.day}')); +void expectInstance(ObjRef result, String kind, String message) { + expect(result, + const TypeMatcher() + .having((InstanceRef instance) => instance.kind, 'kind', kind) + .having((InstanceRef instance) => instance.valueAsString, 'valueAsString', message)); +} + +void expectError(ObjRef result, String message) { + expect(result, + const TypeMatcher() + .having((ErrorRef instance) => instance.message, 'message', message)); } void main() { diff --git a/packages/flutter_tools/test/integration.shard/test_driver.dart b/packages/flutter_tools/test/integration.shard/test_driver.dart index 1f21b07d22d..a7d170ffe44 100644 --- a/packages/flutter_tools/test/integration.shard/test_driver.dart +++ b/packages/flutter_tools/test/integration.shard/test_driver.dart @@ -291,9 +291,9 @@ abstract class FlutterTestDriver { return waitForNextPause ? waitForPause() : null; } - Future evaluateInFrame(String expression) async { - return _timeoutWithMessages( - () async => await _vmService.evaluateInFrame(await _getFlutterIsolateId(), 0, expression) as InstanceRef, + Future evaluateInFrame(String expression) async { + return _timeoutWithMessages( + () async => await _vmService.evaluateInFrame(await _getFlutterIsolateId(), 0, expression) as ObjRef, task: 'Evaluating expression ($expression)', ); }