Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[tool] version-check publish-check commands can check against pub #3840

Merged
merged 18 commits into from
May 11, 2021
1 change: 1 addition & 0 deletions script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## NEXT

- Add `against-pub` flag for version-check.
- Add `log-status` flag for publish-check.
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


## 0.1.0+1

Expand Down
90 changes: 79 additions & 11 deletions script/tool/lib/src/publish_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import 'dart:io' as io;

import 'package:colorize/colorize.dart';
import 'package:file/file.dart';
import 'package:http/http.dart' as http;
import 'package:pub_semver/pub_semver.dart';
import 'package:pubspec_parse/pubspec_parse.dart';

import 'common.dart';
Expand All @@ -18,6 +20,7 @@ class PublishCheckCommand extends PluginCommand {
Directory packagesDir,
FileSystem fileSystem, {
ProcessRunner processRunner = const ProcessRunner(),
this.httpClient,
}) : super(packagesDir, fileSystem, processRunner: processRunner) {
argParser.addFlag(
_allowPrereleaseFlag,
Expand All @@ -26,9 +29,22 @@ class PublishCheckCommand extends PluginCommand {
'the SDK constraint is a pre-release version, is ignored.',
defaultsTo: false,
);
argParser.addFlag(_logStatusFlag,
help:
'Logs the check-publish final status to a defined string as the last line of the command output.\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixing output for humans and output for machines is generally not a good idea; I would rather we make a --machine flag that switches entirely to an output meant for parsing.

'The possible values are:\n'
' $_resultNeedsPublish: There is at least one package need to be published. They also passed all publish checks. \n'
Copy link
Contributor

Choose a reason for hiding this comment

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

s/need to/that needs to be/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove training space since there's a \n

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

' $_resultNoPublish: There are no packages need to be published. Either no pubspec change detected or the version has already been published. \n'
Copy link
Contributor

Choose a reason for hiding this comment

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

s/the version/all versions/, since there can be multiple packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

' $_resultError: Some error has occurred.',
defaultsTo: false,
negatable: true);
}

static const String _allowPrereleaseFlag = 'allow-pre-release';
static const String _logStatusFlag = 'log-status';
static const String _resultNeedsPublish = 'needs-publish';
static const String _resultNoPublish = 'no-publish';
static const String _resultError = 'error';

@override
final String name = 'publish-check';
Expand All @@ -37,13 +53,28 @@ class PublishCheckCommand extends PluginCommand {
final String description =
'Checks to make sure that a plugin *could* be published.';

/// The custom http client used to query versions on pub.
final http.Client httpClient;

@override
Future<void> run() async {
final List<Directory> failedPackages = <Directory>[];

String resultToLog = _resultNoPublish;
await for (final Directory plugin in getPlugins()) {
if (!(await _passesPublishCheck(plugin))) {
failedPackages.add(plugin);
final _PublishCheckResult result = await _passesPublishCheck(plugin);
switch (result) {
case _PublishCheckResult._needsPublish:
if (failedPackages.isEmpty) {
resultToLog = _resultNeedsPublish;
}
break;
case _PublishCheckResult._noPublish:
break;
case _PublishCheckResult.error:
failedPackages.add(plugin);
resultToLog = _resultError;
break;
}
}

Expand All @@ -56,12 +87,19 @@ class PublishCheckCommand extends PluginCommand {
final Colorize colorizedError = Colorize('$error\n$joinedFailedPackages')
..red();
print(colorizedError);
throw ToolExit(1);
} else {
final Colorize passedMessage =
Colorize('All packages passed publish check!')..green();
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the new tests, it's very strange to have terminal color commands in the --machine output. I think we should replace all the Colorize calls with a utility method like:

_printImportantStatusMessage(String message, {@required bool isError}) {
  if (argResults[_machineFlag] as bool) {
    print('${isError ? 'ERROR' : 'SUCCESS'}: $message');
  } else {
    final Colorize colorizedMessage = Colorize(message);
    if (isError) {
      colorizedMessage.red();
    } else {
      colorizedMessage.green();
    }
    print(colorizedMessage);
  }
}

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 updated so that both machine mesasge and human message include the error and success prefix.

print(passedMessage);
}

final Colorize passedMessage =
Colorize('All packages passed publish check!')..green();
print(passedMessage);
// This has to be last output of this command because it is promised in the help section.
Copy link
Contributor

Choose a reason for hiding this comment

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

Obsolete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (argResults[_logStatusFlag] as bool) {
print(resultToLog);
}
if (failedPackages.isNotEmpty) {
throw ToolExit(1);
}
}

Pubspec _tryParsePubspec(Directory package) {
Expand Down Expand Up @@ -121,24 +159,54 @@ class PublishCheckCommand extends PluginCommand {
'Packages with an SDK constraint on a pre-release of the Dart SDK should themselves be published as a pre-release version.');
}

Future<bool> _passesPublishCheck(Directory package) async {
Future<_PublishCheckResult> _passesPublishCheck(Directory package) async {
final String packageName = package.basename;
print('Checking that $packageName can be published.');

final Pubspec pubspec = _tryParsePubspec(package);
if (pubspec == null) {
return false;
print('no pubspec');
return _PublishCheckResult.error;
} else if (pubspec.publishTo == 'none') {
print('Package $packageName is marked as unpublishable. Skipping.');
return true;
return _PublishCheckResult._noPublish;
}

final Version version = pubspec.version;
final bool alreadyPublished = await _checkIfAlreadyPublished(
packageName: packageName, version: version);
if (alreadyPublished) {
print(
'Package $packageName version: $version has already be published on pub.');
return _PublishCheckResult._noPublish;
}

if (await _hasValidPublishCheckRun(package)) {
print('Package $packageName is able to be published.');
return true;
return _PublishCheckResult._needsPublish;
} else {
print('Unable to publish $packageName');
return false;
return _PublishCheckResult.error;
}
}

// Check if `packageName` already has `version` published on pub.
Future<bool> _checkIfAlreadyPublished(
{String packageName, Version version}) async {
final PubVersionFinder pubVersionFinder = PubVersionFinder(
package: packageName, httpClient: httpClient ?? http.Client());
final PubVersionFinderResponse pubVersionFinderResponse =
await pubVersionFinder.getPackageVersion();
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we want a try/catch here to deal with issues reaching pub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need try-catch, but we should check the status of pubVersionFinderResponse. Done.

pubVersionFinder.httpClient.close();
return pubVersionFinderResponse.result == PubVersionFinderResult.success &&
pubVersionFinderResponse.versions.contains(version);
}
}

enum _PublishCheckResult {
_needsPublish,

_noPublish,

error,
}
8 changes: 4 additions & 4 deletions script/tool/lib/src/version_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,20 @@ class VersionCheckCommand extends PluginCommand {
}
Version sourceVersion;
if (argResults[_againstPubFlag] as bool) {
final String pacakgeName = pubspecFile.parent.basename;
final String packageName = pubspecFile.parent.basename;
final PubVersionFinder pubVersionFinder = PubVersionFinder(
package: pacakgeName, httpClient: httpClient ?? http.Client());
package: packageName, httpClient: httpClient ?? http.Client());
final PubVersionFinderResponse pubVersionFinderResponse =
await pubVersionFinder.getPackageVersion();
pubVersionFinder.httpClient.close();
switch (pubVersionFinderResponse.result) {
case PubVersionFinderResult.success:
sourceVersion = pubVersionFinderResponse.versions.first;
print('$indentation$pacakgeName: Current largest version on pub: $sourceVersion');
print('$indentation$packageName: Current largest version on pub: $sourceVersion');
break;
case PubVersionFinderResult.fail:
printErrorAndExit(errorMessage: '''
${indentation}Error fetching version on pub for ${pacakgeName}.
${indentation}Error fetching version on pub for ${packageName}.
${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
''');
Expand Down
187 changes: 187 additions & 0 deletions script/tool/test/publish_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
// found in the LICENSE file.

import 'dart:collection';
import 'dart:convert';
import 'dart:io' as io;

import 'package:args/command_runner.dart';
import 'package:file/file.dart';
import 'package:flutter_plugin_tools/src/common.dart';
import 'package:flutter_plugin_tools/src/publish_check_command.dart';
import 'package:http/http.dart' as http;
import 'package:http/testing.dart';
import 'package:test/test.dart';

import 'mocks.dart';
Expand Down Expand Up @@ -126,6 +129,190 @@ void main() {

expect(runner.run(<String>['publish-check']), throwsA(isA<ToolExit>()));
});

test(
'--log-status: Log no_publish at the end if there are no packages need to be published. ',
() async {
const Map<String, dynamic> httpResponseA = <String, dynamic>{
'name': 'a',
'versions': <String>[
'0.0.1',
'0.1.0',
],
};

const Map<String, dynamic> httpResponseB = <String, dynamic>{
'name': 'b',
'versions': <String>[
'0.0.1',
'0.1.0',
'0.2.0',
],
};

final MockClient mockClient = MockClient((http.Request request) async {
print('url ${request.url}');
print(request.url.pathSegments.last);
if (request.url.pathSegments.last == 'no_publish_a.json') {
return http.Response(json.encode(httpResponseA), 200);
} else if (request.url.pathSegments.last == 'no_publish_b.json') {
return http.Response(json.encode(httpResponseB), 200);
}
return null;
});
final PublishCheckCommand command = PublishCheckCommand(
mockPackagesDir, mockFileSystem,
processRunner: processRunner, httpClient: mockClient);

runner = CommandRunner<void>(
'publish_check_command',
'Test for publish-check command.',
);
runner.addCommand(command);

final Directory plugin1Dir =
createFakePlugin('no_publish_a', includeVersion: true);
final Directory plugin2Dir =
createFakePlugin('no_publish_b', includeVersion: true);

createFakePubspec(plugin1Dir,
name: 'no_publish_a', includeVersion: true, version: '0.1.0');
createFakePubspec(plugin2Dir,
name: 'no_publish_b', includeVersion: true, version: '0.2.0');

processRunner.processesToReturn.add(
MockProcess()..exitCodeCompleter.complete(0),
);

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

expect(output.last, 'no-publish');
});

test(
'--log-status: Log needs-publish at the end if there is at least 1 plugin needs to be published. ',
() async {
const Map<String, dynamic> httpResponseA = <String, dynamic>{
'name': 'a',
'versions': <String>[
'0.0.1',
'0.1.0',
],
};

const Map<String, dynamic> httpResponseB = <String, dynamic>{
'name': 'b',
'versions': <String>[
'0.0.1',
'0.1.0',
],
};

final MockClient mockClient = MockClient((http.Request request) async {
print('url ${request.url}');
print(request.url.pathSegments.last);
if (request.url.pathSegments.last == 'no_publish_a.json') {
return http.Response(json.encode(httpResponseA), 200);
} else if (request.url.pathSegments.last == 'no_publish_b.json') {
return http.Response(json.encode(httpResponseB), 200);
}
return null;
});
final PublishCheckCommand command = PublishCheckCommand(
mockPackagesDir, mockFileSystem,
processRunner: processRunner, httpClient: mockClient);

runner = CommandRunner<void>(
'publish_check_command',
'Test for publish-check command.',
);
runner.addCommand(command);

final Directory plugin1Dir =
createFakePlugin('no_publish_a', includeVersion: true);
final Directory plugin2Dir =
createFakePlugin('no_publish_b', includeVersion: true);

createFakePubspec(plugin1Dir,
name: 'no_publish_a', includeVersion: true, version: '0.1.0');
createFakePubspec(plugin2Dir,
name: 'no_publish_b', includeVersion: true, version: '0.2.0');

processRunner.processesToReturn.add(
MockProcess()..exitCodeCompleter.complete(0),
);

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

expect(output.last, 'needs-publish');
});

test(
'--log-status: Log error at the end if there is at least 1 plugin contains error.',
() async {
const Map<String, dynamic> httpResponseA = <String, dynamic>{
'name': 'a',
'versions': <String>[
'0.0.1',
'0.1.0',
],
};

const Map<String, dynamic> httpResponseB = <String, dynamic>{
'name': 'b',
'versions': <String>[
'0.0.1',
'0.1.0',
],
};

final MockClient mockClient = MockClient((http.Request request) async {
print('url ${request.url}');
print(request.url.pathSegments.last);
if (request.url.pathSegments.last == 'no_publish_a.json') {
return http.Response(json.encode(httpResponseA), 200);
} else if (request.url.pathSegments.last == 'no_publish_b.json') {
return http.Response(json.encode(httpResponseB), 200);
}
return null;
});
final PublishCheckCommand command = PublishCheckCommand(
mockPackagesDir, mockFileSystem,
processRunner: processRunner, httpClient: mockClient);

runner = CommandRunner<void>(
'publish_check_command',
'Test for publish-check command.',
);
runner.addCommand(command);

final Directory plugin1Dir =
createFakePlugin('no_publish_a', includeVersion: true);
final Directory plugin2Dir =
createFakePlugin('no_publish_b', includeVersion: true);

createFakePubspec(plugin1Dir,
name: 'no_publish_a', includeVersion: true, version: '0.1.0');
createFakePubspec(plugin2Dir,
name: 'no_publish_b', includeVersion: true, version: '0.2.0');
await plugin1Dir.childFile('pubspec.yaml').writeAsString('bad-yaml');

processRunner.processesToReturn.add(
MockProcess()..exitCodeCompleter.complete(0),
);

bool hasError = false;
final List<String> output = await runCapturingPrint(
runner, <String>['publish-check', '--log-status'],
errorHandler: (Error error) {
expect(error, isA<ToolExit>());
hasError = true;
});
expect(hasError, isTrue);
expect(output.last, 'error');
});
});
}

Expand Down