Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tool] Add ability to check dependencies independently of dev-dependencies, exclude integration_test from dependencies #6446

Merged
merged 9 commits into from
Apr 3, 2024
17 changes: 16 additions & 1 deletion script/tool/lib/src/pubspec_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
'a topic. Add "$topicName" to the "topics" section.';
}
}

// Validates topic names according to https://dart.dev/tools/pub/pubspec#topics
final RegExp expectedTopicFormat = RegExp(r'^[a-z](?:-?[a-z0-9]+)*$');
final Iterable<String> invalidTopics = topics.where((String topic) =>
Expand Down Expand Up @@ -542,6 +542,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
// there are any that aren't allowed.
String? _checkDependencies(Pubspec pubspec) {
final Set<String> badDependencies = <String>{};
// Shipped dependencies.
for (final Map<String, Dependency> dependencies
in <Map<String, Dependency>>[
pubspec.dependencies,
Expand All @@ -553,6 +554,19 @@ class PubspecCheckCommand extends PackageLoopingCommand {
}
});
}

// Ensure that dev-only dependencies aren't in `dependencies`.
const List<String> devOnlyDependencies = <String>['integration_test'];
// Non-published packages like pidgeon subpackages are allowed to violate
// the dev only dependencies rule.
if (pubspec.publishTo != 'none') {
pubspec.dependencies.forEach((String name, Dependency dependency) {
if (devOnlyDependencies.contains(name)) {
badDependencies.add(name);
}
});
}

stuartmorgan marked this conversation as resolved.
Show resolved Hide resolved
if (badDependencies.isEmpty) {
return null;
}
Expand All @@ -563,6 +577,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
}

// Checks whether a given dependency is allowed.
// Defaults to false.
bool _shouldAllowDependency(String name, Dependency dependency) {
if (dependency is PathDependency || dependency is SdkDependency) {
return true;
Expand Down
59 changes: 59 additions & 0 deletions script/tool/test/pubspec_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,65 @@ ${_topicsSection()}
]),
);
});

test('fails when integration_test is used in non dev dependency',
() async {
final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]);

package.pubspecFile.writeAsStringSync('''
${_headerSection('a_package')}
${_environmentSection()}
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
${_devDependenciesSection()}
${_topicsSection()}
''');

Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
'pubspec-check',
], errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains(
'The following unexpected non-local dependencies were found:\n'
' integration_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 is used in non published package',
() async {
final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]);

package.pubspecFile.writeAsStringSync('''
${_headerSection('a_package', publishable: false)}
${_environmentSection()}
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
${_devDependenciesSection()}
${_topicsSection()}
''');

final List<String> output = await runCapturingPrint(runner, <String>[
'pubspec-check',
]);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Running for a_package...'),
contains('Ran for'),
]),
);
});
});
});

Expand Down