From 658880ecd34baa7941ff03cec90a6830feddaf3f Mon Sep 17 00:00:00 2001 From: Jonas Sander <29028262+Jonas-Sander@users.noreply.github.com> Date: Fri, 7 Apr 2023 14:31:48 +0200 Subject: [PATCH 1/2] `sz` CLI: Move logic out of `package.dart`. --- .../lib/src/commands/src/analyze_command.dart | 27 ++- .../lib/src/commands/src/format_command.dart | 21 +- .../lib/src/commands/src/pub_get_command.dart | 23 +- .../lib/src/commands/src/test_command.dart | 97 ++++++++- .../lib/src/common/src/package.dart | 205 ++---------------- 5 files changed, 183 insertions(+), 190 deletions(-) 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 b3876c629..76542ed9c 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 @@ -8,8 +8,12 @@ import 'dart:async'; +import 'package:sz_repo_cli/src/commands/src/format_command.dart'; import 'package:sz_repo_cli/src/common/common.dart'; +import 'fix_comment_spacing_command.dart'; +import 'pub_get_command.dart'; + class AnalyzeCommand extends ConcurrentCommand { AnalyzeCommand(SharezoneRepo repo) : super(repo); @@ -24,7 +28,26 @@ class AnalyzeCommand extends ConcurrentCommand { Duration get defaultPackageTimeout => Duration(minutes: 5); @override - Future runTaskForPackage(Package package) { - return package.analyzePackage(); + Future runTaskForPackage(Package package) => analyzePackage(package); +} + +Future analyzePackage(Package package) async { + await getPackage(package); + await _runDartAnalyze(package); + await formatCode(package, throwIfCodeChanged: true); + await _checkForCommentsWithBadSpacing(package); +} + +Future _runDartAnalyze(Package package) { + return runProcessSucessfullyOrThrow( + 'fvm', ['dart', 'analyze', '--fatal-infos', '--fatal-warnings'], + workingDirectory: package.path); +} + +Future _checkForCommentsWithBadSpacing(Package package) async { + if (doesPackageIncludeFilesWithBadCommentSpacing(package.path)) { + throw Exception( + 'Package ${package.name} has comments with bad spacing. Fix them by running the `sz fix-comment-spacing` command.'); } + return; } diff --git a/tools/sz_repo_cli/lib/src/commands/src/format_command.dart b/tools/sz_repo_cli/lib/src/commands/src/format_command.dart index 338d4b9e6..a0bfaacc4 100644 --- a/tools/sz_repo_cli/lib/src/commands/src/format_command.dart +++ b/tools/sz_repo_cli/lib/src/commands/src/format_command.dart @@ -24,7 +24,22 @@ class FormatCommand extends ConcurrentCommand { Duration get defaultPackageTimeout => Duration(minutes: 3); @override - Future runTaskForPackage(Package package) { - return package.formatCode(); - } + Future runTaskForPackage(Package package) => formatCode(package); +} + +Future formatCode( + Package package, { + /// Throws if code is not already formatted properly. + /// Useful for code analysis in CI. + bool throwIfCodeChanged = false, +}) { + return runProcessSucessfullyOrThrow( + 'fvm', + [ + 'dart', + 'format', + if (throwIfCodeChanged) '--set-exit-if-changed', + '.', + ], + workingDirectory: package.path); } diff --git a/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart b/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart index 82050c305..921594b7f 100644 --- a/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart +++ b/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart @@ -27,7 +27,26 @@ class PubGetCommand extends ConcurrentCommand { Duration get defaultPackageTimeout => Duration(minutes: 5); @override - Future runTaskForPackage(Package package) { - return package.getPackages(); + Future runTaskForPackage(Package package) => getPackage(package); +} + +Future getPackage(Package package) async { + if (package.type == PackageType.flutter) { + await getPackagesFlutter(package); + } else { + await getPackagesDart(package); } } + +Future getPackagesDart(Package package) async { + await runProcessSucessfullyOrThrow('fvm', ['dart', 'pub', 'get'], + workingDirectory: package.path); +} + +Future getPackagesFlutter(Package package) async { + await runProcessSucessfullyOrThrow( + 'fvm', + ['flutter', 'pub', 'get'], + workingDirectory: package.path, + ); +} 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 7bf66021d..43a0bd99e 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 @@ -8,8 +8,11 @@ import 'dart:async'; +import 'package:meta/meta.dart'; import 'package:sz_repo_cli/src/common/common.dart'; +import 'pub_get_command.dart'; + class TestCommand extends ConcurrentCommand { TestCommand(SharezoneRepo repo) : super(repo) { argParser.addFlag( @@ -44,7 +47,8 @@ class TestCommand extends ConcurrentCommand { if (argResults['only-goldens'] as bool) { return repo.streamPackages().where( (package) => - package is FlutterPackage && package.hasGoldenTestsDirectory, + package.type == PackageType.flutter && + package.hasGoldenTestsDirectory, ); } @@ -53,9 +57,98 @@ class TestCommand extends ConcurrentCommand { @override Future runTaskForPackage(Package package) { - return package.runTests( + return runTests( + package, excludeGoldens: argResults['exclude-goldens'] as bool, onlyGoldens: argResults['only-goldens'] as bool, ); } } + +Future runTests( + Package package, { + @required bool excludeGoldens, + @required bool onlyGoldens, +}) { + if (package.type == PackageType.flutter) { + return _runTestsFlutter( + package, + excludeGoldens: excludeGoldens, + onlyGoldens: onlyGoldens, + ); + } else { + return _runTestsDart( + package, + excludeGoldens: excludeGoldens, + onlyGoldens: onlyGoldens, + ); + } +} + +Future _runTestsDart( + Package package, { + // We can ignore the "excludeGoldens" parameter here because Dart packages + // don't have golden tests. + @required bool excludeGoldens, + @required bool onlyGoldens, +}) async { + if (onlyGoldens) { + // Golden tests are only run in the flutter package. + return; + } + + await getPackage(package); + + await runProcessSucessfullyOrThrow( + 'fvm', + ['dart', 'test'], + workingDirectory: package.path, + ); +} + +Future _runTestsFlutter( + Package package, { + @required bool excludeGoldens, + @required bool onlyGoldens, +}) async { + if (onlyGoldens) { + if (!package.hasGoldenTestsDirectory) { + return; + } + + await runProcessSucessfullyOrThrow( + 'fvm', + ['flutter', 'test', 'test_goldens'], + workingDirectory: package.path, + ); + return; + } + + // If the package has no golden tests, we need to use the normal test + // command. Otherwise the throws the Flutter tool throws an error that it + // couldn't find the "test_goldens" directory. + if (excludeGoldens || !package.hasGoldenTestsDirectory) { + await runProcessSucessfullyOrThrow( + 'fvm', + ['flutter', 'test'], + workingDirectory: package.path, + ); + return; + } + + /// Flutter test lässt automatisch flutter pub get laufen. + /// Deswegen muss nicht erst noch [getPackages] aufgerufen werden. + + await runProcessSucessfullyOrThrow( + 'fvm', + [ + 'flutter', + 'test', + // Directory for golden tests. + 'test_goldens', + // Directory for unit and widget tests. + 'test', + ], + workingDirectory: package.path, + ); +} diff --git a/tools/sz_repo_cli/lib/src/common/src/package.dart b/tools/sz_repo_cli/lib/src/common/src/package.dart index 4ce6a7bb4..5162770b8 100644 --- a/tools/sz_repo_cli/lib/src/common/src/package.dart +++ b/tools/sz_repo_cli/lib/src/common/src/package.dart @@ -8,42 +8,30 @@ import 'dart:io'; -import 'package:sz_repo_cli/src/commands/src/fix_comment_spacing_command.dart'; import 'package:meta/meta.dart'; -import 'package:path/path.dart' as path; +import 'package:path/path.dart' as p; import 'package:yaml/yaml.dart'; -import '../common.dart'; - enum PackageType { pureDart, flutter } -extension PackageTypeToReadableString on PackageType { - String toReadableString() { - switch (this) { - case PackageType.flutter: - return 'Flutter'; - case PackageType.pureDart: - return 'Dart'; - } - throw UnimplementedError(); - } -} - -abstract class Package { +class Package { final Directory location; + String get path => location.path; final String name; final PackageType type; final bool hasTestDirectory; + final bool hasGoldenTestsDirectory; Package({ @required this.location, @required this.name, @required this.type, @required this.hasTestDirectory, + @required this.hasGoldenTestsDirectory, }); factory Package.fromDirectory(Directory directory) { - final pubspecFile = File(path.join(directory.path, 'pubspec.yaml')); + final pubspecFile = File(p.join(directory.path, 'pubspec.yaml')); final YamlMap pubspecYaml = loadYaml(pubspecFile.readAsStringSync()); final YamlMap dependencies = pubspecYaml['dependencies'] ?? YamlMap(); final YamlMap devDependencies = @@ -53,178 +41,33 @@ abstract class Package { false; final name = pubspecYaml['name'] as String; final hasTestDirectory = - Directory(path.join(directory.path, 'test')).existsSync(); + Directory(p.join(directory.path, 'test')).existsSync(); final hasTestGoldensDirectory = - Directory(path.join(directory.path, 'test_goldens')).existsSync(); - - return containsFlutter - ? FlutterPackage( - location: directory, - name: name, - hasTestDirectory: hasTestDirectory, - hasGoldenTestsDirectory: hasTestGoldensDirectory, - ) - : DartPackage( - location: directory, - name: name, - hasTestDirectory: hasTestDirectory, - ); - } - - Future getPackages(); - - Future runTests({ - @required bool excludeGoldens, - @required bool onlyGoldens, - }); - - Future analyzePackage() async { - await getPackages(); - await _runDartAnalyze(); - await _runDartFormat(); - await _checkForCommentsWithBadSpacing(); - } - - Future _runDartAnalyze() { - return runProcessSucessfullyOrThrow( - 'fvm', ['dart', 'analyze', '--fatal-infos', '--fatal-warnings'], - workingDirectory: location.path); - } - - Future _runDartFormat() { - return runProcessSucessfullyOrThrow( - 'fvm', ['dart', 'format', '--set-exit-if-changed', '.'], - workingDirectory: location.path); - } - - Future _checkForCommentsWithBadSpacing() async { - if (doesPackageIncludeFilesWithBadCommentSpacing(location.path)) { - throw Exception( - 'Package $name has comments with bad spacing. Fix them by running the `sz fix-comment-spacing` command.'); - } - return; + Directory(p.join(directory.path, 'test_goldens')).existsSync(); + + return Package( + location: directory, + name: name, + hasTestDirectory: hasTestDirectory, + hasGoldenTestsDirectory: hasTestGoldensDirectory, + type: containsFlutter ? PackageType.flutter : PackageType.pureDart, + ); } @override String toString() { return '$runtimeType(name: $name)'; } - - Future formatCode() { - return runProcessSucessfullyOrThrow('fvm', ['dart', 'format', '.'], - workingDirectory: location.path); - } -} - -class DartPackage extends Package { - DartPackage({ - @required Directory location, - @required String name, - @required bool hasTestDirectory, - }) : super( - name: name, - location: location, - type: PackageType.pureDart, - hasTestDirectory: hasTestDirectory, - ); - - @override - Future getPackages() async { - await runProcessSucessfullyOrThrow('fvm', ['dart', 'pub', 'get'], - workingDirectory: location.path); - } - - @override - Future runTests({ - // We can ignore the "excludeGoldens" parameter here because Dart packages - // don't have golden tests. - @required bool excludeGoldens, - @required bool onlyGoldens, - }) async { - if (onlyGoldens) { - // Golden tests are only run in the flutter package. - return; - } - - await getPackages(); - - await runProcessSucessfullyOrThrow( - 'fvm', - ['dart', 'test'], - workingDirectory: location.path, - ); - } } -class FlutterPackage extends Package { - /// Whether the package has a "test_goldens" directory. - final bool hasGoldenTestsDirectory; - - FlutterPackage({ - @required Directory location, - @required String name, - @required bool hasTestDirectory, - @required this.hasGoldenTestsDirectory, - }) : super( - name: name, - location: location, - type: PackageType.flutter, - hasTestDirectory: hasTestDirectory, - ); - - @override - Future getPackages() async { - await runProcessSucessfullyOrThrow( - 'fvm', - ['flutter', 'pub', 'get'], - workingDirectory: location.path, - ); - } - - @override - Future runTests({ - @required bool excludeGoldens, - @required bool onlyGoldens, - }) async { - if (onlyGoldens) { - if (!hasGoldenTestsDirectory) { - return; - } - - await runProcessSucessfullyOrThrow( - 'fvm', - ['flutter', 'test', 'test_goldens'], - workingDirectory: location.path, - ); - return; - } - - // If the package has no golden tests, we need to use the normal test - // command. Otherwise the throws the Flutter tool throws an error that it - // couldn't find the "test_goldens" directory. - if (excludeGoldens || !hasGoldenTestsDirectory) { - await runProcessSucessfullyOrThrow( - 'fvm', - ['flutter', 'test'], - workingDirectory: location.path, - ); - return; +extension PackageTypeToReadableString on PackageType { + String toReadableString() { + switch (this) { + case PackageType.flutter: + return 'Flutter'; + case PackageType.pureDart: + return 'Dart'; } - - /// Flutter test lässt automatisch flutter pub get laufen. - /// Deswegen muss nicht erst noch [getPackages] aufgerufen werden. - - await runProcessSucessfullyOrThrow( - 'fvm', - [ - 'flutter', - 'test', - // Directory for golden tests. - 'test_goldens', - // Directory for unit and widget tests. - 'test', - ], - workingDirectory: location.path, - ); + throw UnimplementedError(); } } From d452d0cf497a759425977d3af1a115094fbb3575 Mon Sep 17 00:00:00 2001 From: Jonas Sander <29028262+Jonas-Sander@users.noreply.github.com> Date: Fri, 7 Apr 2023 14:35:53 +0200 Subject: [PATCH 2/2] Add `isFlutterPackage` and `isPureDartPackage`. --- tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart | 2 +- tools/sz_repo_cli/lib/src/commands/src/test_command.dart | 5 ++--- tools/sz_repo_cli/lib/src/common/src/package.dart | 2 ++ 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart b/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart index 921594b7f..52f78c2e9 100644 --- a/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart +++ b/tools/sz_repo_cli/lib/src/commands/src/pub_get_command.dart @@ -31,7 +31,7 @@ class PubGetCommand extends ConcurrentCommand { } Future getPackage(Package package) async { - if (package.type == PackageType.flutter) { + if (package.isFlutterPackage) { await getPackagesFlutter(package); } else { await getPackagesDart(package); 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 43a0bd99e..2f4245a49 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 @@ -47,8 +47,7 @@ class TestCommand extends ConcurrentCommand { if (argResults['only-goldens'] as bool) { return repo.streamPackages().where( (package) => - package.type == PackageType.flutter && - package.hasGoldenTestsDirectory, + package.isFlutterPackage && package.hasGoldenTestsDirectory, ); } @@ -70,7 +69,7 @@ Future runTests( @required bool excludeGoldens, @required bool onlyGoldens, }) { - if (package.type == PackageType.flutter) { + if (package.isFlutterPackage) { return _runTestsFlutter( package, excludeGoldens: excludeGoldens, diff --git a/tools/sz_repo_cli/lib/src/common/src/package.dart b/tools/sz_repo_cli/lib/src/common/src/package.dart index 5162770b8..fe5fcba6f 100644 --- a/tools/sz_repo_cli/lib/src/common/src/package.dart +++ b/tools/sz_repo_cli/lib/src/common/src/package.dart @@ -19,6 +19,8 @@ class Package { String get path => location.path; final String name; final PackageType type; + bool get isFlutterPackage => type == PackageType.flutter; + bool get isPureDartPackage => type == PackageType.pureDart; final bool hasTestDirectory; final bool hasGoldenTestsDirectory;