From 256a5e567ed5d05012717e0b71149d326a08c0e6 Mon Sep 17 00:00:00 2001 From: Jonas Sander <29028262+Jonas-Sander@users.noreply.github.com> Date: Fri, 27 Jan 2023 22:36:33 +0100 Subject: [PATCH] Make `sz test` more performant. (#340) Tests are now run concurrently for the different packages, instead of only one at a time. Time to run tests in GitHub CI: Old time: 18min 36s New time: 8-11 min --- .github/workflows/main.yml | 2 +- .../lib/src/commands/src/analyze_command.dart | 8 +- .../lib/src/commands/src/test_command.dart | 99 ++++++++----------- .../lib/src/common/src/shared_args.dart | 12 +++ 4 files changed, 57 insertions(+), 64 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1084f677a..a630b49c6 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -113,7 +113,7 @@ jobs: - run: echo $(pwd)/bin >> $GITHUB_PATH - name: Run tests via "sz test" - run: sz test -v + run: sz test -c 4 # Uploads the results of failed tests as .zip to GitHub. # diff --git a/tools/sz_repo_cli/lib/src/commands/src/analyze_command.dart b/tools/sz_repo_cli/lib/src/commands/src/analyze_command.dart index dd69e95b0..af273c4f0 100644 --- a/tools/sz_repo_cli/lib/src/commands/src/analyze_command.dart +++ b/tools/sz_repo_cli/lib/src/commands/src/analyze_command.dart @@ -15,13 +15,6 @@ import 'package:sz_repo_cli/src/common/common.dart'; class AnalyzeCommand extends Command { AnalyzeCommand(this.repo) { argParser - ..addOption( - maxConcurrentPackagesOptionName, - abbr: 'c', - defaultsTo: '5', - help: - 'How many packages at most should be processed at once. Helpful for a CI or not as powerful PCs to not have so much processing at once.', - ) ..addFlag( 'verbose', abbr: 'v', @@ -29,6 +22,7 @@ class AnalyzeCommand extends Command { negatable: false, defaultsTo: false, ) + ..addConcurrencyOption(defaultMaxConcurrency: 5) ..addPackageTimeoutOption(defaultInMinutes: 7); } diff --git a/tools/sz_repo_cli/lib/src/commands/src/test_command.dart b/tools/sz_repo_cli/lib/src/commands/src/test_command.dart index 3b84bcf50..5466ebd16 100644 --- a/tools/sz_repo_cli/lib/src/commands/src/test_command.dart +++ b/tools/sz_repo_cli/lib/src/commands/src/test_command.dart @@ -7,13 +7,13 @@ // SPDX-License-Identifier: EUPL-1.2 import 'dart:async'; +import 'dart:io'; import 'package:args/command_runner.dart'; - import 'package:sz_repo_cli/src/common/common.dart'; class TestCommand extends Command { - TestCommand(this._repo) { + TestCommand(this.repo) { argParser ..addFlag( 'verbose', @@ -22,9 +22,14 @@ class TestCommand extends Command { negatable: false, defaultsTo: false, ) + ..addConcurrencyOption(defaultMaxConcurrency: 5) ..addPackageTimeoutOption(defaultInMinutes: 10); } + static const maxConcurrentPackagesOptionName = 'max-concurrent-packages'; + + final SharezoneRepo repo; + @override final String name = 'test'; @@ -32,63 +37,45 @@ class TestCommand extends Command { final String description = 'Runs the Dart tests for all packages.\n\n' 'This command requires "flutter" to be in your path.'; - final SharezoneRepo _repo; - - Duration packageTimeout; - @override - Future run() async { - packageTimeout = argResults.packageTimeoutDuration; + Future run() async { isVerbose = argResults['verbose'] ?? false; - await _testFlutterApp(_repo.sharezoneFlutterApp); - await _testPackages(_repo.dartLibraries); - - print('All tests are passing!'); - } - - Future _testFlutterApp(Package flutterApp) async { - print('Starting flutter app tests...'); - - try { - await flutterApp.runTests().timeout(packageTimeout); - } catch (e) { - // Weil der Error ansonsten nicht ausgeprintet wird - print('$e'); - print('Not all flutter app tests passed!'); - throw ToolExit(1); + final _max = argResults[maxConcurrentPackagesOptionName]; + final maxNumberOfPackagesBeingProcessedConcurrently = _max != null + ? int.tryParse(argResults[maxConcurrentPackagesOptionName]) + // null wird nachher als "keine Begrenzung" gehandhabt. + : null; + + final taskRunner = ConcurrentPackageTaskRunner( + getCurrentDateTime: () => DateTime.now(), + ); + + final res = taskRunner + .runTaskForPackages( + packageStream: repo + .streamPackages() + .where((package) => package.hasTestDirectory), + runTask: (package) => package.runTests(), + maxNumberOfPackagesBeingProcessedConcurrently: + maxNumberOfPackagesBeingProcessedConcurrently, + perPackageTaskTimeout: argResults.packageTimeoutDuration, + ) + .asBroadcastStream(); + + final presenter = PackageTasksStatusPresenter(); + presenter.continuouslyPrintTaskStatusUpdatesToConsole(res); + + final failures = await res.allFailures; + + if (failures.isNotEmpty) { + print('There were failures. See above for more information.'); + await presenter.printFailedTasksSummary(failures); + exit(1); + } else { + print('All packages tested successfully!'); + exit(0); } - - print('All flutter app tests passed!'); - } - - Future _testPackages(DartLibraries dartLibraries) async { - final failingPackages = []; - await for (final package in dartLibraries - .streamPackages() - .where((package) => package.hasTestDirectory)) { - final packageName = package.name; - - print( - 'RUNNING $packageName ${[package.type.toReadableString()]} tests...'); - - try { - await package.runTests().timeout(packageTimeout); - print('COMPLETED $packageName tests'); - } catch (e) { - print('FAILURE: $e'); - failingPackages.add(packageName); - } - } - - print('\n\n'); - if (failingPackages.isNotEmpty) { - print('Tests for the following packages are failing (see above):'); - failingPackages.forEach((String package) { - print(' * $package'); - }); - throw ToolExit(1); - } - print('All Dart package tests passed!'); } } + diff --git a/tools/sz_repo_cli/lib/src/common/src/shared_args.dart b/tools/sz_repo_cli/lib/src/common/src/shared_args.dart index 96ff595a9..00c7bcad9 100644 --- a/tools/sz_repo_cli/lib/src/common/src/shared_args.dart +++ b/tools/sz_repo_cli/lib/src/common/src/shared_args.dart @@ -10,6 +10,7 @@ import 'package:args/args.dart'; import 'package:meta/meta.dart'; const _packageTimeoutName = 'package-timeout-minutes'; +const maxConcurrentPackagesOptionName = 'max-concurrent-packages'; extension AddPackageTimeout on ArgParser { void addPackageTimeoutOption({@required int defaultInMinutes}) { @@ -20,6 +21,17 @@ extension AddPackageTimeout on ArgParser { defaultsTo: '$defaultInMinutes', ); } + + void addConcurrencyOption({@required int defaultMaxConcurrency}) { + addOption( + maxConcurrentPackagesOptionName, + abbr: 'c', + defaultsTo: '$defaultMaxConcurrency', + help: ''' +How many packages should be processed concurrently at most. Values <= 0 will cause all packages to be processed at once (no concurrency limit). +Limiting concurrency is helpful for not as powerful machines like e.g. CI-runner.''', + ); + } } extension PackageTimeoutArgResult on ArgResults {