From fbf53f284b3919d4523fdeb56d89def38e22ab05 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 2 May 2022 17:44:12 -0400 Subject: [PATCH] [flutter_plugin_tools] Support non-plugin packages for `drive-examples` (#5468) --- script/tool/CHANGELOG.md | 5 + script/tool/lib/src/common/core.dart | 23 +-- .../src/common/package_looping_command.dart | 1 - script/tool/lib/src/common/plugin_utils.dart | 10 +- .../lib/src/common/repository_package.dart | 14 +- .../tool/lib/src/drive_examples_command.dart | 68 ++++++-- .../src/federation_safety_check_command.dart | 1 - .../lib/src/make_deps_path_based_command.dart | 3 +- .../tool/lib/src/publish_check_command.dart | 1 - .../tool/lib/src/publish_plugin_command.dart | 1 - .../tool/lib/src/pubspec_check_command.dart | 1 - script/tool/lib/src/readme_check_command.dart | 1 - script/tool/lib/src/test_command.dart | 2 +- .../tool/lib/src/version_check_command.dart | 1 - .../test/common/repository_package_test.dart | 43 ++++- .../create_all_plugins_app_command_test.dart | 1 - .../test/drive_examples_command_test.dart | 161 ++++++++++++++++++ script/tool/test/list_command_test.dart | 2 +- .../tool/test/pubspec_check_command_test.dart | 115 +++++++++---- script/tool/test/util.dart | 56 +++--- 20 files changed, 398 insertions(+), 112 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 1bce029a55..2ce644178f 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,8 @@ +## NEXT + +- `drive-examples` now supports non-plugin packages. +- Commands that iterate over examples now include non-Flutter example packages. + ## 0.8.4 - `readme-check` now validates that there's a info tag on code blocks to diff --git a/script/tool/lib/src/common/core.dart b/script/tool/lib/src/common/core.dart index de1cefd722..13678d720a 100644 --- a/script/tool/lib/src/common/core.dart +++ b/script/tool/lib/src/common/core.dart @@ -4,7 +4,6 @@ import 'package:colorize/colorize.dart'; import 'package:file/file.dart'; -import 'package:yaml/yaml.dart'; /// The signature for a print handler for commands that allow overriding the /// print destination. @@ -31,26 +30,14 @@ const String platformWindows = 'windows'; /// Key for enable experiment. const String kEnableExperiment = 'enable-experiment'; -/// Returns whether the given directory contains a Flutter package. -bool isFlutterPackage(FileSystemEntity entity) { +/// Returns whether the given directory is a Dart package. +bool isPackage(FileSystemEntity entity) { if (entity is! Directory) { return false; } - - try { - final File pubspecFile = entity.childFile('pubspec.yaml'); - final YamlMap pubspecYaml = - loadYaml(pubspecFile.readAsStringSync()) as YamlMap; - final YamlMap? dependencies = pubspecYaml['dependencies'] as YamlMap?; - if (dependencies == null) { - return false; - } - return dependencies.containsKey('flutter'); - } on FileSystemException { - return false; - } on YamlException { - return false; - } + // Per https://dart.dev/guides/libraries/create-library-packages#what-makes-a-library-package + return entity.childFile('pubspec.yaml').existsSync() && + entity.childDirectory('lib').existsSync(); } /// Prints `successMessage` in green. diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index b75aaa4a4a..b48743be31 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -10,7 +10,6 @@ import 'package:git/git.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'core.dart'; import 'plugin_command.dart'; diff --git a/script/tool/lib/src/common/plugin_utils.dart b/script/tool/lib/src/common/plugin_utils.dart index 94f294ebd9..f33d3d73bb 100644 --- a/script/tool/lib/src/common/plugin_utils.dart +++ b/script/tool/lib/src/common/plugin_utils.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common/repository_package.dart'; import 'package:yaml/yaml.dart'; @@ -111,13 +110,8 @@ YamlMap? _readPlatformPubspecSectionForPlugin( /// section from [plugin]'s pubspec.yaml, or null if either it is not present, /// or the pubspec couldn't be read. YamlMap? _readPluginPubspecSection(RepositoryPackage package) { - final File pubspecFile = package.pubspecFile; - if (!pubspecFile.existsSync()) { - return null; - } - final YamlMap pubspecYaml = - loadYaml(pubspecFile.readAsStringSync()) as YamlMap; - final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?; + final Pubspec pubspec = package.parsePubspec(); + final Map? flutterSection = pubspec.flutter; if (flutterSection == null) { return null; } diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index 72e7f948fe..76519e040a 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -8,6 +8,8 @@ import 'package:pubspec_parse/pubspec_parse.dart'; import 'core.dart'; +export 'package:pubspec_parse/pubspec_parse.dart' show Pubspec; + /// A package in the repository. // // TODO(stuartmorgan): Add more package-related info here, such as an on-demand @@ -59,6 +61,12 @@ class RepositoryPackage { /// Caches for future use. Pubspec parsePubspec() => _parsedPubspec; + /// Returns true if the package depends on Flutter. + bool requiresFlutter() { + final Pubspec pubspec = parsePubspec(); + return pubspec.dependencies.containsKey('flutter'); + } + /// True if this appears to be a federated plugin package, according to /// repository conventions. bool get isFederated => @@ -91,7 +99,7 @@ class RepositoryPackage { if (!exampleDirectory.existsSync()) { return []; } - if (isFlutterPackage(exampleDirectory)) { + if (isPackage(exampleDirectory)) { return [RepositoryPackage(exampleDirectory)]; } // Only look at the subdirectories of the example directory if the example @@ -99,8 +107,8 @@ class RepositoryPackage { // example directory for other Dart packages. return exampleDirectory .listSync() - .where((FileSystemEntity entity) => isFlutterPackage(entity)) - // isFlutterPackage guarantees that the cast to Directory is safe. + .where((FileSystemEntity entity) => isPackage(entity)) + // isPackage guarantees that the cast to Directory is safe. .map((FileSystemEntity entity) => RepositoryPackage(entity as Directory)); } diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 0e1efa842e..68ad9713a9 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -123,6 +123,8 @@ class DriveExamplesCommand extends PackageLoopingCommand { @override Future runForPackage(RepositoryPackage package) async { + final bool isPlugin = isFlutterPlugin(package); + if (package.isPlatformInterface && !package.getSingleExampleDeprecated().directory.existsSync()) { // Platform interface packages generally aren't intended to have @@ -131,23 +133,23 @@ class DriveExamplesCommand extends PackageLoopingCommand { 'Platform interfaces are not expected to have integration tests.'); } - final List deviceFlags = []; - for (final MapEntry> entry - in _targetDeviceFlags.entries) { - final String platform = entry.key; - if (pluginSupportsPlatform(platform, package)) { - deviceFlags.addAll(entry.value); - } else { - print('Skipping unsupported platform ${entry.key}...'); + // For plugin packages, skip if the plugin itself doesn't support any + // requested platform(s). + if (isPlugin) { + final Iterable requestedPlatforms = _targetDeviceFlags.keys; + final Iterable unsupportedPlatforms = requestedPlatforms.where( + (String platform) => !pluginSupportsPlatform(platform, package)); + for (final String platform in unsupportedPlatforms) { + print('Skipping unsupported platform $platform...'); + } + if (unsupportedPlatforms.length == requestedPlatforms.length) { + return PackageResult.skip( + '${package.displayName} does not support any requested platform.'); } - } - // If there is no supported target platform, skip the plugin. - if (deviceFlags.isEmpty) { - return PackageResult.skip( - '${package.displayName} does not support any requested platform.'); } int examplesFound = 0; + int supportedExamplesFound = 0; bool testsRan = false; final List errors = []; for (final RepositoryPackage example in package.getExamples()) { @@ -155,6 +157,15 @@ class DriveExamplesCommand extends PackageLoopingCommand { final String exampleName = getRelativePosixPath(example.directory, from: packagesDir); + // Skip examples that don't support any requested platform(s). + final List deviceFlags = _deviceFlagsForExample(example); + if (deviceFlags.isEmpty) { + print( + 'Skipping $exampleName; does not support any requested platforms.'); + continue; + } + ++supportedExamplesFound; + final List drivers = await _getDrivers(example); if (drivers.isEmpty) { print('No driver tests found for $exampleName'); @@ -195,14 +206,41 @@ class DriveExamplesCommand extends PackageLoopingCommand { } } if (!testsRan) { - printError('No driver tests were run ($examplesFound example(s) found).'); - errors.add('No tests ran (use --exclude if this is intentional).'); + // It is an error for a plugin not to have integration tests, because that + // is the only way to test the method channel communication. + if (isPlugin) { + printError( + 'No driver tests were run ($examplesFound example(s) found).'); + errors.add('No tests ran (use --exclude if this is intentional).'); + } else { + return PackageResult.skip(supportedExamplesFound == 0 + ? 'No example supports requested platform(s).' + : 'No example is configured for driver tests.'); + } } return errors.isEmpty ? PackageResult.success() : PackageResult.fail(errors); } + /// Returns the device flags for the intersection of the requested platforms + /// and the platforms supported by [example]. + List _deviceFlagsForExample(RepositoryPackage example) { + final List deviceFlags = []; + for (final MapEntry> entry + in _targetDeviceFlags.entries) { + final String platform = entry.key; + if (example.directory.childDirectory(platform).existsSync()) { + deviceFlags.addAll(entry.value); + } else { + final String exampleName = + getRelativePosixPath(example.directory, from: packagesDir); + print('Skipping unsupported platform $platform for $exampleName'); + } + } + return deviceFlags; + } + Future> _getDevicesForPlatform(String platform) async { final List deviceIds = []; diff --git a/script/tool/lib/src/federation_safety_check_command.dart b/script/tool/lib/src/federation_safety_check_command.dart index df9d86892e..383637a9e8 100644 --- a/script/tool/lib/src/federation_safety_check_command.dart +++ b/script/tool/lib/src/federation_safety_check_command.dart @@ -8,7 +8,6 @@ import 'package:git/git.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'common/core.dart'; import 'common/file_utils.dart'; diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index 45a4427d32..370fc3559f 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -3,15 +3,14 @@ // found in the LICENSE file. import 'package:file/file.dart'; -import 'package:flutter_plugin_tools/src/common/repository_package.dart'; import 'package:git/git.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'common/core.dart'; import 'common/git_version_finder.dart'; import 'common/plugin_command.dart'; +import 'common/repository_package.dart'; const int _exitPackageNotFound = 3; const int _exitCannotUpdatePubspec = 4; diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index 8fd96b818c..b6b83dabcb 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -10,7 +10,6 @@ import 'package:file/file.dart'; import 'package:http/http.dart' as http; import 'package:platform/platform.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'common/core.dart'; import 'common/package_looping_command.dart'; diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 28d17a3a24..05f0afd0c0 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -13,7 +13,6 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:yaml/yaml.dart'; import 'common/core.dart'; diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart index 2c27c91e04..654675ebb8 100644 --- a/script/tool/lib/src/pubspec_check_command.dart +++ b/script/tool/lib/src/pubspec_check_command.dart @@ -5,7 +5,6 @@ import 'package:file/file.dart'; import 'package:git/git.dart'; import 'package:platform/platform.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:yaml/yaml.dart'; import 'common/core.dart'; diff --git a/script/tool/lib/src/readme_check_command.dart b/script/tool/lib/src/readme_check_command.dart index 9432c4b7fa..0cb64920de 100644 --- a/script/tool/lib/src/readme_check_command.dart +++ b/script/tool/lib/src/readme_check_command.dart @@ -5,7 +5,6 @@ import 'package:file/file.dart'; import 'package:git/git.dart'; import 'package:platform/platform.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:yaml/yaml.dart'; import 'common/core.dart'; diff --git a/script/tool/lib/src/test_command.dart b/script/tool/lib/src/test_command.dart index 2c5dd9934b..d115b6557a 100644 --- a/script/tool/lib/src/test_command.dart +++ b/script/tool/lib/src/test_command.dart @@ -43,7 +43,7 @@ class TestCommand extends PackageLoopingCommand { } bool passed; - if (isFlutterPackage(package.directory)) { + if (package.requiresFlutter()) { passed = await _runFlutterTests(package); } else { passed = await _runDartTests(package); diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index fcaea33592..f69611f5a0 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -9,7 +9,6 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'common/core.dart'; import 'common/git_version_finder.dart'; diff --git a/script/tool/test/common/repository_package_test.dart b/script/tool/test/common/repository_package_test.dart index 0a8ea36eb9..2d0e11475c 100644 --- a/script/tool/test/common/repository_package_test.dart +++ b/script/tool/test/common/repository_package_test.dart @@ -5,7 +5,6 @@ import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common/repository_package.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:test/test.dart'; import '../util.dart'; @@ -97,7 +96,7 @@ void main() { }); group('getExamples', () { - test('handles a single example', () async { + test('handles a single Flutter example', () async { final Directory plugin = createFakePlugin('a_plugin', packagesDir); final List examples = @@ -107,7 +106,7 @@ void main() { expect(examples[0].path, plugin.childDirectory('example').path); }); - test('handles multiple examples', () async { + test('handles multiple Flutter examples', () async { final Directory plugin = createFakePlugin('a_plugin', packagesDir, examples: ['example1', 'example2']); @@ -120,6 +119,30 @@ void main() { expect(examples[1].path, plugin.childDirectory('example').childDirectory('example2').path); }); + + test('handles a single non-Flutter example', () async { + final Directory package = createFakePackage('a_package', packagesDir); + + final List examples = + RepositoryPackage(package).getExamples().toList(); + + expect(examples.length, 1); + expect(examples[0].path, package.childDirectory('example').path); + }); + + test('handles multiple non-Flutter examples', () async { + final Directory package = createFakePackage('a_package', packagesDir, + examples: ['example1', 'example2']); + + final List examples = + RepositoryPackage(package).getExamples().toList(); + + expect(examples.length, 2); + expect(examples[0].path, + package.childDirectory('example').childDirectory('example1').path); + expect(examples[1].path, + package.childDirectory('example').childDirectory('example2').path); + }); }); group('federated plugin queries', () { @@ -179,4 +202,18 @@ void main() { expect(pubspec.name, 'a_plugin'); }); }); + + group('requiresFlutter', () { + test('returns true for Flutter package', () async { + final Directory package = + createFakePackage('a_package', packagesDir, isFlutter: true); + expect(RepositoryPackage(package).requiresFlutter(), true); + }); + + test('returns false for non-Flutter package', () async { + final Directory package = + createFakePackage('a_package', packagesDir, isFlutter: false); + expect(RepositoryPackage(package).requiresFlutter(), false); + }); + }); } diff --git a/script/tool/test/create_all_plugins_app_command_test.dart b/script/tool/test/create_all_plugins_app_command_test.dart index 917adca020..9e2ee29326 100644 --- a/script/tool/test/create_all_plugins_app_command_test.dart +++ b/script/tool/test/create_all_plugins_app_command_test.dart @@ -10,7 +10,6 @@ import 'package:file/local.dart'; import 'package:flutter_plugin_tools/src/common/repository_package.dart'; import 'package:flutter_plugin_tools/src/create_all_plugins_app_command.dart'; import 'package:platform/platform.dart'; -import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:test/test.dart'; import 'util.dart'; diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index ac57eb7251..214efb4202 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -128,6 +128,7 @@ void main() { extraFiles: [ 'example/test_driver/integration_test.dart', 'example/integration_test/foo_test.dart', + 'example/ios/ios.m', ], platformSupport: { platformIOS: const PlatformDetails(PlatformSupport.inline), @@ -193,6 +194,8 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/android/android.java', + 'example/ios/ios.m', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -243,6 +246,8 @@ void main() { packagesDir, extraFiles: [ 'example/test_driver/plugin_test.dart', + 'example/android/android.java', + 'example/ios/ios.m', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -276,6 +281,8 @@ void main() { packagesDir, extraFiles: [ 'example/lib/main.dart', + 'example/android/android.java', + 'example/ios/ios.m', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -312,6 +319,8 @@ void main() { 'example/integration_test/bar_test.dart', 'example/integration_test/foo_test.dart', 'example/integration_test/ignore_me.dart', + 'example/android/android.java', + 'example/ios/ios.m', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -398,6 +407,7 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/linux/linux.cc', ], platformSupport: { platformLinux: const PlatformDetails(PlatformSupport.inline), @@ -542,6 +552,7 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/web/index.html', ], platformSupport: { platformWeb: const PlatformDetails(PlatformSupport.inline), @@ -591,6 +602,7 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/web/index.html', ], platformSupport: { platformWeb: const PlatformDetails(PlatformSupport.inline), @@ -668,6 +680,7 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/windows/windows.cpp', ], platformSupport: { platformWindows: const PlatformDetails(PlatformSupport.inline), @@ -715,6 +728,7 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/android/android.java', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -853,6 +867,8 @@ void main() { extraFiles: [ 'example/test_driver/plugin_test.dart', 'example/test_driver/plugin.dart', + 'example/android/android.java', + 'example/ios/ios.m', ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline), @@ -927,6 +943,7 @@ void main() { extraFiles: [ 'example/integration_test/bar_test.dart', 'example/integration_test/foo_test.dart', + 'example/web/index.html', ], platformSupport: { platformWeb: const PlatformDetails(PlatformSupport.inline), @@ -959,6 +976,7 @@ void main() { packagesDir, extraFiles: [ 'example/test_driver/integration_test.dart', + 'example/web/index.html', ], platformSupport: { platformWeb: const PlatformDetails(PlatformSupport.inline), @@ -995,6 +1013,7 @@ void main() { 'example/test_driver/integration_test.dart', 'example/integration_test/bar_test.dart', 'example/integration_test/foo_test.dart', + 'example/macos/macos.swift', ], platformSupport: { platformMacOS: const PlatformDetails(PlatformSupport.inline), @@ -1060,5 +1079,147 @@ void main() { pluginExampleDirectory.path), ])); }); + + group('packages', () { + test('can be driven', () async { + final Directory package = + createFakePackage('a_package', packagesDir, extraFiles: [ + 'example/integration_test/foo_test.dart', + 'example/test_driver/integration_test.dart', + 'example/web/index.html', + ]); + final Directory exampleDirectory = package.childDirectory('example'); + + final List output = await runCapturingPrint(runner, [ + 'drive-examples', + '--web', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for a_package'), + contains('No issues found!'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const [ + 'drive', + '-d', + 'web-server', + '--web-port=7357', + '--browser-name=chrome', + '--driver', + 'test_driver/integration_test.dart', + '--target', + 'integration_test/foo_test.dart' + ], + exampleDirectory.path), + ])); + }); + + test('are skipped when example does not support platform', () async { + createFakePackage('a_package', packagesDir, + isFlutter: true, + extraFiles: [ + 'example/integration_test/foo_test.dart', + 'example/test_driver/integration_test.dart', + ]); + + final List output = await runCapturingPrint(runner, [ + 'drive-examples', + '--web', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for a_package'), + contains('Skipping a_package/example; does not support any ' + 'requested platforms'), + contains('SKIPPING: No example supports requested platform(s).'), + ]), + ); + + expect(processRunner.recordedCalls.isEmpty, true); + }); + + test('drive only supported examples if there is more than one', () async { + final Directory package = createFakePackage('a_package', packagesDir, + isFlutter: true, + examples: [ + 'with_web', + 'without_web' + ], + extraFiles: [ + 'example/with_web/integration_test/foo_test.dart', + 'example/with_web/test_driver/integration_test.dart', + 'example/with_web/web/index.html', + 'example/without_web/integration_test/foo_test.dart', + 'example/without_web/test_driver/integration_test.dart', + ]); + final Directory supportedExampleDirectory = + package.childDirectory('example').childDirectory('with_web'); + + final List output = await runCapturingPrint(runner, [ + 'drive-examples', + '--web', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for a_package'), + contains( + 'Skipping a_package/example/without_web; does not support any requested platforms.'), + contains('No issues found!'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall( + getFlutterCommand(mockPlatform), + const [ + 'drive', + '-d', + 'web-server', + '--web-port=7357', + '--browser-name=chrome', + '--driver', + 'test_driver/integration_test.dart', + '--target', + 'integration_test/foo_test.dart' + ], + supportedExampleDirectory.path), + ])); + }); + + test('are skipped when there is no integration testing', () async { + createFakePackage('a_package', packagesDir, + isFlutter: true, extraFiles: ['example/web/index.html']); + + final List output = await runCapturingPrint(runner, [ + 'drive-examples', + '--web', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for a_package'), + contains('SKIPPING: No example is configured for driver tests.'), + ]), + ); + + expect(processRunner.recordedCalls.isEmpty, true); + }); + }); }); } diff --git a/script/tool/test/list_command_test.dart b/script/tool/test/list_command_test.dart index fcdf9fafdb..9e70f72e74 100644 --- a/script/tool/test/list_command_test.dart +++ b/script/tool/test/list_command_test.dart @@ -12,7 +12,7 @@ import 'mocks.dart'; import 'util.dart'; void main() { - group('$ListCommand', () { + group('ListCommand', () { late FileSystem fileSystem; late MockPlatform mockPlatform; late Directory packagesDir; diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart index 42d20240da..30b6ab6004 100644 --- a/script/tool/test/pubspec_check_command_test.dart +++ b/script/tool/test/pubspec_check_command_test.dart @@ -155,6 +155,21 @@ ${_flutterSection(isPlugin: true)} ${_dependenciesSection()} ${_devDependenciesSection()} ${_falseSecretsSection()} +'''); + + pluginDirectory + .childDirectory('example') + .childFile('pubspec.yaml') + .writeAsStringSync(''' +${_headerSection( + 'plugin_example', + publishable: false, + includeRepository: false, + includeIssueTracker: false, + )} +${_environmentSection()} +${_dependenciesSection()} +${_flutterSection()} '''); final List output = await runCapturingPrint(runner, [ @@ -172,15 +187,31 @@ ${_falseSecretsSection()} }); test('passes for a Flutter package following conventions', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory packageDirectory = + createFakePackage('a_package', packagesDir); - pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' -${_headerSection('plugin')} + packageDirectory.childFile('pubspec.yaml').writeAsStringSync(''' +${_headerSection('a_package')} ${_environmentSection()} ${_dependenciesSection()} ${_devDependenciesSection()} ${_flutterSection()} ${_falseSecretsSection()} +'''); + + packageDirectory + .childDirectory('example') + .childFile('pubspec.yaml') + .writeAsStringSync(''' +${_headerSection( + 'a_package', + publishable: false, + includeRepository: false, + includeIssueTracker: false, + )} +${_environmentSection()} +${_dependenciesSection()} +${_flutterSection()} '''); final List output = await runCapturingPrint(runner, [ @@ -190,8 +221,8 @@ ${_falseSecretsSection()} expect( output, containsAllInOrder([ - contains('Running for plugin...'), - contains('Running for plugin/example...'), + contains('Running for a_package...'), + contains('Running for a_package/example...'), contains('No issues found!'), ]), ); @@ -221,7 +252,8 @@ ${_dependenciesSection()} }); test('fails when homepage is included', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, includeHomepage: true)} @@ -248,7 +280,8 @@ ${_devDependenciesSection()} }); test('fails when repository is missing', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, includeRepository: false)} @@ -274,7 +307,8 @@ ${_devDependenciesSection()} }); test('fails when homepage is given instead of repository', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, includeHomepage: true, includeRepository: false)} @@ -301,7 +335,8 @@ ${_devDependenciesSection()} }); test('fails when repository is incorrect', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, repositoryPackagesDirRelativePath: 'different_plugin')} @@ -327,7 +362,8 @@ ${_devDependenciesSection()} }); test('fails when issue tracker is missing', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, includeIssueTracker: false)} @@ -353,8 +389,9 @@ ${_devDependenciesSection()} }); test('fails when description is too short', () async { - final Directory pluginDirectory = - createFakePlugin('a_plugin', packagesDir.childDirectory('a_plugin')); + final Directory pluginDirectory = createFakePlugin( + 'a_plugin', packagesDir.childDirectory('a_plugin'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, description: 'Too short')} @@ -383,7 +420,8 @@ ${_devDependenciesSection()} test( 'allows short descriptions for non-app-facing parts of federated plugins', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true, description: 'Too short')} @@ -410,7 +448,8 @@ ${_devDependenciesSection()} }); test('fails when description is too long', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); const String description = 'This description is too long. It just goes ' 'on and on and on and on and on. pub.dev will down-score it because ' @@ -442,7 +481,8 @@ ${_devDependenciesSection()} }); test('fails when environment section is out of order', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true)} @@ -469,7 +509,8 @@ ${_environmentSection()} }); test('fails when flutter section is out of order', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true)} @@ -496,7 +537,8 @@ ${_devDependenciesSection()} }); test('fails when dependencies section is out of order', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true)} @@ -550,7 +592,8 @@ ${_dependenciesSection()} }); test('fails when false_secrets section is out of order', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin', isPlugin: true)} @@ -580,7 +623,8 @@ ${_devDependenciesSection()} test('fails when an implemenation package is missing "implements"', () async { final Directory pluginDirectory = createFakePlugin( - 'plugin_a_foo', packagesDir.childDirectory('plugin_a')); + 'plugin_a_foo', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin_a_foo', isPlugin: true)} @@ -608,7 +652,8 @@ ${_devDependenciesSection()} test('fails when an implemenation package has the wrong "implements"', () async { final Directory pluginDirectory = createFakePlugin( - 'plugin_a_foo', packagesDir.childDirectory('plugin_a')); + 'plugin_a_foo', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('plugin_a_foo', isPlugin: true)} @@ -636,7 +681,8 @@ ${_devDependenciesSection()} test('passes for a correct implemenation package', () async { final Directory pluginDirectory = createFakePlugin( - 'plugin_a_foo', packagesDir.childDirectory('plugin_a')); + 'plugin_a_foo', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection( @@ -663,8 +709,9 @@ ${_devDependenciesSection()} }); test('fails when a "default_package" looks incorrect', () async { - final Directory pluginDirectory = - createFakePlugin('plugin_a', packagesDir.childDirectory('plugin_a')); + final Directory pluginDirectory = createFakePlugin( + 'plugin_a', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection( @@ -702,8 +749,9 @@ ${_devDependenciesSection()} test( 'fails when a "default_package" does not have a corresponding dependency', () async { - final Directory pluginDirectory = - createFakePlugin('plugin_a', packagesDir.childDirectory('plugin_a')); + final Directory pluginDirectory = createFakePlugin( + 'plugin_a', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection( @@ -739,8 +787,9 @@ ${_devDependenciesSection()} }); test('passes for an app-facing package without "implements"', () async { - final Directory pluginDirectory = - createFakePlugin('plugin_a', packagesDir.childDirectory('plugin_a')); + final Directory pluginDirectory = createFakePlugin( + 'plugin_a', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection( @@ -769,8 +818,8 @@ ${_devDependenciesSection()} test('passes for a platform interface package without "implements"', () async { final Directory pluginDirectory = createFakePlugin( - 'plugin_a_platform_interface', - packagesDir.childDirectory('plugin_a')); + 'plugin_a_platform_interface', packagesDir.childDirectory('plugin_a'), + examples: []); pluginDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection( @@ -799,7 +848,8 @@ ${_devDependenciesSection()} test('validates some properties even for unpublished packages', () async { final Directory pluginDirectory = createFakePlugin( - 'plugin_a_foo', packagesDir.childDirectory('plugin_a')); + 'plugin_a_foo', packagesDir.childDirectory('plugin_a'), + examples: []); // Environment section is in the wrong location. // Missing 'implements'. @@ -829,7 +879,8 @@ ${_environmentSection()} }); test('ignores some checks for unpublished packages', () async { - final Directory pluginDirectory = createFakePlugin('plugin', packagesDir); + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, examples: []); // Missing metadata that is only useful for published packages, such as // repository and issue tracker. @@ -886,7 +937,7 @@ ${_devDependenciesSection()} test('repository check works', () async { final Directory packageDirectory = - createFakePackage('package', packagesDir); + createFakePackage('package', packagesDir, examples: []); packageDirectory.childFile('pubspec.yaml').writeAsStringSync(''' ${_headerSection('package')} diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 2b1719ee80..8a2bf099cc 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -107,6 +107,12 @@ Directory createFakePlugin( /// /// [extraFiles] is an optional list of package-relative paths, using unix-style /// separators, of extra files to create in the package. +/// +/// If [includeCommonFiles] is true, common but non-critical files like +/// CHANGELOG.md and AUTHORS will be included. +/// +/// If non-null, [directoryName] will be used for the directory instead of +/// [name]. // TODO(stuartmorgan): Convert the return to a RepositoryPackage. Directory createFakePackage( String name, @@ -116,37 +122,43 @@ Directory createFakePackage( bool isFlutter = false, String? version = '0.0.1', String flutterConstraint = '>=2.5.0', + bool includeCommonFiles = true, + String? directoryName, + String? publishTo, }) { - final Directory packageDirectory = parentDirectory.childDirectory(name); + final Directory packageDirectory = + parentDirectory.childDirectory(directoryName ?? name); packageDirectory.createSync(recursive: true); + packageDirectory.childDirectory('lib').createSync(); createFakePubspec(packageDirectory, name: name, isFlutter: isFlutter, version: version, flutterConstraint: flutterConstraint); - createFakeCHANGELOG(packageDirectory, ''' + if (includeCommonFiles) { + createFakeCHANGELOG(packageDirectory, ''' ## $version * Some changes. '''); - createFakeAuthors(packageDirectory); + createFakeAuthors(packageDirectory); + } if (examples.length == 1) { - final Directory exampleDir = packageDirectory.childDirectory(examples.first) - ..createSync(); - createFakePubspec(exampleDir, - name: '${name}_example', + createFakePackage('${name}_example', packageDirectory, + directoryName: examples.first, + examples: [], + includeCommonFiles: false, isFlutter: isFlutter, publishTo: 'none', flutterConstraint: flutterConstraint); } else if (examples.isNotEmpty) { - final Directory exampleDir = packageDirectory.childDirectory('example') - ..createSync(); - for (final String example in examples) { - final Directory currentExample = exampleDir.childDirectory(example) - ..createSync(); - createFakePubspec(currentExample, - name: example, + final Directory examplesDirectory = + packageDirectory.childDirectory('example')..createSync(); + for (final String exampleName in examples) { + createFakePackage(exampleName, examplesDirectory, + examples: [], + includeCommonFiles: false, isFlutter: isFlutter, publishTo: 'none', flutterConstraint: flutterConstraint); @@ -179,7 +191,7 @@ void createFakePubspec( bool isPlugin = false, Map platformSupport = const {}, - String publishTo = 'http://no_pub_server.com', + String? publishTo, String? version, String dartConstraint = '>=2.0.0 <3.0.0', String flutterConstraint = '>=2.5.0', @@ -219,9 +231,16 @@ flutter: } } - String yaml = ''' + // Default to a fake server to avoid ever accidentally publishing something + // from a test. Does not use 'none' since that changes the behavior of some + // commands. + final String publishToSection = + 'publish_to: ${publishTo ?? 'http://no_pub_server.com'}'; + + final String yaml = ''' name: $name ${(version != null) ? 'version: $version' : ''} +$publishToSection $environmentSection @@ -230,11 +249,6 @@ $dependenciesSection $pluginSection '''; - if (publishTo.isNotEmpty) { - yaml += ''' -publish_to: $publishTo # Hardcoded safeguard to prevent this from somehow being published by a broken test. -'''; - } parent.childFile('pubspec.yaml').createSync(); parent.childFile('pubspec.yaml').writeAsStringSync(yaml); }