Skip to content

Commit bad79d1

Browse files
stuartmorgan-gTecHaxter
stuartmorgan-g
authored and
TecHaxter
committed
[ci] Add more dev dependency checks, and fix errors (flutter#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 flutter/flutter#117905
1 parent 3cff2e1 commit bad79d1

File tree

4 files changed

+94
-56
lines changed

4 files changed

+94
-56
lines changed

‎packages/camera/camera_avfoundation/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.9.15+3
2+
3+
* Moves `pigeon` to `dev_dependencies`.
4+
15
## 0.9.15+2
26

37
* Converts camera query to Pigeon.

‎packages/camera/camera_avfoundation/pubspec.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: camera_avfoundation
22
description: iOS implementation of the camera plugin.
33
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
5-
version: 0.9.15+2
5+
version: 0.9.15+3
66

77
environment:
88
sdk: ^3.2.3
@@ -20,7 +20,6 @@ dependencies:
2020
camera_platform_interface: ^2.7.0
2121
flutter:
2222
sdk: flutter
23-
pigeon: ^18.0.0
2423
stream_transform: ^2.0.0
2524

2625
dev_dependencies:
@@ -29,6 +28,7 @@ dev_dependencies:
2928
flutter_test:
3029
sdk: flutter
3130
mockito: 5.4.4
31+
pigeon: ^18.0.0
3232

3333
topics:
3434
- camera

‎script/tool/lib/src/pubspec_check_command.dart

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,10 @@ class PubspecCheckCommand extends PackageLoopingCommand {
200200

201201
final String? dependenciesError = _checkDependencies(pubspec);
202202
if (dependenciesError != null) {
203-
printError('$indentation$dependenciesError');
203+
printError(dependenciesError
204+
.split('\n')
205+
.map((String line) => '$indentation$line')
206+
.join('\n'));
204207
passing = false;
205208
}
206209

@@ -542,6 +545,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
542545
// there are any that aren't allowed.
543546
String? _checkDependencies(Pubspec pubspec) {
544547
final Set<String> badDependencies = <String>{};
548+
final Set<String> misplacedDevDependencies = <String>{};
545549
// Shipped dependencies.
546550
for (final Map<String, Dependency> dependencies
547551
in <Map<String, Dependency>>[
@@ -556,24 +560,40 @@ class PubspecCheckCommand extends PackageLoopingCommand {
556560
}
557561

558562
// Ensure that dev-only dependencies aren't in `dependencies`.
559-
const Set<String> devOnlyDependencies = <String>{'integration_test', 'test', 'flutter_test'};
560-
// Non-published packages like pidgeon subpackages are allowed to violate
563+
const Set<String> devOnlyDependencies = <String>{
564+
'build_runner',
565+
'integration_test',
566+
'flutter_test',
567+
'mockito',
568+
'pigeon',
569+
'test',
570+
};
571+
// Non-published packages like pigeon subpackages are allowed to violate
561572
// the dev only dependencies rule.
562573
if (pubspec.publishTo != 'none') {
563574
pubspec.dependencies.forEach((String name, Dependency dependency) {
564575
if (devOnlyDependencies.contains(name)) {
565-
badDependencies.add(name);
576+
misplacedDevDependencies.add(name);
566577
}
567578
});
568579
}
569580

570-
if (badDependencies.isEmpty) {
571-
return null;
572-
}
573-
return 'The following unexpected non-local dependencies were found:\n'
574-
'${badDependencies.map((String name) => ' $name').join('\n')}\n'
575-
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
576-
'for more information and next steps.';
581+
final List<String> errors = <String>[
582+
if (badDependencies.isNotEmpty)
583+
'''
584+
The following unexpected non-local dependencies were found:
585+
${badDependencies.map((String name) => ' $name').join('\n')}
586+
Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies
587+
for more information and next steps.
588+
''',
589+
if (misplacedDevDependencies.isNotEmpty)
590+
'''
591+
The following dev dependencies were found in the dependencies section:
592+
${misplacedDevDependencies.map((String name) => ' $name').join('\n')}
593+
Please move them to dev_dependencies.
594+
''',
595+
];
596+
return errors.isEmpty ? null : errors.join('\n\n');
577597
}
578598

579599
// Checks whether a given dependency is allowed.

‎script/tool/test/pubspec_check_command_test.dart

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,10 +1611,10 @@ ${_topicsSection()}
16111611
output,
16121612
containsAllInOrder(<Matcher>[
16131613
contains(
1614-
'The following unexpected non-local dependencies were found:\n'
1615-
' bad_dependency\n'
1616-
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
1617-
'for more information and next steps.'),
1614+
' The following unexpected non-local dependencies were found:\n'
1615+
' bad_dependency\n'
1616+
' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
1617+
' for more information and next steps.'),
16181618
]),
16191619
);
16201620
});
@@ -1643,10 +1643,10 @@ ${_topicsSection()}
16431643
output,
16441644
containsAllInOrder(<Matcher>[
16451645
contains(
1646-
'The following unexpected non-local dependencies were found:\n'
1647-
' bad_dependency\n'
1648-
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
1649-
'for more information and next steps.'),
1646+
' The following unexpected non-local dependencies were found:\n'
1647+
' bad_dependency\n'
1648+
' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
1649+
' for more information and next steps.'),
16501650
]),
16511651
);
16521652
});
@@ -1727,54 +1727,68 @@ ${_topicsSection()}
17271727
output,
17281728
containsAllInOrder(<Matcher>[
17291729
contains(
1730-
'The following unexpected non-local dependencies were found:\n'
1731-
' allow_pinned\n'
1732-
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
1733-
'for more information and next steps.'),
1730+
' The following unexpected non-local dependencies were found:\n'
1731+
' allow_pinned\n'
1732+
' Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies\n'
1733+
' for more information and next steps.'),
17341734
]),
17351735
);
17361736
});
17371737

