From 98c117bb38c43d1fc65d68fffabb9687e1b9513c Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 15 Aug 2018 12:22:30 -0700 Subject: [PATCH] Implement `flutter test -j` (#20493) --- .../flutter_tools/bin/fuchsia_tester.dart | 3 ++ .../flutter_tools/lib/src/commands/test.dart | 35 +++++++++++----- .../flutter_tools/lib/src/test/runner.dart | 41 ++++++++----------- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/packages/flutter_tools/bin/fuchsia_tester.dart b/packages/flutter_tools/bin/fuchsia_tester.dart index 10d8acafcce..b77017d5e5a 100644 --- a/packages/flutter_tools/bin/fuchsia_tester.dart +++ b/packages/flutter_tools/bin/fuchsia_tester.dart @@ -3,12 +3,14 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:math' as math; import 'package:args/args.dart'; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/context_runner.dart'; import 'package:flutter_tools/src/dart/package_map.dart'; @@ -121,6 +123,7 @@ Future run(List args) async { enableObservatory: collector != null, previewDart2: true, precompiledDillPath: dillFile.path, + concurrency: math.max(1, platform.numberOfProcessors - 2), ); if (collector != null) { diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index 1541dfc0cd5..ca8c571cf92 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -3,9 +3,11 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:math' as math; import '../base/common.dart'; import '../base/file_system.dart'; +import '../base/platform.dart'; import '../cache.dart'; import '../runner/flutter_command.dart'; import '../test/coverage_collector.dart'; @@ -77,7 +79,12 @@ class TestCommand extends FlutterCommand { negatable: false, help: 'Whether matchesGoldenFile() calls within your test methods should\n' 'update the golden files rather than test for an existing match.', - ); + ) + ..addOption('concurrency', + abbr: 'j', + defaultsTo: math.max(1, platform.numberOfProcessors - 2).toString(), + help: 'The number of concurrent test processes to run.', + valueHelp: 'jobs'); } @override @@ -91,10 +98,10 @@ class TestCommand extends FlutterCommand { await super.validateCommand(); if (!fs.isFileSync('pubspec.yaml')) { throwToolExit( - 'Error: No pubspec.yaml file found in the current working directory.\n' - 'Run this command from the root of your project. Test files must be\n' - 'called *_test.dart and must reside in the package\'s \'test\'\n' - 'directory (or one of its subdirectories).'); + 'Error: No pubspec.yaml file found in the current working directory.\n' + 'Run this command from the root of your project. Test files must be\n' + 'called *_test.dart and must reside in the package\'s \'test\'\n' + 'directory (or one of its subdirectories).'); } } @@ -108,8 +115,16 @@ class TestCommand extends FlutterCommand { final bool startPaused = argResults['start-paused']; if (startPaused && files.length != 1) { throwToolExit( - 'When using --start-paused, you must specify a single test file to run.', - exitCode: 1); + 'When using --start-paused, you must specify a single test file to run.', + exitCode: 1, + ); + } + + final int jobs = int.tryParse(argResults['concurrency']); + if (jobs == null || jobs <= 0 || !jobs.isFinite) { + throwToolExit( + 'Could not parse -j/--concurrency argument. It must be an integer greater than zero.' + ); } Directory workDir; @@ -123,7 +138,7 @@ class TestCommand extends FlutterCommand { if (files.isEmpty) { throwToolExit( 'Test directory "${workDir.path}" does not appear to contain any test files.\n' - 'Test files must be in that directory and end with the pattern "_test.dart".' + 'Test files must be in that directory and end with the pattern "_test.dart".' ); } } @@ -135,8 +150,7 @@ class TestCommand extends FlutterCommand { final bool machine = argResults['machine']; if (collector != null && machine) { - throwToolExit( - "The test command doesn't support --machine and coverage together"); + throwToolExit("The test command doesn't support --machine and coverage together"); } TestWatcher watcher; @@ -161,6 +175,7 @@ class TestCommand extends FlutterCommand { previewDart2: argResults['preview-dart-2'], trackWidgetCreation: argResults['track-widget-creation'], updateGoldens: argResults['update-goldens'], + concurrency: jobs, ); if (collector != null) { diff --git a/packages/flutter_tools/lib/src/test/runner.dart b/packages/flutter_tools/lib/src/test/runner.dart index bc97ec6df57..a286df6bbc6 100644 --- a/packages/flutter_tools/lib/src/test/runner.dart +++ b/packages/flutter_tools/lib/src/test/runner.dart @@ -5,8 +5,8 @@ import 'dart:async'; import 'package:args/command_runner.dart'; -// ignore: implementation_imports -import 'package:test/src/executable.dart' as test; +import 'package:meta/meta.dart'; +import 'package:test/src/executable.dart' as test; // ignore: implementation_imports import '../artifacts.dart'; import '../base/common.dart'; @@ -21,20 +21,21 @@ import 'watcher.dart'; /// Runs tests using package:test and the Flutter engine. Future runTests( - List testFiles, { - Directory workDir, - List names = const [], - List plainNames = const [], - bool enableObservatory = false, - bool startPaused = false, - bool ipv6 = false, - bool machine = false, - bool previewDart2 = false, - String precompiledDillPath, - bool trackWidgetCreation = false, - bool updateGoldens = false, - TestWatcher watcher, - }) async { + List testFiles, { + Directory workDir, + List names = const [], + List plainNames = const [], + bool enableObservatory = false, + bool startPaused = false, + bool ipv6 = false, + bool machine = false, + bool previewDart2 = false, + String precompiledDillPath, + bool trackWidgetCreation = false, + bool updateGoldens = false, + TestWatcher watcher, + @required int concurrency, +}) async { if (trackWidgetCreation && !previewDart2) { throw new UsageException( '--track-widget-creation is valid only when --preview-dart-2 is specified.', @@ -54,13 +55,7 @@ Future runTests( testArgs.addAll(['-r', 'compact']); } - if (enableObservatory) { // (In particular, for collecting code coverage.) - // Turn on concurrency, but just barely. This is a trade-off between running - // too many tests such that they all time out, and too few tests such that - // the tests overall take too much time. The current number is empirically - // based on what our infrastructure can handle, which isn't ideal... - testArgs.add('--concurrency=2'); - } + testArgs.add('--concurrency=$concurrency'); for (String name in names) { testArgs..add('--name')..add(name);