[ci] Add more dev dependency checks, and fix errors (#6563)

- Adds `build_runner`, `mockito`, and `pigeon` to the list of dependencies that we expect to only be in dev_dependencies.
- Fixes the error message for violations; I had missed in initial review that the errorr message was the same as the one for non-repo-local depndencies, making it misleading about what the problem was and how to fix it. (Also fixed the indentation, which had always been wrong.)
- Fixes `camera_avfoundation`, which had a violation introduced in my recent PR to start the Pigeon conversion.

See https://github.com/flutter/flutter/issues/117905
This commit is contained in:
stuartmorgan
2024-04-18 17:43:04 -04:00
committed by GitHub
parent b0e6d7f3b8
commit bb82cb79b9
4 changed files with 92 additions and 54 deletions

View File

@ -1,3 +1,7 @@
## 0.9.15+3
* Moves `pigeon` to `dev_dependencies`.
## 0.9.15+2 ## 0.9.15+2
* Converts camera query to Pigeon. * Converts camera query to Pigeon.

View File

@ -2,7 +2,7 @@ name: camera_avfoundation
description: iOS implementation of the camera plugin. description: iOS implementation of the camera plugin.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.9.15+2 version: 0.9.15+3
environment: environment:
sdk: ^3.2.3 sdk: ^3.2.3
@ -20,7 +20,6 @@ dependencies:
camera_platform_interface: ^2.7.0 camera_platform_interface: ^2.7.0
flutter: flutter:
sdk: flutter sdk: flutter
pigeon: ^18.0.0
stream_transform: ^2.0.0 stream_transform: ^2.0.0
dev_dependencies: dev_dependencies:
@ -29,6 +28,7 @@ dev_dependencies:
flutter_test: flutter_test:
sdk: flutter sdk: flutter
mockito: 5.4.4 mockito: 5.4.4
pigeon: ^18.0.0
topics: topics:
- camera - camera

View File

@ -200,7 +200,10 @@ class PubspecCheckCommand extends PackageLoopingCommand {
final String? dependenciesError = _checkDependencies(pubspec); final String? dependenciesError = _checkDependencies(pubspec);
if (dependenciesError != null) { if (dependenciesError != null) {
printError('$indentation$dependenciesError'); printError(dependenciesError
.split('\n')
.map((String line) => '$indentation$line')
.join('\n'));
passing = false; passing = false;
} }
@ -542,6 +545,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
// there are any that aren't allowed. // there are any that aren't allowed.
String? _checkDependencies(Pubspec pubspec) { String? _checkDependencies(Pubspec pubspec) {
final Set<String> badDependencies = <String>{}; final Set<String> badDependencies = <String>{};
final Set<String> misplacedDevDependencies = <String>{};
// Shipped dependencies. // Shipped dependencies.
for (final Map<String, Dependency> dependencies for (final Map<String, Dependency> dependencies
in <Map<String, Dependency>>[ in <Map<String, Dependency>>[
@ -556,24 +560,40 @@ class PubspecCheckCommand extends PackageLoopingCommand {
} }
// Ensure that dev-only dependencies aren't in `dependencies`. // Ensure that dev-only dependencies aren't in `dependencies`.
const Set<String> devOnlyDependencies = <String>{'integration_test', 'test', 'flutter_test'}; const Set<String> devOnlyDependencies = <String>{
// Non-published packages like pidgeon subpackages are allowed to violate 'build_runner',
'integration_test',
'flutter_test',
'mockito',
'pigeon',
'test',
};
// Non-published packages like pigeon subpackages are allowed to violate
// the dev only dependencies rule. // the dev only dependencies rule.
if (pubspec.publishTo != 'none') { if (pubspec.publishTo != 'none') {
pubspec.dependencies.forEach((String name, Dependency dependency) { pubspec.dependencies.forEach((String name, Dependency dependency) {
if (devOnlyDependencies.contains(name)) { if (devOnlyDependencies.contains(name)) {
badDependencies.add(name); misplacedDevDependencies.add(name);
} }
}); });
} }
if (badDependencies.isEmpty) { final List<String> errors = <String>[
return null; if (badDependencies.isNotEmpty)
} '''
return 'The following unexpected non-local dependencies were found:\n' The following unexpected non-local dependencies were found:
'${badDependencies.map((String name) => ' $name').join('\n')}\n' ${badDependencies.map((String name) => ' $name').join('\n')}
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies ' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies
'for more information and next steps.'; for more information and next steps.
''',
if (misplacedDevDependencies.isNotEmpty)
'''
The following dev dependencies were found in the dependencies section:
${misplacedDevDependencies.map((String name) => ' $name').join('\n')}
Please move them to dev_dependencies.
''',
];
return errors.isEmpty ? null : errors.join('\n\n');
} }
// Checks whether a given dependency is allowed. // Checks whether a given dependency is allowed.

View File

@ -1611,10 +1611,10 @@ ${_topicsSection()}
output, output,
containsAllInOrder(<Matcher>[ containsAllInOrder(<Matcher>[
contains( contains(
'The following unexpected non-local dependencies were found:\n' ' The following unexpected non-local dependencies were found:\n'
' bad_dependency\n' ' bad_dependency\n'
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies ' ' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
'for more information and next steps.'), ' for more information and next steps.'),
]), ]),
); );
}); });
@ -1643,10 +1643,10 @@ ${_topicsSection()}
output, output,
containsAllInOrder(<Matcher>[ containsAllInOrder(<Matcher>[
contains( contains(
'The following unexpected non-local dependencies were found:\n' ' The following unexpected non-local dependencies were found:\n'
' bad_dependency\n' ' bad_dependency\n'
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies ' ' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
'for more information and next steps.'), ' for more information and next steps.'),
]), ]),
); );
}); });
@ -1727,33 +1727,47 @@ ${_topicsSection()}
output, output,
containsAllInOrder(<Matcher>[ containsAllInOrder(<Matcher>[
contains( contains(
'The following unexpected non-local dependencies were found:\n' ' The following unexpected non-local dependencies were found:\n'
' allow_pinned\n' ' allow_pinned\n'
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies ' ' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
'for more information and next steps.'), ' for more information and next steps.'),
]), ]),
); );
}); });
test('fails when integration_test, flutter_test or test are used in non dev dependency', group('dev dependencies', () {
const List<String> packages = <String>[
'build_runner',
'integration_test',
'flutter_test',
'mockito',
'pigeon',
'test',
];
for (final String dependency in packages) {
test('fails when $dependency is used in non dev dependency',
() async { () async {
final RepositoryPackage package = final RepositoryPackage package = createFakePackage(
createFakePackage('a_package', packagesDir, examples: <String>[]); 'a_package', packagesDir,
examples: <String>[]);
final String version =
dependency == 'integration_test' || dependency == 'flutter_test'
? '{ sdk: flutter }'
: '1.0.0';
package.pubspecFile.writeAsStringSync(''' package.pubspecFile.writeAsStringSync('''
${_headerSection('a_package')} ${_headerSection('a_package')}
${_environmentSection()} ${_environmentSection()}
${_dependenciesSection(<String>[ ${_dependenciesSection(<String>[
'integration_test: \n sdk: flutter', '$dependency: $version',
'flutter_test: \n sdk: flutter',
'test: 1.0.0'
])} ])}
${_devDependenciesSection()} ${_devDependenciesSection()}
${_topicsSection()} ${_topicsSection()}
'''); ''');
Error? commandError; Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[ final List<String> output =
await runCapturingPrint(runner, <String>[
'pubspec-check', 'pubspec-check',
], errorHandler: (Error e) { ], errorHandler: (Error e) {
commandError = e; commandError = e;
@ -1764,17 +1778,17 @@ ${_topicsSection()}
output, output,
containsAllInOrder(<Matcher>[ containsAllInOrder(<Matcher>[
contains( contains(
'The following unexpected non-local dependencies were found:\n' ' The following dev dependencies were found in the dependencies section:\n'
' test\n' ' $dependency\n'
' integration_test\n' ' Please move them to dev_dependencies.'),
' flutter_test\n'
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
'for more information and next steps.'),
]), ]),
); );
}); });
}
});
test('passes when integration_test or flutter_test are used in non published package', test(
'passes when integration_test or flutter_test are used in non published package',
() async { () async {
final RepositoryPackage package = final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]); createFakePackage('a_package', packagesDir, examples: <String>[]);