From 3eddc2a8e7820442c652d8f0aed7adcfbcecbd23 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 30 Nov 2020 08:35:00 -0800 Subject: [PATCH 01/24] use frontend server for VM test platform --- .../test_core/lib/src/runner/vm/platform.dart | 31 +- .../lib/src/runner/vm/test_compiler.dart | 273 ++++++++++++++++++ 2 files changed, 285 insertions(+), 19 deletions(-) create mode 100644 pkgs/test_core/lib/src/runner/vm/test_compiler.dart diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index 040173824..89f5a872d 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -14,6 +14,7 @@ import 'package:stream_channel/stream_channel.dart'; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/suite_platform.dart'; // ignore: implementation_imports +import 'package:test_core/src/runner/vm/test_compiler.dart'; import 'package:vm_service/vm_service.dart' hide Isolate; import 'package:vm_service/vm_service_io.dart'; @@ -24,14 +25,14 @@ import '../../runner/platform.dart'; import '../../runner/plugin/platform_helpers.dart'; import '../../runner/runner_suite.dart'; import '../../runner/suite.dart'; -import '../../util/dart.dart' as dart; -import '../package_version.dart'; import 'environment.dart'; /// A platform that loads tests in isolates spawned within this Dart process. class VMPlatform extends PlatformPlugin { /// The test runner configuration. final _config = Configuration.current; + final _compiler = TestCompiler( + p.join(Directory.current.path, '.dart_tool', 'pkg_test_kernel.bin')); VMPlatform(); @@ -108,6 +109,11 @@ class VMPlatform extends PlatformPlugin { return await controller.suite; } + @override + Future close() async { + await _compiler.dispose(); + } + /// Spawns an isolate and passes it [message]. /// /// This isolate connects an [IsolateChannel] to [message] and sends the @@ -120,27 +126,14 @@ class VMPlatform extends PlatformPlugin { } else if (_config.pubServeUrl != null) { return _spawnPubServeIsolate(path, message, _config.pubServeUrl!); } else { - return _spawnDataIsolate(path, message, suiteMetadata); + final compiledDill = + await _compiler.compile(File(path).absolute.uri, suiteMetadata); + return await Isolate.spawnUri(p.toUri(compiledDill!), [], message, + checked: true); } } } -Future _spawnDataIsolate( - String path, SendPort message, Metadata suiteMetadata) async { - return await dart.runInIsolate(''' - ${suiteMetadata.languageVersionComment ?? await rootPackageLanguageVersionComment} - import "dart:isolate"; - - import "package:test_core/src/bootstrap/vm.dart"; - - import "${p.toUri(p.absolute(path))}" as test; - - void main(_, SendPort sendPort) { - internalBootstrapVmTest(() => test.main, sendPort); - } - ''', message); -} - Future _spawnPrecompiledIsolate( String testPath, SendPort message, String precompiledPath) async { testPath = p.absolute(p.join(precompiledPath, testPath) + '.vm_test.dart'); diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart new file mode 100644 index 000000000..81ed78bc2 --- /dev/null +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -0,0 +1,273 @@ +// Copyright (c) 2018, 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. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'dart:math' as math; +import 'dart:typed_data'; + +// ignore: import_of_legacy_library_into_null_safe +import 'package:package_config/package_config.dart'; +import 'package:path/path.dart' as path; +import 'package:test_api/backend.dart'; // ignore: deprecated_member_use + +/// A request to the [TestCompiler] for recompilation. +class _CompilationRequest { + _CompilationRequest(this.mainUri, this.result, this.metadata); + + Uri mainUri; + Metadata metadata; + Completer result; +} + +class TestCompiler { + TestCompiler(this._dillCachePath) + : _outputDillDirectory = + Directory.systemTemp.createTempSync('dart_test.') { + _outputDill = File(path.join(_outputDillDirectory.path, 'output.dill')); + _compilerController.stream.listen(_onCompilationRequest, onDone: () { + _outputDillDirectory.deleteSync(recursive: true); + }); + } + + final String _dillCachePath; + final Directory _outputDillDirectory; + final _compilerController = StreamController<_CompilationRequest>(); + final _compilationQueue = <_CompilationRequest>[]; + final _stdoutHandler = _StdoutHandler(); + + File? _outputDill; + Process? _compiler; + PackageConfig? _packageConfig; + + Future compile(Uri mainDart, Metadata metadata) { + final completer = Completer(); + if (_compilerController.isClosed) { + return Future.value(null); + } + _compilerController.add(_CompilationRequest(mainDart, completer, metadata)); + return completer.future; + } + + Future _shutdown() async { + if (_compiler != null) { + _compiler!.kill(); + _compiler = null; + } + } + + Future dispose() async { + await _compilerController.close(); + await _shutdown(); + } + + Future _languageVersionComment(Uri testUri) async { + var localPackageConfig = _packageConfig ??= await loadPackageConfig(File( + path.join( + Directory.current.path, '.dart_tool', 'package_config.json'))); + var package = localPackageConfig.packageOf(testUri) as Package?; + if (package == null) { + return ''; + } + return '// @dart=${package.languageVersion.major}.${package.languageVersion.minor}'; + } + + Future _generateEntrypoint( + Uri testUri, Metadata suiteMetadata) async { + return ''' + ${suiteMetadata.languageVersionComment ?? await _languageVersionComment(testUri)} + import "dart:isolate"; + + import "package:test_core/src/bootstrap/vm.dart"; + + import "$testUri" as test; + + void main(_, SendPort sendPort) { + internalBootstrapVmTest(() => test.main, sendPort); + } + '''; + } + + // Handle a compilation request. + Future _onCompilationRequest(_CompilationRequest request) async { + final isEmpty = _compilationQueue.isEmpty; + _compilationQueue.add(request); + if (!isEmpty) { + return; + } + while (_compilationQueue.isNotEmpty) { + final request = _compilationQueue.first; + var firstCompile = false; + _CompilerOutput? compilerOutput; + final contents = + await _generateEntrypoint(request.mainUri, request.metadata); + final tempFile = File(path.join(_outputDillDirectory.path, 'test.dart')) + ..writeAsStringSync(contents); + + if (_compiler == null) { + compilerOutput = await _createCompiler(tempFile.uri); + firstCompile = true; + } else { + compilerOutput = await _recompile( + tempFile.uri, + ); + } + final outputPath = compilerOutput?.outputFilename; + if (outputPath == null || compilerOutput!.errorCount > 0) { + request.result.complete(null); + await _shutdown(); + } else { + final outputFile = File(outputPath); + final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); + final testCache = File(_dillCachePath); + if (firstCompile || + !testCache.existsSync() || + (testCache.lengthSync() < outputFile.lengthSync())) { + // Keep the cache file up-to-date and include as many packages as possible, + // using the kernel size as an approximation. + if (!testCache.parent.existsSync()) { + testCache.parent.createSync(recursive: true); + } + await outputFile.copy(_dillCachePath); + } + request.result.complete(kernelReadyToRun.absolute.path); + _accept(); + _reset(); + } + _compilationQueue.removeAt(0); + } + } + + String _generateInputKey(math.Random random) { + final bytes = Uint8List(16); + for (var i = 0; i < 16; i++) { + bytes[i] = random.nextInt(25) + 65; + } + return String.fromCharCodes(bytes); + } + + Future<_CompilerOutput?> _recompile(Uri mainUri) { + _stdoutHandler.reset(); + final inputKey = _generateInputKey(math.Random()); + _compiler!.stdin.writeln('recompile $mainUri $inputKey'); + _compiler!.stdin.writeln('$mainUri'); + _compiler!.stdin.writeln('$inputKey'); + return _stdoutHandler.compilerOutput.future; + } + + void _accept() { + _compiler!.stdin.writeln('accept'); + } + + void _reset() { + _compiler!.stdin.writeln('reset'); + _stdoutHandler.reset(expectSources: false); + } + + Future<_CompilerOutput?> _createCompiler(Uri testUri) async { + final frontendServer = path.normalize(path.join(Platform.resolvedExecutable, + '..', 'snapshots', 'frontend_server.dart.snapshot')); + final sdkRoot = Directory( + path.relative(path.join(Platform.resolvedExecutable, '..', '..'))) + .uri; + final platformDill = 'lib/_internal/vm_platform_strong.dill'; + final process = await Process.start(Platform.resolvedExecutable, [ + '--disable-dart-dev', + frontendServer, + '--incremental', + '--sdk-root=$sdkRoot', + '--no-print-incremental-dependencies', + '--target=vm', + '--output-dill=${_outputDill!.path}', + '--initialize-from-dill=$_dillCachePath', + '--platform=$platformDill', + '--packages=${path.join(Directory.current.path, '.packages')}' + ]); + process.stdout + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen(_stdoutHandler.handler); + process.stderr + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen(print); + process.stdin.writeln('compile $testUri'); + _compiler = process; + return _stdoutHandler.compilerOutput.future; + } +} + +enum _StdoutState { CollectDiagnostic, CollectDependencies } + +class _CompilerOutput { + const _CompilerOutput(this.outputFilename, this.errorCount, this.sources); + + final String? outputFilename; + final int errorCount; + final List sources; +} + +class _StdoutHandler { + String? boundaryKey; + _StdoutState state = _StdoutState.CollectDiagnostic; + Completer<_CompilerOutput> compilerOutput = Completer<_CompilerOutput>(); + final sources = []; + + bool _suppressCompilerMessages = false; + bool _expectSources = true; + + void handler(String message) { + const kResultPrefix = 'result '; + if (boundaryKey == null && message.startsWith(kResultPrefix)) { + boundaryKey = message.substring(kResultPrefix.length); + return; + } + if (message.startsWith(boundaryKey!)) { + if (_expectSources) { + if (state == _StdoutState.CollectDiagnostic) { + state = _StdoutState.CollectDependencies; + return; + } + } + if (message.length <= boundaryKey!.length) { + compilerOutput.complete(null); + return; + } + final spaceDelimiter = message.lastIndexOf(' '); + compilerOutput.complete(_CompilerOutput( + message.substring(boundaryKey!.length + 1, spaceDelimiter), + int.parse(message.substring(spaceDelimiter + 1).trim()), + sources)); + return; + } + if (state == _StdoutState.CollectDiagnostic) { + if (!_suppressCompilerMessages) { + print(message); + } + } else { + assert(state == _StdoutState.CollectDependencies); + switch (message[0]) { + case '+': + sources.add(Uri.parse(message.substring(1))); + break; + case '-': + sources.remove(Uri.parse(message.substring(1))); + break; + default: + } + } + } + + // This is needed to get ready to process next compilation result output, + // with its own boundary key and new completer. + void reset( + {bool suppressCompilerMessages = false, bool expectSources = true}) { + boundaryKey = null; + compilerOutput = Completer<_CompilerOutput>(); + _suppressCompilerMessages = suppressCompilerMessages; + _expectSources = expectSources; + state = _StdoutState.CollectDiagnostic; + } +} From 778dbbb9f1098d48a4c7f99d4a72020f4f8d003f Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 30 Nov 2020 08:50:02 -0800 Subject: [PATCH 02/24] opt out of null safety --- .../test_core/lib/src/runner/vm/platform.dart | 5 ++- .../lib/src/runner/vm/test_compiler.dart | 45 ++++++++++--------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index 386a2617b..e8c745e53 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -130,7 +130,10 @@ class VMPlatform extends PlatformPlugin { } else { final compiledDill = await _compiler.compile(File(path).absolute.uri, suiteMetadata); - return await Isolate.spawnUri(p.toUri(compiledDill!), [], message, + if (compiledDill == null) { + throw Exception('Failed to compile $path'); + } + return await Isolate.spawnUri(p.toUri(compiledDill), [], message, checked: true); } } diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 81ed78bc2..865579b86 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -1,6 +1,8 @@ // Copyright (c) 2018, 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. +// +// @dart=2.9 import 'dart:async'; import 'dart:convert'; @@ -8,7 +10,6 @@ import 'dart:io'; import 'dart:math' as math; import 'dart:typed_data'; -// ignore: import_of_legacy_library_into_null_safe import 'package:package_config/package_config.dart'; import 'package:path/path.dart' as path; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use @@ -38,11 +39,11 @@ class TestCompiler { final _compilationQueue = <_CompilationRequest>[]; final _stdoutHandler = _StdoutHandler(); - File? _outputDill; - Process? _compiler; - PackageConfig? _packageConfig; + File _outputDill; + Process _compiler; + PackageConfig _packageConfig; - Future compile(Uri mainDart, Metadata metadata) { + Future compile(Uri mainDart, Metadata metadata) { final completer = Completer(); if (_compilerController.isClosed) { return Future.value(null); @@ -53,7 +54,7 @@ class TestCompiler { Future _shutdown() async { if (_compiler != null) { - _compiler!.kill(); + _compiler.kill(); _compiler = null; } } @@ -67,7 +68,7 @@ class TestCompiler { var localPackageConfig = _packageConfig ??= await loadPackageConfig(File( path.join( Directory.current.path, '.dart_tool', 'package_config.json'))); - var package = localPackageConfig.packageOf(testUri) as Package?; + var package = localPackageConfig.packageOf(testUri); if (package == null) { return ''; } @@ -100,7 +101,7 @@ class TestCompiler { while (_compilationQueue.isNotEmpty) { final request = _compilationQueue.first; var firstCompile = false; - _CompilerOutput? compilerOutput; + _CompilerOutput compilerOutput; final contents = await _generateEntrypoint(request.mainUri, request.metadata); final tempFile = File(path.join(_outputDillDirectory.path, 'test.dart')) @@ -115,7 +116,7 @@ class TestCompiler { ); } final outputPath = compilerOutput?.outputFilename; - if (outputPath == null || compilerOutput!.errorCount > 0) { + if (outputPath == null || compilerOutput.errorCount > 0) { request.result.complete(null); await _shutdown(); } else { @@ -148,25 +149,25 @@ class TestCompiler { return String.fromCharCodes(bytes); } - Future<_CompilerOutput?> _recompile(Uri mainUri) { + Future<_CompilerOutput> _recompile(Uri mainUri) { _stdoutHandler.reset(); final inputKey = _generateInputKey(math.Random()); - _compiler!.stdin.writeln('recompile $mainUri $inputKey'); - _compiler!.stdin.writeln('$mainUri'); - _compiler!.stdin.writeln('$inputKey'); + _compiler.stdin.writeln('recompile $mainUri $inputKey'); + _compiler.stdin.writeln('$mainUri'); + _compiler.stdin.writeln('$inputKey'); return _stdoutHandler.compilerOutput.future; } void _accept() { - _compiler!.stdin.writeln('accept'); + _compiler.stdin.writeln('accept'); } void _reset() { - _compiler!.stdin.writeln('reset'); + _compiler.stdin.writeln('reset'); _stdoutHandler.reset(expectSources: false); } - Future<_CompilerOutput?> _createCompiler(Uri testUri) async { + Future<_CompilerOutput> _createCompiler(Uri testUri) async { final frontendServer = path.normalize(path.join(Platform.resolvedExecutable, '..', 'snapshots', 'frontend_server.dart.snapshot')); final sdkRoot = Directory( @@ -180,7 +181,7 @@ class TestCompiler { '--sdk-root=$sdkRoot', '--no-print-incremental-dependencies', '--target=vm', - '--output-dill=${_outputDill!.path}', + '--output-dill=${_outputDill.path}', '--initialize-from-dill=$_dillCachePath', '--platform=$platformDill', '--packages=${path.join(Directory.current.path, '.packages')}' @@ -204,13 +205,13 @@ enum _StdoutState { CollectDiagnostic, CollectDependencies } class _CompilerOutput { const _CompilerOutput(this.outputFilename, this.errorCount, this.sources); - final String? outputFilename; + final String outputFilename; final int errorCount; final List sources; } class _StdoutHandler { - String? boundaryKey; + String boundaryKey; _StdoutState state = _StdoutState.CollectDiagnostic; Completer<_CompilerOutput> compilerOutput = Completer<_CompilerOutput>(); final sources = []; @@ -224,20 +225,20 @@ class _StdoutHandler { boundaryKey = message.substring(kResultPrefix.length); return; } - if (message.startsWith(boundaryKey!)) { + if (message.startsWith(boundaryKey)) { if (_expectSources) { if (state == _StdoutState.CollectDiagnostic) { state = _StdoutState.CollectDependencies; return; } } - if (message.length <= boundaryKey!.length) { + if (message.length <= boundaryKey.length) { compilerOutput.complete(null); return; } final spaceDelimiter = message.lastIndexOf(' '); compilerOutput.complete(_CompilerOutput( - message.substring(boundaryKey!.length + 1, spaceDelimiter), + message.substring(boundaryKey.length + 1, spaceDelimiter), int.parse(message.substring(spaceDelimiter + 1).trim()), sources)); return; From c0879bab81149cab0d3bfc357e26e683743b3752 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 3 Dec 2020 15:49:02 -0800 Subject: [PATCH 03/24] switch to frontend_server_client --- .../lib/src/runner/vm/test_compiler.dart | 188 +++--------------- pkgs/test_core/pubspec.yaml | 1 + 2 files changed, 30 insertions(+), 159 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 865579b86..51ded856e 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -5,14 +5,12 @@ // @dart=2.9 import 'dart:async'; -import 'dart:convert'; import 'dart:io'; -import 'dart:math' as math; -import 'dart:typed_data'; import 'package:package_config/package_config.dart'; -import 'package:path/path.dart' as path; +import 'package:path/path.dart' as p; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use +import 'package:frontend_server_client/frontend_server_client.dart'; /// A request to the [TestCompiler] for recompilation. class _CompilationRequest { @@ -27,21 +25,18 @@ class TestCompiler { TestCompiler(this._dillCachePath) : _outputDillDirectory = Directory.systemTemp.createTempSync('dart_test.') { - _outputDill = File(path.join(_outputDillDirectory.path, 'output.dill')); - _compilerController.stream.listen(_onCompilationRequest, onDone: () { - _outputDillDirectory.deleteSync(recursive: true); - }); + _outputDill = File(p.join(_outputDillDirectory.path, 'output.dill')); + _compilerController.stream.listen(_onCompilationRequest); } final String _dillCachePath; final Directory _outputDillDirectory; final _compilerController = StreamController<_CompilationRequest>(); final _compilationQueue = <_CompilationRequest>[]; - final _stdoutHandler = _StdoutHandler(); File _outputDill; - Process _compiler; PackageConfig _packageConfig; + FrontendServerClient _frontendServerClient; Future compile(Uri mainDart, Metadata metadata) { final completer = Completer(); @@ -52,22 +47,16 @@ class TestCompiler { return completer.future; } - Future _shutdown() async { - if (_compiler != null) { - _compiler.kill(); - _compiler = null; - } - } - Future dispose() async { await _compilerController.close(); - await _shutdown(); + _frontendServerClient?.kill(); + _frontendServerClient = null; + _outputDillDirectory.deleteSync(recursive: true); } Future _languageVersionComment(Uri testUri) async { - var localPackageConfig = _packageConfig ??= await loadPackageConfig(File( - path.join( - Directory.current.path, '.dart_tool', 'package_config.json'))); + var localPackageConfig = _packageConfig ??= await loadPackageConfig( + File(p.join(p.current, '.dart_tool', 'package_config.json'))); var package = localPackageConfig.packageOf(testUri); if (package == null) { return ''; @@ -101,24 +90,22 @@ class TestCompiler { while (_compilationQueue.isNotEmpty) { final request = _compilationQueue.first; var firstCompile = false; - _CompilerOutput compilerOutput; + CompileResult compilerOutput; final contents = await _generateEntrypoint(request.mainUri, request.metadata); - final tempFile = File(path.join(_outputDillDirectory.path, 'test.dart')) + final tempFile = File(p.join(_outputDillDirectory.path, 'test.dart')) ..writeAsStringSync(contents); - if (_compiler == null) { + if (_frontendServerClient == null) { compilerOutput = await _createCompiler(tempFile.uri); firstCompile = true; } else { - compilerOutput = await _recompile( - tempFile.uri, - ); + compilerOutput = + await _frontendServerClient.compile([tempFile.uri]); } - final outputPath = compilerOutput?.outputFilename; + final outputPath = compilerOutput?.dillOutput; if (outputPath == null || compilerOutput.errorCount > 0) { request.result.complete(null); - await _shutdown(); } else { final outputFile = File(outputPath); final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); @@ -134,141 +121,24 @@ class TestCompiler { await outputFile.copy(_dillCachePath); } request.result.complete(kernelReadyToRun.absolute.path); - _accept(); - _reset(); + _frontendServerClient.accept(); + _frontendServerClient.reset(); } _compilationQueue.removeAt(0); } } - String _generateInputKey(math.Random random) { - final bytes = Uint8List(16); - for (var i = 0; i < 16; i++) { - bytes[i] = random.nextInt(25) + 65; - } - return String.fromCharCodes(bytes); - } - - Future<_CompilerOutput> _recompile(Uri mainUri) { - _stdoutHandler.reset(); - final inputKey = _generateInputKey(math.Random()); - _compiler.stdin.writeln('recompile $mainUri $inputKey'); - _compiler.stdin.writeln('$mainUri'); - _compiler.stdin.writeln('$inputKey'); - return _stdoutHandler.compilerOutput.future; - } - - void _accept() { - _compiler.stdin.writeln('accept'); - } - - void _reset() { - _compiler.stdin.writeln('reset'); - _stdoutHandler.reset(expectSources: false); - } - - Future<_CompilerOutput> _createCompiler(Uri testUri) async { - final frontendServer = path.normalize(path.join(Platform.resolvedExecutable, - '..', 'snapshots', 'frontend_server.dart.snapshot')); - final sdkRoot = Directory( - path.relative(path.join(Platform.resolvedExecutable, '..', '..'))) - .uri; + Future _createCompiler(Uri testUri) async { final platformDill = 'lib/_internal/vm_platform_strong.dill'; - final process = await Process.start(Platform.resolvedExecutable, [ - '--disable-dart-dev', - frontendServer, - '--incremental', - '--sdk-root=$sdkRoot', - '--no-print-incremental-dependencies', - '--target=vm', - '--output-dill=${_outputDill.path}', - '--initialize-from-dill=$_dillCachePath', - '--platform=$platformDill', - '--packages=${path.join(Directory.current.path, '.packages')}' - ]); - process.stdout - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen(_stdoutHandler.handler); - process.stderr - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen(print); - process.stdin.writeln('compile $testUri'); - _compiler = process; - return _stdoutHandler.compilerOutput.future; - } -} - -enum _StdoutState { CollectDiagnostic, CollectDependencies } - -class _CompilerOutput { - const _CompilerOutput(this.outputFilename, this.errorCount, this.sources); - - final String outputFilename; - final int errorCount; - final List sources; -} - -class _StdoutHandler { - String boundaryKey; - _StdoutState state = _StdoutState.CollectDiagnostic; - Completer<_CompilerOutput> compilerOutput = Completer<_CompilerOutput>(); - final sources = []; - - bool _suppressCompilerMessages = false; - bool _expectSources = true; - - void handler(String message) { - const kResultPrefix = 'result '; - if (boundaryKey == null && message.startsWith(kResultPrefix)) { - boundaryKey = message.substring(kResultPrefix.length); - return; - } - if (message.startsWith(boundaryKey)) { - if (_expectSources) { - if (state == _StdoutState.CollectDiagnostic) { - state = _StdoutState.CollectDependencies; - return; - } - } - if (message.length <= boundaryKey.length) { - compilerOutput.complete(null); - return; - } - final spaceDelimiter = message.lastIndexOf(' '); - compilerOutput.complete(_CompilerOutput( - message.substring(boundaryKey.length + 1, spaceDelimiter), - int.parse(message.substring(spaceDelimiter + 1).trim()), - sources)); - return; - } - if (state == _StdoutState.CollectDiagnostic) { - if (!_suppressCompilerMessages) { - print(message); - } - } else { - assert(state == _StdoutState.CollectDependencies); - switch (message[0]) { - case '+': - sources.add(Uri.parse(message.substring(1))); - break; - case '-': - sources.remove(Uri.parse(message.substring(1))); - break; - default: - } - } - } - - // This is needed to get ready to process next compilation result output, - // with its own boundary key and new completer. - void reset( - {bool suppressCompilerMessages = false, bool expectSources = true}) { - boundaryKey = null; - compilerOutput = Completer<_CompilerOutput>(); - _suppressCompilerMessages = suppressCompilerMessages; - _expectSources = expectSources; - state = _StdoutState.CollectDiagnostic; + final sdkRoot = + Directory(p.relative(p.join(Platform.resolvedExecutable, '..', '..'))) + .uri; + _frontendServerClient = await FrontendServerClient.start( + testUri.toString(), + _outputDill.path, + platformDill, + sdkRoot: sdkRoot.path, + ); + return _frontendServerClient.compile(); } } diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index e87db7db0..d87980e0f 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -13,6 +13,7 @@ dependencies: boolean_selector: ">=2.1.0-nullsafety <2.1.0" collection: '>=1.15.0-nullsafety <1.15.0' coverage: ">=0.13.3 <0.15.0" + frontend_server_client: ^1.0.0 glob: ^1.0.0 io: ^0.3.0 meta: '>=1.3.0-nullsafety <1.3.0' From 6c6f95946e56d4891291f5652e62040e2d1f5f17 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 4 Dec 2020 09:21:47 -0800 Subject: [PATCH 04/24] use rootPackageLanguageVersionComment --- .../lib/src/runner/vm/test_compiler.dart | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 51ded856e..2caf8f532 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -7,11 +7,12 @@ import 'dart:async'; import 'dart:io'; -import 'package:package_config/package_config.dart'; import 'package:path/path.dart' as p; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:frontend_server_client/frontend_server_client.dart'; +import '../package_version.dart'; + /// A request to the [TestCompiler] for recompilation. class _CompilationRequest { _CompilationRequest(this.mainUri, this.result, this.metadata); @@ -35,7 +36,6 @@ class TestCompiler { final _compilationQueue = <_CompilationRequest>[]; File _outputDill; - PackageConfig _packageConfig; FrontendServerClient _frontendServerClient; Future compile(Uri mainDart, Metadata metadata) { @@ -54,20 +54,10 @@ class TestCompiler { _outputDillDirectory.deleteSync(recursive: true); } - Future _languageVersionComment(Uri testUri) async { - var localPackageConfig = _packageConfig ??= await loadPackageConfig( - File(p.join(p.current, '.dart_tool', 'package_config.json'))); - var package = localPackageConfig.packageOf(testUri); - if (package == null) { - return ''; - } - return '// @dart=${package.languageVersion.major}.${package.languageVersion.minor}'; - } - Future _generateEntrypoint( Uri testUri, Metadata suiteMetadata) async { return ''' - ${suiteMetadata.languageVersionComment ?? await _languageVersionComment(testUri)} + ${suiteMetadata.languageVersionComment ?? await rootPackageLanguageVersionComment} import "dart:isolate"; import "package:test_core/src/bootstrap/vm.dart"; From 8bac872c0f6f52dbad5e0b6b3e853a6571615030 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sun, 6 Dec 2020 18:14:14 -0800 Subject: [PATCH 05/24] fix year --- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 2caf8f532..eee715c6b 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2020, 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. // @@ -94,7 +94,7 @@ class TestCompiler { await _frontendServerClient.compile([tempFile.uri]); } final outputPath = compilerOutput?.dillOutput; - if (outputPath == null || compilerOutput.errorCount > 0) { + if (outputPath == null) { request.result.complete(null); } else { final outputFile = File(outputPath); From f932ad65bd8dd7b4842eafcf363f35fe67ba3e0e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 26 Feb 2021 09:28:32 -0800 Subject: [PATCH 06/24] tweak tests and add comment --- pkgs/test/test/runner/test_on_test.dart | 17 +++++++++++++++++ .../lib/src/runner/vm/test_compiler.dart | 6 ++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pkgs/test/test/runner/test_on_test.dart b/pkgs/test/test/runner/test_on_test.dart index 23258fa1f..21f48170e 100644 --- a/pkgs/test/test/runner/test_on_test.dart +++ b/pkgs/test/test/runner/test_on_test.dart @@ -7,7 +7,10 @@ @TestOn('vm') import 'dart:async'; +import 'dart:convert'; +import 'dart:isolate'; +import 'package:package_config/package_config.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_core/src/util/io.dart'; @@ -16,6 +19,20 @@ import 'package:test/test.dart'; import '../io.dart'; void main() { + PackageConfig currentPackageConfig; + + setUpAll(() async { + currentPackageConfig = + await loadPackageConfigUri(await Isolate.packageConfig); + }); + + setUp(() async { + await d + .file('package_config.json', + jsonEncode(PackageConfig.toJson(currentPackageConfig))) + .create(); + }); + group('for suite', () { test('runs a test suite on a matching platform', () async { await _writeTestFile('vm_test.dart', suiteTestOn: 'vm'); diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index eee715c6b..cfae99eeb 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -100,11 +100,13 @@ class TestCompiler { final outputFile = File(outputPath); final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); final testCache = File(_dillCachePath); + // Keep the cache file up-to-date and use the size of the kernel file + // as an approximation for how many packages are included. Larger files + // are prefered, since re-using more packages will reduce the number of + // files the frontend server needs to load and parse. if (firstCompile || !testCache.existsSync() || (testCache.lengthSync() < outputFile.lengthSync())) { - // Keep the cache file up-to-date and include as many packages as possible, - // using the kernel size as an approximation. if (!testCache.parent.existsSync()) { testCache.parent.createSync(recursive: true); } From c98910bcf9d2083a0d0cba2857173b3c03107e6a Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 09:58:32 -0700 Subject: [PATCH 07/24] require latest frontend_server_client --- pkgs/test_core/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index 62012a0c6..769a81314 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -7,7 +7,7 @@ environment: sdk: ">=2.12.0-0 <3.0.0" dependencies: - frontend_server_client: ^1.0.0 + frontend_server_client: ^2.0.0 analyzer: '>=0.39.5 <2.0.0' async: ^2.5.0 args: ">=1.4.0 <3.0.0" From 635326534351a8240f44204eeef812667f00659e Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 10:26:42 -0700 Subject: [PATCH 08/24] fix up some null safety issues --- pkgs/test/test/runner/test_on_test.dart | 4 ++-- .../lib/src/runner/vm/test_compiler.dart | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkgs/test/test/runner/test_on_test.dart b/pkgs/test/test/runner/test_on_test.dart index f845833ba..67109f290 100644 --- a/pkgs/test/test/runner/test_on_test.dart +++ b/pkgs/test/test/runner/test_on_test.dart @@ -17,11 +17,11 @@ import 'package:test/test.dart'; import '../io.dart'; void main() { - PackageConfig currentPackageConfig; + late PackageConfig currentPackageConfig; setUpAll(() async { currentPackageConfig = - await loadPackageConfigUri(await Isolate.packageConfig); + await loadPackageConfigUri((await Isolate.packageConfig)!); }); setUp(() async { diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index cfae99eeb..7867d35ed 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -1,8 +1,6 @@ // Copyright (c) 2020, 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. -// -// @dart=2.9 import 'dart:async'; import 'dart:io'; @@ -35,10 +33,10 @@ class TestCompiler { final _compilerController = StreamController<_CompilationRequest>(); final _compilationQueue = <_CompilationRequest>[]; - File _outputDill; - FrontendServerClient _frontendServerClient; + late final File _outputDill; + FrontendServerClient? _frontendServerClient; - Future compile(Uri mainDart, Metadata metadata) { + Future compile(Uri mainDart, Metadata metadata) { final completer = Completer(); if (_compilerController.isClosed) { return Future.value(null); @@ -80,7 +78,7 @@ class TestCompiler { while (_compilationQueue.isNotEmpty) { final request = _compilationQueue.first; var firstCompile = false; - CompileResult compilerOutput; + CompileResult? compilerOutput; final contents = await _generateEntrypoint(request.mainUri, request.metadata); final tempFile = File(p.join(_outputDillDirectory.path, 'test.dart')) @@ -91,8 +89,10 @@ class TestCompiler { firstCompile = true; } else { compilerOutput = - await _frontendServerClient.compile([tempFile.uri]); + await _frontendServerClient!.compile([tempFile.uri]); } + // The client is guaranteed initialized at this point. + final client = _frontendServerClient!; final outputPath = compilerOutput?.dillOutput; if (outputPath == null) { request.result.complete(null); @@ -113,24 +113,24 @@ class TestCompiler { await outputFile.copy(_dillCachePath); } request.result.complete(kernelReadyToRun.absolute.path); - _frontendServerClient.accept(); - _frontendServerClient.reset(); + client.accept(); + client.reset(); } _compilationQueue.removeAt(0); } } - Future _createCompiler(Uri testUri) async { + Future _createCompiler(Uri testUri) async { final platformDill = 'lib/_internal/vm_platform_strong.dill'; final sdkRoot = Directory(p.relative(p.join(Platform.resolvedExecutable, '..', '..'))) .uri; - _frontendServerClient = await FrontendServerClient.start( + var client = _frontendServerClient = await FrontendServerClient.start( testUri.toString(), _outputDill.path, platformDill, sdkRoot: sdkRoot.path, ); - return _frontendServerClient.compile(); + return client.compile(); } } From 3fb3e7a92907649f7dd9f527f180f3bb5b876488 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 12:55:09 -0700 Subject: [PATCH 09/24] produce a LoadException if the app fails to compile --- .../test_core/lib/src/runner/vm/platform.dart | 9 ++++---- .../lib/src/runner/vm/test_compiler.dart | 21 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index e29ca8c32..cb42fdb2e 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -31,8 +31,8 @@ import 'environment.dart'; class VMPlatform extends PlatformPlugin { /// The test runner configuration. final _config = Configuration.current; - final _compiler = TestCompiler( - p.join(Directory.current.path, '.dart_tool', 'pkg_test_kernel.bin')); + final _compiler = + TestCompiler(p.join(p.current, '.dart_tool', 'pkg_test_kernel.bin')); VMPlatform(); @@ -126,10 +126,11 @@ class VMPlatform extends PlatformPlugin { } else if (_config.pubServeUrl != null) { return _spawnPubServeIsolate(path, message, _config.pubServeUrl!); } else { - final compiledDill = + final response = await _compiler.compile(File(path).absolute.uri, suiteMetadata); + var compiledDill = response.kernelOutputUri?.toFilePath(); if (compiledDill == null) { - throw Exception('Failed to compile $path'); + throw LoadException(path, response.compilerOutput ?? 'unknown error'); } return await Isolate.spawnUri(p.toUri(compiledDill), [], message, checked: true); diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 7867d35ed..f003bdf28 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -9,6 +9,7 @@ import 'package:path/path.dart' as p; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:frontend_server_client/frontend_server_client.dart'; +import '../load_exception.dart'; import '../package_version.dart'; /// A request to the [TestCompiler] for recompilation. @@ -17,7 +18,14 @@ class _CompilationRequest { Uri mainUri; Metadata metadata; - Completer result; + Completer result; +} + +class CompilationResponse { + final String? compilerOutput; + final Uri? kernelOutputUri; + + CompilationResponse({this.compilerOutput, this.kernelOutputUri}); } class TestCompiler { @@ -36,8 +44,8 @@ class TestCompiler { late final File _outputDill; FrontendServerClient? _frontendServerClient; - Future compile(Uri mainDart, Metadata metadata) { - final completer = Completer(); + Future compile(Uri mainDart, Metadata metadata) { + final completer = Completer(); if (_compilerController.isClosed) { return Future.value(null); } @@ -95,7 +103,8 @@ class TestCompiler { final client = _frontendServerClient!; final outputPath = compilerOutput?.dillOutput; if (outputPath == null) { - request.result.complete(null); + request.result.complete(CompilationResponse( + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'))); } else { final outputFile = File(outputPath); final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); @@ -112,7 +121,9 @@ class TestCompiler { } await outputFile.copy(_dillCachePath); } - request.result.complete(kernelReadyToRun.absolute.path); + request.result.complete(CompilationResponse( + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + kernelOutputUri: kernelReadyToRun.absolute.uri)); client.accept(); client.reset(); } From 0a5831c7014daa80c2c36337f936398014ef8e4a Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 13:03:46 -0700 Subject: [PATCH 10/24] remove unused import --- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index f003bdf28..05ab26344 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -9,7 +9,6 @@ import 'package:path/path.dart' as p; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:frontend_server_client/frontend_server_client.dart'; -import '../load_exception.dart'; import '../package_version.dart'; /// A request to the [TestCompiler] for recompilation. From 4998cc9c64769598481d95865b855736e09526fd Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 13:32:55 -0700 Subject: [PATCH 11/24] add errorCount and check it --- pkgs/test_core/lib/src/runner/vm/platform.dart | 2 +- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index cb42fdb2e..f05d35c1b 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -129,7 +129,7 @@ class VMPlatform extends PlatformPlugin { final response = await _compiler.compile(File(path).absolute.uri, suiteMetadata); var compiledDill = response.kernelOutputUri?.toFilePath(); - if (compiledDill == null) { + if (compiledDill == null || response.errorCount > 0) { throw LoadException(path, response.compilerOutput ?? 'unknown error'); } return await Isolate.spawnUri(p.toUri(compiledDill), [], message, diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 05ab26344..f2dc5a5e1 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -22,9 +22,11 @@ class _CompilationRequest { class CompilationResponse { final String? compilerOutput; + final int errorCount; final Uri? kernelOutputUri; - CompilationResponse({this.compilerOutput, this.kernelOutputUri}); + CompilationResponse( + {this.compilerOutput, this.errorCount = 0, this.kernelOutputUri}); } class TestCompiler { @@ -103,7 +105,8 @@ class TestCompiler { final outputPath = compilerOutput?.dillOutput; if (outputPath == null) { request.result.complete(CompilationResponse( - compilerOutput: compilerOutput?.compilerOutputLines.join('\n'))); + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + errorCount: compilerOutput?.errorCount ?? 0)); } else { final outputFile = File(outputPath); final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); @@ -122,6 +125,7 @@ class TestCompiler { } request.result.complete(CompilationResponse( compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + errorCount: compilerOutput?.errorCount ?? 0, kernelOutputUri: kernelReadyToRun.absolute.uri)); client.accept(); client.reset(); From b323cab628fc6714a00b9e16f56d84b080d1fe3f Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 18 Mar 2021 13:37:16 -0700 Subject: [PATCH 12/24] pass the current package config to the frotnend server --- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index f2dc5a5e1..8670214c7 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -10,6 +10,7 @@ import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:frontend_server_client/frontend_server_client.dart'; import '../package_version.dart'; +import '../../util/package_config.dart'; /// A request to the [TestCompiler] for recompilation. class _CompilationRequest { @@ -144,6 +145,7 @@ class TestCompiler { _outputDill.path, platformDill, sdkRoot: sdkRoot.path, + packagesJson: (await packageConfigUri).toFilePath(), ); return client.compile(); } From bf1aa14512e826b3c7fb7d91ebc3f9553e98b7b1 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Fri, 19 Mar 2021 09:04:18 -0700 Subject: [PATCH 13/24] fix tests and expectations to match the new output --- pkgs/test/test/runner/runner_test.dart | 9 +++------ pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 4 +++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 6100fe681..2d6a97188 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -167,8 +167,7 @@ $_usage'''); test.stdout, containsInOrder([ 'Failed to load "test.dart":', - 'Unable to spawn isolate: test.dart:1:9: Error: ' - "Expected ';' after this.", + "test.dart:1:9: Error: Expected ';' after this.", 'invalid Dart file' ])); @@ -186,8 +185,7 @@ $_usage'''); containsInOrder([ '-1: loading test.dart [E]', 'Failed to load "test.dart":', - 'Unable to spawn isolate: test.dart:1:14: ' - "Error: Expected ';' after this" + "test.dart:1:14: Error: Expected ';' after this" ])); await test.shouldExit(1); @@ -204,8 +202,7 @@ $_usage'''); containsInOrder([ '-1: loading test.dart [E]', 'Failed to load "test.dart":', - 'Unable to spawn isolate: test.dart:1:8: Error: ' - "Expected a declaration, but got ')'", + "test.dart:1:8: Error: Expected a declaration, but got ')'", '@TestOn)', ])); diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 8670214c7..849e22c91 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -59,7 +59,9 @@ class TestCompiler { await _compilerController.close(); _frontendServerClient?.kill(); _frontendServerClient = null; - _outputDillDirectory.deleteSync(recursive: true); + if (await _outputDillDirectory.exists()) { + await _outputDillDirectory.delete(recursive: true); + } } Future _generateEntrypoint( From ad98660602646630e70432de5831b8456eb7ec55 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Fri, 19 Mar 2021 09:14:20 -0700 Subject: [PATCH 14/24] delete output dill dir synchronously --- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 849e22c91..c64431a4d 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -59,8 +59,8 @@ class TestCompiler { await _compilerController.close(); _frontendServerClient?.kill(); _frontendServerClient = null; - if (await _outputDillDirectory.exists()) { - await _outputDillDirectory.delete(recursive: true); + if (_outputDillDirectory.existsSync()) { + _outputDillDirectory.deleteSync(recursive: true); } } From e6ea03eb596084d55bd71b6e986d778d8d28892b Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Fri, 19 Mar 2021 09:37:19 -0700 Subject: [PATCH 15/24] dont print incremental dependencies --- pkgs/test_core/lib/src/runner/vm/test_compiler.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index c64431a4d..ec5ead421 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -148,6 +148,7 @@ class TestCompiler { platformDill, sdkRoot: sdkRoot.path, packagesJson: (await packageConfigUri).toFilePath(), + printIncrementalDependencies: false, ); return client.compile(); } From 164bf1be4298933c28d66e372daf38413d6d0dfa Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 10:24:17 -0700 Subject: [PATCH 16/24] add --use-data-isolate-strategy flag for use primarily in bazel --- pkgs/test/CHANGELOG.md | 7 +++- pkgs/test/pubspec.yaml | 2 +- pkgs/test_core/CHANGELOG.md | 5 +++ .../lib/src/runner/configuration.dart | 17 ++++++++ .../lib/src/runner/configuration/args.dart | 6 +++ .../test_core/lib/src/runner/vm/platform.dart | 41 +++++++++++++++---- 6 files changed, 68 insertions(+), 10 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 727e12872..ceca94333 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,4 +1,9 @@ -## 1.16.9-dev +## 1.17.0-dev + +* Change the default way VM tests are launched and ran to greatly speed up + loading performance. + * You can force the old strategy with `--use-data-isolate-strategy` flag if + you run into issues, but please also file a bug. ## 1.16.8 diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 5edce4897..5cff44ccc 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 1.16.9-dev +version: 1.17.0-dev description: A full featured library for writing and running Dart tests. repository: https://github.com/dart-lang/test/blob/master/pkgs/test diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 9c0eace9f..c72091629 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -1,5 +1,10 @@ ## 0.3.20-dev +* Change the default way VM tests are launched and ran to greatly speed up + loading performance. + * You can force the old strategy with `--use-data-isolate-strategy` flag if + you run into issues, but please also file a bug. + ## 0.3.19 * Disable stack trace chaining by default. diff --git a/pkgs/test_core/lib/src/runner/configuration.dart b/pkgs/test_core/lib/src/runner/configuration.dart index 0628a1873..9be23afb1 100644 --- a/pkgs/test_core/lib/src/runner/configuration.dart +++ b/pkgs/test_core/lib/src/runner/configuration.dart @@ -174,6 +174,14 @@ class Configuration { }); Set? _knownPresets; + /// Whether to use the original `data:` uri isolate spawning strategy for vm + /// tests. + /// + /// This can make more sense than the default strategy in systems such as + /// `bazel` where only a single test suite is ran at a time. + bool get useDataIsolateStrategy => _useDataIsolateStrategy ?? false; + final bool? _useDataIsolateStrategy; + /// Built-in runtimes whose settings are overridden by the user. final Map overrideRuntimes; @@ -243,6 +251,7 @@ class Configuration { Map? overrideRuntimes, Map? defineRuntimes, bool? noRetry, + bool? useDataIsolateStrategy, // Suite-level configuration bool? jsTrace, @@ -292,6 +301,7 @@ class Configuration { overrideRuntimes: overrideRuntimes, defineRuntimes: defineRuntimes, noRetry: noRetry, + useDataIsolateStrategy: useDataIsolateStrategy, suiteDefaults: SuiteConfiguration( jsTrace: jsTrace, runSkipped: runSkipped, @@ -354,6 +364,7 @@ class Configuration { Map? overrideRuntimes, Map? defineRuntimes, bool? noRetry, + bool? useDataIsolateStrategy, SuiteConfiguration? suiteDefaults}) : _help = help, customHtmlTemplatePath = customHtmlTemplatePath, @@ -379,6 +390,7 @@ class Configuration { overrideRuntimes = _map(overrideRuntimes), defineRuntimes = _map(defineRuntimes), _noRetry = noRetry, + _useDataIsolateStrategy = useDataIsolateStrategy, suiteDefaults = pauseAfterLoad == true ? suiteDefaults?.change(timeout: Timeout.none) ?? SuiteConfiguration(timeout: Timeout.none) @@ -513,6 +525,8 @@ class Configuration { defineRuntimes: mergeUnmodifiableMaps(defineRuntimes, other.defineRuntimes), noRetry: other._noRetry ?? _noRetry, + useDataIsolateStrategy: + other._useDataIsolateStrategy ?? _useDataIsolateStrategy, suiteDefaults: suiteDefaults.merge(other.suiteDefaults)); result = result._resolvePresets(); @@ -549,6 +563,7 @@ class Configuration { Map? overrideRuntimes, Map? defineRuntimes, bool? noRetry, + bool? useDataIsolateStrategy, // Suite-level configuration bool? jsTrace, @@ -595,6 +610,8 @@ class Configuration { overrideRuntimes: overrideRuntimes ?? this.overrideRuntimes, defineRuntimes: defineRuntimes ?? this.defineRuntimes, noRetry: noRetry ?? _noRetry, + useDataIsolateStrategy: + useDataIsolateStrategy ?? _useDataIsolateStrategy, suiteDefaults: suiteDefaults.change( jsTrace: jsTrace, runSkipped: runSkipped, diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index afb064be4..8b41c003d 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -107,6 +107,11 @@ final ArgParser _parser = (() { help: "Don't rerun tests that have retry set.", defaultsTo: false, negatable: false); + parser.addFlag('use-data-isolate-strategy', + help: 'Use `data:` uri isolates when spawning VM tests instead of the ' + 'default strategy. This may be faster when you only ever running a ' + 'single test suite at a time.', + defaultsTo: false); parser.addOption('test-randomize-ordering-seed', help: 'Use the specified seed to randomize the execution order of test' ' cases.\n' @@ -268,6 +273,7 @@ class _Parser { includeTags: includeTags, excludeTags: excludeTags, noRetry: _ifParsed('no-retry'), + useDataIsolateStrategy: _ifParsed('use-data-isolate-strategy'), testRandomizeOrderingSeed: testRandomizeOrderingSeed); } diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index f05d35c1b..23c608571 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -25,6 +25,9 @@ import '../../runner/platform.dart'; import '../../runner/plugin/platform_helpers.dart'; import '../../runner/runner_suite.dart'; import '../../runner/suite.dart'; +import '../../util/dart.dart' as dart; +import '../../util/package_config.dart'; +import '../package_version.dart'; import 'environment.dart'; /// A platform that loads tests in isolates spawned within this Dart process. @@ -125,17 +128,39 @@ class VMPlatform extends PlatformPlugin { return _spawnPrecompiledIsolate(path, message, precompiledPath); } else if (_config.pubServeUrl != null) { return _spawnPubServeIsolate(path, message, _config.pubServeUrl!); + } else if (_config.useDataIsolateStrategy) { + return _spawnDataIsolate(path, message, suiteMetadata); } else { - final response = - await _compiler.compile(File(path).absolute.uri, suiteMetadata); - var compiledDill = response.kernelOutputUri?.toFilePath(); - if (compiledDill == null || response.errorCount > 0) { - throw LoadException(path, response.compilerOutput ?? 'unknown error'); - } - return await Isolate.spawnUri(p.toUri(compiledDill), [], message, - checked: true); + return _spawnKernelIsolate(path, message, suiteMetadata); } } + + /// Compiles [path] to kernel using [_compiler] and spawns that in an + /// isolate. + Future _spawnKernelIsolate( + String path, SendPort message, Metadata suiteMetadata) async { + final response = + await _compiler.compile(File(path).absolute.uri, suiteMetadata); + var compiledDill = response.kernelOutputUri?.toFilePath(); + if (compiledDill == null || response.errorCount > 0) { + throw LoadException(path, response.compilerOutput ?? 'unknown error'); + } + return await Isolate.spawnUri(p.toUri(compiledDill), [], message, + packageConfig: await packageConfigUri, checked: true); + } +} + +Future _spawnDataIsolate( + String path, SendPort message, Metadata suiteMetadata) async { + return await dart.runInIsolate(''' + ${suiteMetadata.languageVersionComment ?? await rootPackageLanguageVersionComment} + import "dart:isolate"; + import "package:test_core/src/bootstrap/vm.dart"; + import "${p.toUri(p.absolute(path))}" as test; + void main(_, SendPort sendPort) { + internalBootstrapVmTest(() => test.main, sendPort); + } + ''', message); } Future _spawnPrecompiledIsolate( From 21a1b2516bd2f1b79564f3f97caf12da3a1a4504 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 10:59:56 -0700 Subject: [PATCH 17/24] add some basic data isolate strategy tests --- .../runner/data_isolate_strategy_test.dart | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pkgs/test/test/runner/data_isolate_strategy_test.dart diff --git a/pkgs/test/test/runner/data_isolate_strategy_test.dart b/pkgs/test/test/runner/data_isolate_strategy_test.dart new file mode 100644 index 000000000..f15bc61bf --- /dev/null +++ b/pkgs/test/test/runner/data_isolate_strategy_test.dart @@ -0,0 +1,68 @@ +// Copyright (c) 2021, 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. + +@TestOn('vm') + +import 'dart:async'; +import 'dart:convert'; +import 'dart:isolate'; + +import 'package:package_config/package_config.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +import 'package:test_core/src/util/io.dart'; +import 'package:test/test.dart'; + +import '../io.dart'; + +void main() { + late PackageConfig currentPackageConfig; + + setUpAll(() async { + currentPackageConfig = + await loadPackageConfigUri((await Isolate.packageConfig)!); + }); + + setUp(() async { + await d + .file('package_config.json', + jsonEncode(PackageConfig.toJson(currentPackageConfig))) + .create(); + }); + + group('The data isolate strategy', () { + test('can be enabled', () async { + /// We confirm it is enabled by checking the error output for an invalid + /// test, it looks a bit different. + await d.file('test.dart', 'invalid Dart file').create(); + var test = await runTest(['--use-data-isolate-strategy', 'test.dart']); + + expect( + test.stdout, + containsInOrder([ + 'Failed to load "test.dart":', + "Unable to spawn isolate: test.dart:1:9: Error: Expected ';' after this.", + 'invalid Dart file' + ])); + + await test.shouldExit(1); + }); + + test('can run tests', () async { + await d.file('test.dart', ''' +import 'package:test/test.dart'; + +void main() { + test('true is true', () { + expect(true, isTrue); + }); +} + ''').create(); + var test = await runTest(['--use-data-isolate-strategy', 'test.dart']); + + expect(test.stdout, emitsThrough(contains('+1: All tests passed!'))); + await test.shouldExit(0); + }); + }); +} From 2a78037a5599613aaf61f19be064aceb3b907094 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 11:06:53 -0700 Subject: [PATCH 18/24] fix up help output and expectations --- pkgs/test/test/runner/runner_test.dart | 3 +++ pkgs/test_core/lib/src/runner/configuration/args.dart | 9 +++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 2d6a97188..3cf37f733 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -93,6 +93,9 @@ Running Tests: to provide improved test performance but at the cost of debuggability. --no-retry Don't rerun tests that have retry set. + --use-data-isolate-strategy Use `data:` uri isolates when spawning VM tests instead of the + default strategy. This may be faster when you only ever running + a single test suite at a time. --test-randomize-ordering-seed Use the specified seed to randomize the execution order of test cases. Must be a 32bit unsigned integer or "random". If "random", pick a random seed to use. diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index 8b41c003d..04eff6d9f 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -108,10 +108,11 @@ final ArgParser _parser = (() { defaultsTo: false, negatable: false); parser.addFlag('use-data-isolate-strategy', - help: 'Use `data:` uri isolates when spawning VM tests instead of the ' - 'default strategy. This may be faster when you only ever running a ' - 'single test suite at a time.', - defaultsTo: false); + help: 'Use `data:` uri isolates when spawning VM tests instead of the\n' + 'default strategy. This may be faster when you only ever running\n' + 'a single test suite at a time.', + defaultsTo: false, + negatable: false); parser.addOption('test-randomize-ordering-seed', help: 'Use the specified seed to randomize the execution order of test' ' cases.\n' From e85f2282708398a56cf84b5395fcc89ccfed1108 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 11:08:19 -0700 Subject: [PATCH 19/24] remove unused imports --- pkgs/test/test/runner/data_isolate_strategy_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkgs/test/test/runner/data_isolate_strategy_test.dart b/pkgs/test/test/runner/data_isolate_strategy_test.dart index f15bc61bf..1678b91c4 100644 --- a/pkgs/test/test/runner/data_isolate_strategy_test.dart +++ b/pkgs/test/test/runner/data_isolate_strategy_test.dart @@ -4,14 +4,12 @@ @TestOn('vm') -import 'dart:async'; import 'dart:convert'; import 'dart:isolate'; import 'package:package_config/package_config.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; -import 'package:test_core/src/util/io.dart'; import 'package:test/test.dart'; import '../io.dart'; From 8d96df06925a7a3d0145aa9fdd56028bd223eb87 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 11:38:23 -0700 Subject: [PATCH 20/24] code review updates --- pkgs/test/test/runner/runner_test.dart | 4 ++-- pkgs/test_core/lib/src/runner/configuration/args.dart | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 3cf37f733..e2e2cd14c 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -94,8 +94,8 @@ Running Tests: debuggability. --no-retry Don't rerun tests that have retry set. --use-data-isolate-strategy Use `data:` uri isolates when spawning VM tests instead of the - default strategy. This may be faster when you only ever running - a single test suite at a time. + default strategy. This may be faster when you only ever run a + single test suite at a time. --test-randomize-ordering-seed Use the specified seed to randomize the execution order of test cases. Must be a 32bit unsigned integer or "random". If "random", pick a random seed to use. diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index 04eff6d9f..1f8b0fca6 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -109,8 +109,8 @@ final ArgParser _parser = (() { negatable: false); parser.addFlag('use-data-isolate-strategy', help: 'Use `data:` uri isolates when spawning VM tests instead of the\n' - 'default strategy. This may be faster when you only ever running\n' - 'a single test suite at a time.', + 'default strategy. This may be faster when you only ever run a\n' + 'single test suite at a time.', defaultsTo: false, negatable: false); parser.addOption('test-randomize-ordering-seed', From dae0aa29e44ea2646bc634667144a405c8478f7c Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 12:52:10 -0700 Subject: [PATCH 21/24] update to use package:pool, move fields above constructor --- .../lib/src/runner/vm/test_compiler.dart | 141 ++++++++---------- 1 file changed, 64 insertions(+), 77 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index ec5ead421..62e999f20 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -6,21 +6,13 @@ import 'dart:async'; import 'dart:io'; import 'package:path/path.dart' as p; +import 'package:pool/pool.dart'; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use import 'package:frontend_server_client/frontend_server_client.dart'; import '../package_version.dart'; import '../../util/package_config.dart'; -/// A request to the [TestCompiler] for recompilation. -class _CompilationRequest { - _CompilationRequest(this.mainUri, this.result, this.metadata); - - Uri mainUri; - Metadata metadata; - Completer result; -} - class CompilationResponse { final String? compilerOutput; final int errorCount; @@ -31,32 +23,37 @@ class CompilationResponse { } class TestCompiler { - TestCompiler(this._dillCachePath) - : _outputDillDirectory = - Directory.systemTemp.createTempSync('dart_test.') { - _outputDill = File(p.join(_outputDillDirectory.path, 'output.dill')); - _compilerController.stream.listen(_onCompilationRequest); - } - + final _compilePool = Pool(1); final String _dillCachePath; final Directory _outputDillDirectory; - final _compilerController = StreamController<_CompilationRequest>(); - final _compilationQueue = <_CompilationRequest>[]; - late final File _outputDill; + late final _outputDill = + File(p.join(_outputDillDirectory.path, 'output.dill')); FrontendServerClient? _frontendServerClient; + TestCompiler(this._dillCachePath) + : _outputDillDirectory = + Directory.systemTemp.createTempSync('dart_test.'); + + /// Enqueues a request to compile [mainDart] and returns the result. + /// + /// This request may need to wait for ongoing compilations. + /// + /// If [dispose] has already been called, then this immediately returns a + /// failed response indicating the compiler was shut down. + /// + /// The entrypoint [mainDart] is wrapped in a script which bootstraps it with + /// a call to `internalBootstrapVmTest`. Future compile(Uri mainDart, Metadata metadata) { - final completer = Completer(); - if (_compilerController.isClosed) { - return Future.value(null); + if (_compilePool.isClosed) { + return Future.value(CompilationResponse( + errorCount: 1, compilerOutput: 'Compiler was already shut down.')); } - _compilerController.add(_CompilationRequest(mainDart, completer, metadata)); - return completer.future; + return _compilePool.withResource(() => _compile(mainDart, metadata)); } Future dispose() async { - await _compilerController.close(); + await _compilePool.close(); _frontendServerClient?.kill(); _frontendServerClient = null; if (_outputDillDirectory.existsSync()) { @@ -80,61 +77,51 @@ class TestCompiler { '''; } - // Handle a compilation request. - Future _onCompilationRequest(_CompilationRequest request) async { - final isEmpty = _compilationQueue.isEmpty; - _compilationQueue.add(request); - if (!isEmpty) { - return; + Future _compile(Uri mainUri, Metadata metadata) async { + var firstCompile = false; + CompileResult? compilerOutput; + final contents = await _generateEntrypoint(mainUri, metadata); + final tempFile = File(p.join(_outputDillDirectory.path, 'test.dart')) + ..writeAsStringSync(contents); + + if (_frontendServerClient == null) { + compilerOutput = await _createCompiler(tempFile.uri); + firstCompile = true; + } else { + compilerOutput = + await _frontendServerClient!.compile([tempFile.uri]); } - while (_compilationQueue.isNotEmpty) { - final request = _compilationQueue.first; - var firstCompile = false; - CompileResult? compilerOutput; - final contents = - await _generateEntrypoint(request.mainUri, request.metadata); - final tempFile = File(p.join(_outputDillDirectory.path, 'test.dart')) - ..writeAsStringSync(contents); - - if (_frontendServerClient == null) { - compilerOutput = await _createCompiler(tempFile.uri); - firstCompile = true; - } else { - compilerOutput = - await _frontendServerClient!.compile([tempFile.uri]); - } - // The client is guaranteed initialized at this point. - final client = _frontendServerClient!; - final outputPath = compilerOutput?.dillOutput; - if (outputPath == null) { - request.result.complete(CompilationResponse( - compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), - errorCount: compilerOutput?.errorCount ?? 0)); - } else { - final outputFile = File(outputPath); - final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); - final testCache = File(_dillCachePath); - // Keep the cache file up-to-date and use the size of the kernel file - // as an approximation for how many packages are included. Larger files - // are prefered, since re-using more packages will reduce the number of - // files the frontend server needs to load and parse. - if (firstCompile || - !testCache.existsSync() || - (testCache.lengthSync() < outputFile.lengthSync())) { - if (!testCache.parent.existsSync()) { - testCache.parent.createSync(recursive: true); - } - await outputFile.copy(_dillCachePath); - } - request.result.complete(CompilationResponse( - compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), - errorCount: compilerOutput?.errorCount ?? 0, - kernelOutputUri: kernelReadyToRun.absolute.uri)); - client.accept(); - client.reset(); + // The client is guaranteed initialized at this point. + final client = _frontendServerClient!; + final outputPath = compilerOutput?.dillOutput; + if (outputPath == null) { + return CompilationResponse( + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + errorCount: compilerOutput?.errorCount ?? 0); + } + + final outputFile = File(outputPath); + final kernelReadyToRun = await outputFile.copy('${tempFile.path}.dill'); + final testCache = File(_dillCachePath); + // Keep the cache file up-to-date and use the size of the kernel file + // as an approximation for how many packages are included. Larger files + // are prefered, since re-using more packages will reduce the number of + // files the frontend server needs to load and parse. + if (firstCompile || + !testCache.existsSync() || + (testCache.lengthSync() < outputFile.lengthSync())) { + if (!testCache.parent.existsSync()) { + testCache.parent.createSync(recursive: true); } - _compilationQueue.removeAt(0); + await outputFile.copy(_dillCachePath); } + + client.accept(); + client.reset(); + return CompilationResponse( + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + errorCount: compilerOutput?.errorCount ?? 0, + kernelOutputUri: kernelReadyToRun.absolute.uri); } Future _createCompiler(Uri testUri) async { From 2e2fac3f2f09d5a6124cea7d3fb9de0eae5f806b Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 13:51:42 -0700 Subject: [PATCH 22/24] improve error handling of a sigkill during suite loading --- .../test_core/lib/src/runner/vm/platform.dart | 38 +++++++----- .../lib/src/runner/vm/test_compiler.dart | 58 +++++++++++-------- 2 files changed, 57 insertions(+), 39 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index 23c608571..f0fe96506 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -7,6 +7,7 @@ import 'dart:developer'; import 'dart:io'; import 'dart:isolate'; +import 'package:async/async.dart'; import 'package:coverage/coverage.dart'; import 'package:path/path.dart' as p; import 'package:stream_channel/isolate_channel.dart'; @@ -36,6 +37,7 @@ class VMPlatform extends PlatformPlugin { final _config = Configuration.current; final _compiler = TestCompiler(p.join(p.current, '.dart_tool', 'pkg_test_kernel.bin')); + final _closeMemo = AsyncMemoizer(); VMPlatform(); @@ -44,15 +46,16 @@ class VMPlatform extends PlatformPlugin { throw UnimplementedError(); @override - Future load(String path, SuitePlatform platform, + Future load(String path, SuitePlatform platform, SuiteConfiguration suiteConfig, Object message) async { assert(platform.runtime == Runtime.vm); var receivePort = ReceivePort(); - Isolate isolate; + Isolate? isolate; try { isolate = await _spawnIsolate(path, receivePort.sendPort, suiteConfig.metadata); + if (isolate == null) return null; } catch (error) { receivePort.close(); rethrow; @@ -62,7 +65,7 @@ class VMPlatform extends PlatformPlugin { StreamSubscription? eventSub; var channel = IsolateChannel.connectReceive(receivePort) .transformStream(StreamTransformer.fromHandlers(handleDone: (sink) { - isolate.kill(); + isolate!.kill(); eventSub?.cancel(); client?.dispose(); sink.close(); @@ -113,25 +116,28 @@ class VMPlatform extends PlatformPlugin { } @override - Future close() async { - await _compiler.dispose(); - } + Future close() => _closeMemo.runOnce(() => _compiler.dispose()); /// Spawns an isolate and passes it [message]. /// /// This isolate connects an [IsolateChannel] to [message] and sends the /// serialized tests over that channel. - Future _spawnIsolate( + Future _spawnIsolate( String path, SendPort message, Metadata suiteMetadata) async { - var precompiledPath = _config.suiteDefaults.precompiledPath; - if (precompiledPath != null) { - return _spawnPrecompiledIsolate(path, message, precompiledPath); - } else if (_config.pubServeUrl != null) { - return _spawnPubServeIsolate(path, message, _config.pubServeUrl!); - } else if (_config.useDataIsolateStrategy) { - return _spawnDataIsolate(path, message, suiteMetadata); - } else { - return _spawnKernelIsolate(path, message, suiteMetadata); + try { + var precompiledPath = _config.suiteDefaults.precompiledPath; + if (precompiledPath != null) { + return _spawnPrecompiledIsolate(path, message, precompiledPath); + } else if (_config.pubServeUrl != null) { + return _spawnPubServeIsolate(path, message, _config.pubServeUrl!); + } else if (_config.useDataIsolateStrategy) { + return _spawnDataIsolate(path, message, suiteMetadata); + } else { + return _spawnKernelIsolate(path, message, suiteMetadata); + } + } catch (_) { + if (_closeMemo.hasRun) return null; + rethrow; } } diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart index 62e999f20..0fb955464 100644 --- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:io'; +import 'package:async/async.dart'; import 'package:path/path.dart' as p; import 'package:pool/pool.dart'; import 'package:test_api/backend.dart'; // ignore: deprecated_member_use @@ -18,8 +19,11 @@ class CompilationResponse { final int errorCount; final Uri? kernelOutputUri; - CompilationResponse( + const CompilationResponse( {this.compilerOutput, this.errorCount = 0, this.kernelOutputUri}); + + static const _wasShutdown = CompilationResponse( + errorCount: 1, compilerOutput: 'Compiler no longer active.'); } class TestCompiler { @@ -31,6 +35,10 @@ class TestCompiler { File(p.join(_outputDillDirectory.path, 'output.dill')); FrontendServerClient? _frontendServerClient; + final _closeMemo = AsyncMemoizer(); + + /// No work is done until the first call to [compile] is recieved, at which + /// point the compiler process is started. TestCompiler(this._dillCachePath) : _outputDillDirectory = Directory.systemTemp.createTempSync('dart_test.'); @@ -44,22 +52,19 @@ class TestCompiler { /// /// The entrypoint [mainDart] is wrapped in a script which bootstraps it with /// a call to `internalBootstrapVmTest`. - Future compile(Uri mainDart, Metadata metadata) { - if (_compilePool.isClosed) { - return Future.value(CompilationResponse( - errorCount: 1, compilerOutput: 'Compiler was already shut down.')); - } + Future compile(Uri mainDart, Metadata metadata) async { + if (_compilePool.isClosed) return CompilationResponse._wasShutdown; return _compilePool.withResource(() => _compile(mainDart, metadata)); } - Future dispose() async { - await _compilePool.close(); - _frontendServerClient?.kill(); - _frontendServerClient = null; - if (_outputDillDirectory.existsSync()) { - _outputDillDirectory.deleteSync(recursive: true); - } - } + Future dispose() => _closeMemo.runOnce(() async { + await _compilePool.close(); + _frontendServerClient?.kill(); + _frontendServerClient = null; + if (_outputDillDirectory.existsSync()) { + _outputDillDirectory.deleteSync(recursive: true); + } + }); Future _generateEntrypoint( Uri testUri, Metadata suiteMetadata) async { @@ -78,21 +83,30 @@ class TestCompiler { } Future _compile(Uri mainUri, Metadata metadata) async { + if (_closeMemo.hasRun) return CompilationResponse._wasShutdown; var firstCompile = false; CompileResult? compilerOutput; final contents = await _generateEntrypoint(mainUri, metadata); final tempFile = File(p.join(_outputDillDirectory.path, 'test.dart')) ..writeAsStringSync(contents); - if (_frontendServerClient == null) { - compilerOutput = await _createCompiler(tempFile.uri); - firstCompile = true; - } else { - compilerOutput = - await _frontendServerClient!.compile([tempFile.uri]); + try { + if (_frontendServerClient == null) { + compilerOutput = await _createCompiler(tempFile.uri); + firstCompile = true; + } else { + compilerOutput = + await _frontendServerClient!.compile([tempFile.uri]); + } + } catch (e, s) { + if (_closeMemo.hasRun) return CompilationResponse._wasShutdown; + return CompilationResponse(errorCount: 1, compilerOutput: '$e\n$s'); + } finally { + _frontendServerClient?.accept(); + _frontendServerClient?.reset(); } + // The client is guaranteed initialized at this point. - final client = _frontendServerClient!; final outputPath = compilerOutput?.dillOutput; if (outputPath == null) { return CompilationResponse( @@ -116,8 +130,6 @@ class TestCompiler { await outputFile.copy(_dillCachePath); } - client.accept(); - client.reset(); return CompilationResponse( compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), errorCount: compilerOutput?.errorCount ?? 0, From aa23f0c1a787876118486dc156bf5b840f3c95cd Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 13:55:38 -0700 Subject: [PATCH 23/24] Update pkgs/test_core/lib/src/runner/configuration.dart Co-authored-by: Nate Bosch --- pkgs/test_core/lib/src/runner/configuration.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/test_core/lib/src/runner/configuration.dart b/pkgs/test_core/lib/src/runner/configuration.dart index 9be23afb1..5d6098dbd 100644 --- a/pkgs/test_core/lib/src/runner/configuration.dart +++ b/pkgs/test_core/lib/src/runner/configuration.dart @@ -174,7 +174,7 @@ class Configuration { }); Set? _knownPresets; - /// Whether to use the original `data:` uri isolate spawning strategy for vm + /// Whether to use the original `data:` URI isolate spawning strategy for VM /// tests. /// /// This can make more sense than the default strategy in systems such as From 98cc83d1a0b978cb294dec29fd688352f4a9c644 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 22 Mar 2021 13:55:47 -0700 Subject: [PATCH 24/24] Update pkgs/test/test/runner/data_isolate_strategy_test.dart Co-authored-by: Nate Bosch --- pkgs/test/test/runner/data_isolate_strategy_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test/test/runner/data_isolate_strategy_test.dart b/pkgs/test/test/runner/data_isolate_strategy_test.dart index 1678b91c4..7063d50e4 100644 --- a/pkgs/test/test/runner/data_isolate_strategy_test.dart +++ b/pkgs/test/test/runner/data_isolate_strategy_test.dart @@ -31,8 +31,8 @@ void main() { group('The data isolate strategy', () { test('can be enabled', () async { - /// We confirm it is enabled by checking the error output for an invalid - /// test, it looks a bit different. + // We confirm it is enabled by checking the error output for an invalid + // test, it looks a bit different. await d.file('test.dart', 'invalid Dart file').create(); var test = await runTest(['--use-data-isolate-strategy', 'test.dart']);