Skip to content

Commit

Permalink
Make sz test more performant. (#340)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jonas-Sander authored Jan 27, 2023
1 parent 0e0c53a commit 256a5e5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
8 changes: 1 addition & 7 deletions tools/sz_repo_cli/lib/src/commands/src/analyze_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,14 @@ 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',
help: 'if verbose output should be printed (helpful for debugging)',
negatable: false,
defaultsTo: false,
)
..addConcurrencyOption(defaultMaxConcurrency: 5)
..addPackageTimeoutOption(defaultInMinutes: 7);
}

Expand Down
99 changes: 43 additions & 56 deletions tools/sz_repo_cli/lib/src/commands/src/test_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -22,73 +22,60 @@ 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';

@override
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<void> run() async {
packageTimeout = argResults.packageTimeoutDuration;
Future<Null> 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<void> _testPackages(DartLibraries dartLibraries) async {
final failingPackages = <String>[];
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!');
}
}

12 changes: 12 additions & 0 deletions tools/sz_repo_cli/lib/src/common/src/shared_args.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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}) {
Expand All @@ -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 {
Expand Down

0 comments on commit 256a5e5

Please sign in to comment.