1738-
test('fails when integration_test, flutter_test or test are used in non dev dependency',
1739-
() async {
1740-
final RepositoryPackage package =
1741-
createFakePackage('a_package', packagesDir, examples: <String>[]);
1742-
1743-
package.pubspecFile.writeAsStringSync('''
1738+
group('dev dependencies', () {
1739+
const List<String> packages = <String>[
1740+
'build_runner',
1741+
'integration_test',
1742+
'flutter_test',
1743+
'mockito',
1744+
'pigeon',
1745+
'test',
1746+
];
1747+
for (final String dependency in packages) {
1748+
test('fails when $dependency is used in non dev dependency',
1749+
() async {
1750+
final RepositoryPackage package = createFakePackage(
1751+
'a_package', packagesDir,
1752+
examples: <String>[]);
1753+
1754+
final String version =
1755+
dependency == 'integration_test' || dependency == 'flutter_test'
1756+
? '{ sdk: flutter }'
1757+
: '1.0.0';
1758+
package.pubspecFile.writeAsStringSync('''
17441759
${_headerSection('a_package')}
17451760
${_environmentSection()}
17461761
${_dependenciesSection(<String>[
1747-
'integration_test: \n sdk: flutter',
1748-
'flutter_test: \n sdk: flutter',
1749-
'test: 1.0.0'
1750-
])}
1762+
'$dependency: $version',
1763+
])}
17511764
${_devDependenciesSection()}
17521765
${_topicsSection()}
17531766
''');
17541767

1755-
Error? commandError;
1756-
final List<String> output = await runCapturingPrint(runner, <String>[
1757-
'pubspec-check',
1758-
], errorHandler: (Error e) {
1759-
commandError = e;
1760-
});
1761-
1762-
expect(commandError, isA<ToolExit>());
1763-
expect(
1764-
output,
1765-
containsAllInOrder(<Matcher>[
1766-
contains(
1767-
'The following unexpected non-local dependencies were found:\n'
1768-
' test\n'
1769-
' integration_test\n'
1770-
' flutter_test\n'
1771-
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
1772-
'for more information and next steps.'),
1773-
]),
1774-
);
1768+
Error? commandError;
1769+
final List<String> output =
1770+
await runCapturingPrint(runner, <String>[
1771+
'pubspec-check',
1772+
], errorHandler: (Error e) {
1773+
commandError = e;
1774+
});
1775+
1776+
expect(commandError, isA<ToolExit>());
1777+
expect(
1778+
output,
1779+
containsAllInOrder(<Matcher>[
1780+
contains(
1781+
' The following dev dependencies were found in the dependencies section:\n'
1782+
' $dependency\n'
1783+
' Please move them to dev_dependencies.'),
1784+
]),
1785+
);
1786+
});
1787+
}
17751788
});
17761789

1777-
test('passes when integration_test or flutter_test are used in non published package',
1790+
test(
1791+
'passes when integration_test or flutter_test are used in non published package',
17781792
() async {
17791793
final RepositoryPackage package =
17801794
createFakePackage('a_package', packagesDir, examples: <String>[]);

0 commit comments

Comments
 (0)