Skip to content

[tool] Run pre_publish.dart before publish --dry-run #7258

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 31, 2024
Merged

[tool] Run pre_publish.dart before publish --dry-run #7258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions script/tool/lib/src/publish_check_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,26 @@ class PublishCheckCommand extends PackageLoopingCommand {

@override
Future<PackageResult> runForPackage(RepositoryPackage package) async {
_PublishCheckResult? result = await _passesPublishCheck(package);
if (result == null) {
if (_isMarkedAsUnpublishable(package)) {
return PackageResult.skip('Package is marked as unpublishable.');
}

// The pre-publish hook must be run first if it exists, since passing the
// publish check may rely on its execution.
final bool prePublishHookPassesOrNoops =
await _validatePrePublishHook(package);
// Given that, don't run publish check if the pre-publish hook fails.
_PublishCheckResult result = prePublishHookPassesOrNoops
? await _passesPublishCheck(package)
: _PublishCheckResult.error;

// But do continue with other checks to find all problems at once.
if (!_passesAuthorsCheck(package)) {
_printImportantStatusMessage(
'No AUTHORS file found. Packages must include an AUTHORS file.',
isError: true);
result = _PublishCheckResult.error;
}
if (!await _validatePrePublishHook(package)) {
result = _PublishCheckResult.error;
}

if (result.index > _overallResult.index) {
_overallResult = result;
Expand Down Expand Up @@ -200,17 +207,22 @@ class PublishCheckCommand extends PackageLoopingCommand {
'Packages with an SDK constraint on a pre-release of the Dart SDK should themselves be published as a pre-release version.');
}

/// Returns true if the package has been explicitly marked as not for
/// publishing.
bool _isMarkedAsUnpublishable(RepositoryPackage package) {
final Pubspec? pubspec = _tryParsePubspec(package);
return pubspec?.publishTo == 'none';
}

/// Returns the result of the publish check, or null if the package is marked
/// as unpublishable.
Future<_PublishCheckResult?> _passesPublishCheck(
Future<_PublishCheckResult> _passesPublishCheck(
RepositoryPackage package) async {
final String packageName = package.directory.basename;
final Pubspec? pubspec = _tryParsePubspec(package);
if (pubspec == null) {
print('No valid pubspec found.');
return _PublishCheckResult.error;
} else if (pubspec.publishTo == 'none') {
return null;
}

final Version? version = pubspec.version;
Expand Down
53 changes: 53 additions & 0 deletions script/tool/test/publish_check_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,22 @@ void main() {
));
});

test('skips packages that are marked as not for publishing', () async {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked to make sure we had a test for this since I was refactoring the logic that handles this behavior, and we didn't, thus the extra test.

createFakePackage('a_package', packagesDir,
version: '0.1.0', publishTo: 'none');

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

expect(
output,
containsAllInOrder(<Matcher>[
contains('SKIPPING: Package is marked as unpublishable.'),
]),
);
expect(processRunner.recordedCalls, isEmpty);
});

test(
'runs validation even for packages that are already published and reports success',
() async {
Expand Down Expand Up @@ -405,6 +421,43 @@ void main() {
]));
});

test('runs before publish --dry-run', () async {
final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]);
package.prePublishScript.createSync(recursive: true);

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

expect(
output,
containsAllInOrder(<Matcher>[
contains('Running pre-publish hook tool/pre_publish.dart...'),
]),
);
expect(
processRunner.recordedCalls,
containsAllInOrder(<ProcessCall>[
ProcessCall(
'dart',
const <String>[
'run',
'tool/pre_publish.dart',
],
package.directory.path),
ProcessCall(
'flutter',
const <String>[
'pub',
'publish',
'--',
'--dry-run',
],
package.directory.path),
]));
});

test('causes command failure if it fails', () async {
final RepositoryPackage package = createFakePackage(
'a_package', packagesDir,
Expand Down