diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index a2cfadbbd..e814d0ebb 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,3 +1,10 @@ +## 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.9 * Disable stack trace chaining by default. It can be re-enabled by explicitly diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 9516d3c1a..5cff44ccc 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 1.16.9 +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/test/runner/data_isolate_strategy_test.dart b/pkgs/test/test/runner/data_isolate_strategy_test.dart new file mode 100644 index 000000000..7063d50e4 --- /dev/null +++ b/pkgs/test/test/runner/data_isolate_strategy_test.dart @@ -0,0 +1,66 @@ +// 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:convert'; +import 'dart:isolate'; + +import 'package:package_config/package_config.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +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); + }); + }); +} diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index dd0cb3278..ec958294f 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 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. @@ -167,8 +170,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 +188,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 +205,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/test/runner/test_on_test.dart b/pkgs/test/test/runner/test_on_test.dart index f1d9574c6..67109f290 100644 --- a/pkgs/test/test/runner/test_on_test.dart +++ b/pkgs/test/test/runner/test_on_test.dart @@ -5,7 +5,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'; @@ -14,6 +17,20 @@ 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('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/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index a3a04ebc3..b628cafd0 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -1,7 +1,12 @@ -## 0.3.20 +## 0.3.20-dev * Disable stack trace chaining by default. +* 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..5d6098dbd 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..1f8b0fca6 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -107,6 +107,12 @@ 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\n' + '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', help: 'Use the specified seed to randomize the execution order of test' ' cases.\n' @@ -268,6 +274,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 989feec38..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'; @@ -14,6 +15,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'; @@ -25,6 +27,7 @@ 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'; @@ -32,6 +35,9 @@ import 'environment.dart'; class VMPlatform extends PlatformPlugin { /// The test runner configuration. final _config = Configuration.current; + final _compiler = + TestCompiler(p.join(p.current, '.dart_tool', 'pkg_test_kernel.bin')); + final _closeMemo = AsyncMemoizer(); VMPlatform(); @@ -40,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; @@ -58,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(); @@ -108,21 +115,45 @@ class VMPlatform extends PlatformPlugin { return await controller.suite; } + @override + 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 { - return _spawnDataIsolate(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; } } + + /// 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( @@ -130,11 +161,8 @@ Future _spawnDataIsolate( 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); } 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..0fb955464 --- /dev/null +++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart @@ -0,0 +1,154 @@ +// 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. + +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 +import 'package:frontend_server_client/frontend_server_client.dart'; + +import '../package_version.dart'; +import '../../util/package_config.dart'; + +class CompilationResponse { + final String? compilerOutput; + final int errorCount; + final Uri? kernelOutputUri; + + const CompilationResponse( + {this.compilerOutput, this.errorCount = 0, this.kernelOutputUri}); + + static const _wasShutdown = CompilationResponse( + errorCount: 1, compilerOutput: 'Compiler no longer active.'); +} + +class TestCompiler { + final _compilePool = Pool(1); + final String _dillCachePath; + final Directory _outputDillDirectory; + + late final _outputDill = + 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.'); + + /// 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) async { + if (_compilePool.isClosed) return CompilationResponse._wasShutdown; + return _compilePool.withResource(() => _compile(mainDart, metadata)); + } + + 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 { + return ''' + ${suiteMetadata.languageVersionComment ?? await rootPackageLanguageVersionComment} + import "dart:isolate"; + + import "package:test_core/src/bootstrap/vm.dart"; + + import "$testUri" as test; + + void main(_, SendPort sendPort) { + internalBootstrapVmTest(() => test.main, sendPort); + } + '''; + } + + 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); + + 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 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); + } + await outputFile.copy(_dillCachePath); + } + + return CompilationResponse( + compilerOutput: compilerOutput?.compilerOutputLines.join('\n'), + errorCount: compilerOutput?.errorCount ?? 0, + kernelOutputUri: kernelReadyToRun.absolute.uri); + } + + Future _createCompiler(Uri testUri) async { + final platformDill = 'lib/_internal/vm_platform_strong.dill'; + final sdkRoot = + Directory(p.relative(p.join(Platform.resolvedExecutable, '..', '..'))) + .uri; + var client = _frontendServerClient = await FrontendServerClient.start( + testUri.toString(), + _outputDill.path, + platformDill, + sdkRoot: sdkRoot.path, + packagesJson: (await packageConfigUri).toFilePath(), + printIncrementalDependencies: false, + ); + return client.compile(); + } +} diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index d7cc05cf0..afea13c06 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -1,5 +1,5 @@ name: test_core -version: 0.3.20 +version: 0.3.20-dev description: A basic library for writing tests and running them on the VM. homepage: https://github.com/dart-lang/test/blob/master/pkgs/test_core @@ -13,6 +13,7 @@ dependencies: boolean_selector: ^2.1.0 collection: ^1.15.0 coverage: ^1.0.0 + frontend_server_client: ^2.0.0 glob: ^2.0.0 io: ^1.0.0 meta: ^1.3.0