From b6a1cdd2398d5d7d2c8f74769a9e56f93654afa8 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 12:22:38 -0700 Subject: [PATCH 01/14] add version check against pub --- script/tool/lib/src/common.dart | 93 +++++++++++ .../tool/lib/src/version_check_command.dart | 43 ++++- script/tool/test/common_test.dart | 82 ++++++++++ script/tool/test/version_check_test.dart | 151 +++++++++++++++++- 4 files changed, 365 insertions(+), 4 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index c16ba8e957e0..27ce8008851b 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -11,6 +11,7 @@ import 'package:args/command_runner.dart'; import 'package:colorize/colorize.dart'; import 'package:file/file.dart'; import 'package:git/git.dart'; +import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; @@ -549,6 +550,98 @@ class ProcessRunner { } } +/// Finding version of [package] that is published on pub. +class PubVersionFinder { + + /// Constructor. + PubVersionFinder({@required this.package, this.pubHost = defaultPubHost, @required this.httpClient}); + + /// The default pub host to use. + static const String defaultPubHost = 'https://pub.dev'; + + /// The package name. + final String package; + + /// The pub host url, defaults to `https://pub.dev`. + final String pubHost; + + /// The http client, can override for testing. + final http.Client httpClient; + + /// Get the package version on pub. + Future getPackageVersion() async { + final Uri pubHostUri = Uri.parse(pubHost); + final Uri url = pubHostUri.replace(path: '/packages/$package.json'); + final http.Response response = await httpClient.get(url); + + if (response.statusCode == 404) { + return PubVersionFinderResponse( + versions: null, + result: PubVersionFinderResult.noPackageFound, + httpResponse: response); + } else if (response.statusCode != 200) { + print(''' +Error fetching version on pub for $package. +HTTP Status ${response.statusCode} +HTTP response: ${response.body} +'''); + return PubVersionFinderResponse( + versions: null, + result: PubVersionFinderResult.fail, + httpResponse: response); + } + final List versions = + (json.decode(response.body)['versions'] as List) + .map( + (final dynamic versionString) => Version.parse(versionString as String)) + .toList(); + + versions.sort((Version a, Version b) { + /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. + return b.compareTo(a); + }); + + return PubVersionFinderResponse( + versions: versions, + result: PubVersionFinderResult.success, + httpResponse: response); + } +} + +/// Represents a response for [PubVersionFinder]. +class PubVersionFinderResponse { + /// Constructor. + const PubVersionFinderResponse({this.versions, this.result, this.httpResponse}); + + /// The versions found in [PubVersionFinder]. + /// + /// This is sorted by largest to smallest, so the first element in the list is the largest version. + /// Might be `null` if the [result] is not [PubVersionFinderResult.success]. + final List versions; + + /// The result of the version finder. + final PubVersionFinderResult result; + + /// The response object of the http request. + final http.Response httpResponse; +} + +/// An enum represents the result of [PubVersionFinder]. +enum PubVersionFinderResult { + /// The version finder successfully found a version. + success, + + /// The version finder failed to find a valid version. + /// + /// This might due to http connection errors or user errors. + fail, + + /// The version finder failed to locate the package. + /// + /// This is usually OK when the package is new. + noPackageFound, +} + /// Finding diffs based on `baseGitDir` and `baseSha`. class GitVersionFinder { /// Constructor diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 0b552e8bff4d..6f03bf5de297 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; import 'package:file/file.dart'; import 'package:git/git.dart'; +import 'package:http/http.dart' as http; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; @@ -74,8 +75,19 @@ class VersionCheckCommand extends PluginCommand { FileSystem fileSystem, { ProcessRunner processRunner = const ProcessRunner(), GitDir gitDir, + this.httpClient, }) : super(packagesDir, fileSystem, - processRunner: processRunner, gitDir: gitDir); + processRunner: processRunner, gitDir: gitDir) { + argParser.addFlag( + _againstPubFlag, + help: 'Whether the version check should run against the version on pub.\n' + 'Defaults to false, which means the version check only run against the previous version in code.', + defaultsTo: false, + negatable: true, + ); + } + + static const String _againstPubFlag = 'against-pub'; @override final String name = 'version-check'; @@ -86,6 +98,8 @@ class VersionCheckCommand extends PluginCommand { 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' 'This command requires "pub" and "flutter" to be in your path.'; + final http.Client httpClient; + @override Future run() async { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); @@ -115,8 +129,31 @@ class VersionCheckCommand extends PluginCommand { 'intentionally has no version should be marked ' '"publish_to: none".'); } - final Version masterVersion = - await gitVersionFinder.getPackageVersion(pubspecPath); + Version masterVersion; + if (argResults[_againstPubFlag] as bool) { + final PubVersionFinder pubVersionFinder = PubVersionFinder( + package: pubspecFile.parent.basename, + httpClient: httpClient ?? http.Client()); + final PubVersionFinderResponse pubVersionFinderResponse = + await pubVersionFinder.getPackageVersion(); + switch (pubVersionFinderResponse.result) { + case PubVersionFinderResult.success: + masterVersion = pubVersionFinderResponse.versions.first; + break; + case PubVersionFinderResult.fail: + printErrorAndExit(errorMessage: ''' +${indentation}Error fetching version on pub for ${pubspecFile.parent.basename}. +${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} +${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} +'''); + break; + case PubVersionFinderResult.noPackageFound: + masterVersion = null; + break; + } + } else { + masterVersion = await gitVersionFinder.getPackageVersion(pubspecPath); + } if (masterVersion == null) { print('${indentation}Unable to find pubspec in master. ' 'Safe to ignore if the project is new.'); diff --git a/script/tool/test/common_test.dart b/script/tool/test/common_test.dart index 57bc1e20f915..ac5e04cf20f6 100644 --- a/script/tool/test/common_test.dart +++ b/script/tool/test/common_test.dart @@ -2,13 +2,17 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:convert'; import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:flutter_plugin_tools/src/common.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; import 'package:git/git.dart'; import 'package:mockito/mockito.dart'; +import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; import 'util.dart'; @@ -300,6 +304,84 @@ file2/file2.cc .runCommand(['diff', '--name-only', customBaseSha, 'HEAD'])); }); }); + + group('$PubVersionFinder', () { + + test('Package does not exist.', () async { + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response('', 404); + }); + final PubVersionFinder finder = + PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinderResponse response = + await finder.getPackageVersion(); + + expect(response.versions, isNull); + expect(response.result, PubVersionFinderResult.noPackageFound); + expect(response.httpResponse.statusCode, 404); + expect(response.httpResponse.body, ''); + }); + + test('HTTP error when getting versions from pub', () async { + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response('', 400); + }); + final PubVersionFinder finder = + PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinderResponse response = + await finder.getPackageVersion(); + + expect(response.versions, isNull); + expect(response.result, PubVersionFinderResult.fail); + expect(response.httpResponse.statusCode, 400); + expect(response.httpResponse.body, ''); + }); + + test('Get a correct list of versions when http response is OK.', () async { + const Map httpResponse = { + 'name': 'some_package', + 'versions': [ + '0.0.1', + '0.0.2', + '0.0.2+2', + '0.1.1', + '0.0.1+1', + '0.1.0', + '0.2.0', + '0.1.0+1', + '0.0.2+1', + '2.0.0', + '1.2.0', + '1.0.0', + ], + }; + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response(json.encode(httpResponse), 200); + }); + final PubVersionFinder finder = + PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinderResponse response = + await finder.getPackageVersion(); + + expect(response.versions, [ + Version.parse('2.0.0'), + Version.parse('1.2.0'), + Version.parse('1.0.0'), + Version.parse('0.2.0'), + Version.parse('0.1.1'), + Version.parse('0.1.0+1'), + Version.parse('0.1.0'), + Version.parse('0.0.2+2'), + Version.parse('0.0.2+1'), + Version.parse('0.0.2'), + Version.parse('0.0.1+1'), + Version.parse('0.0.1'), + ]); + expect(response.result, PubVersionFinderResult.success); + expect(response.httpResponse.statusCode, 200); + expect(response.httpResponse.body, json.encode(httpResponse)); + }); + }); } class SamplePluginCommand extends PluginCommand { diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_test.dart index 04348310a2f6..365ddb3be487 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_test.dart @@ -3,12 +3,15 @@ // found in the LICENSE file. import 'dart:async'; +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:git/git.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; import 'package:flutter_plugin_tools/src/version_check_command.dart'; @@ -40,18 +43,20 @@ class MockGitDir extends Mock implements GitDir {} class MockProcessResult extends Mock implements io.ProcessResult {} void main() { + const String indentation = ' '; group('$VersionCheckCommand', () { CommandRunner runner; RecordingProcessRunner processRunner; List> gitDirCommands; String gitDiffResponse; Map gitShowResponses; + MockGitDir gitDir; setUp(() { gitDirCommands = >[]; gitDiffResponse = ''; gitShowResponses = {}; - final MockGitDir gitDir = MockGitDir(); + gitDir = MockGitDir(); when(gitDir.runCommand(any)).thenAnswer((Invocation invocation) { gitDirCommands.add(invocation.positionalArguments[0] as List); final MockProcessResult mockProcessResult = MockProcessResult(); @@ -522,6 +527,150 @@ void main() { ); } on ToolExit catch (_) {} }); + + test('allows valid against pub', () async { + const Map httpResponse = { + 'name': 'some_package', + 'versions': [ + '0.0.1', + '0.0.2', + '1.0.0', + ], + }; + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response(json.encode(httpResponse), 200); + }); + final VersionCheckCommand command = VersionCheckCommand( + mockPackagesDir, mockFileSystem, + processRunner: processRunner, gitDir: gitDir, httpClient: mockClient); + + runner = CommandRunner( + 'version_check_command', 'Test for $VersionCheckCommand'); + runner.addCommand(command); + + createFakePlugin('plugin', includeChangeLog: true, includeVersion: true); + gitDiffResponse = 'packages/plugin/pubspec.yaml'; + gitShowResponses = { + 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', + 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', + }; + final List output = await runCapturingPrint(runner, + ['version-check', '--base-sha=master', '--against-pub']); + + expect( + output, + containsAllInOrder([ + 'No version check errors found!', + ]), + ); + expect(gitDirCommands.length, equals(2)); + }); + + test('denies invalid against pub', () async { + const Map httpResponse = { + 'name': 'some_package', + 'versions': [ + '0.0.1', + '0.0.2', + ], + }; + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response(json.encode(httpResponse), 200); + }); + final VersionCheckCommand command = VersionCheckCommand( + mockPackagesDir, mockFileSystem, + processRunner: processRunner, gitDir: gitDir, httpClient: mockClient); + + runner = CommandRunner( + 'version_check_command', 'Test for $VersionCheckCommand'); + runner.addCommand(command); + + createFakePlugin('plugin', includeChangeLog: true, includeVersion: true); + gitDiffResponse = 'packages/plugin/pubspec.yaml'; + gitShowResponses = { + 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', + 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', + }; + final Future> result = runCapturingPrint(runner, + ['version-check', '--base-sha=master', '--against-pub']); + + await expectLater( + result, + throwsA(const TypeMatcher()), + ); + }); + + test( + 'throw and print error message if http request failed when checking against pub', + () async { + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response('xx', 400); + }); + final VersionCheckCommand command = VersionCheckCommand( + mockPackagesDir, mockFileSystem, + processRunner: processRunner, gitDir: gitDir, httpClient: mockClient); + + runner = CommandRunner( + 'version_check_command', 'Test for $VersionCheckCommand'); + runner.addCommand(command); + + createFakePlugin('plugin', includeChangeLog: true, includeVersion: true); + gitDiffResponse = 'packages/plugin/pubspec.yaml'; + gitShowResponses = { + 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', + 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', + }; + final Future> result = runCapturingPrint(runner, + ['version-check', '--base-sha=master', '--against-pub']); + + await expectLater( + result, + throwsA(const TypeMatcher()), + ); + try { + final List outputValue = await result; + await expectLater( + outputValue, + containsAllInOrder([ + ''' +${indentation}Error fetching version on pub for plugin}. +${indentation}HTTP Status 404} +${indentation}HTTP response: xx} +''', + ]), + ); + } on ToolExit catch (_) {} + }); + + test('when checking against pub, allow any version if http status is 404.', + () async { + final MockClient mockClient = MockClient((http.Request request) async { + return http.Response('xx', 404); + }); + final VersionCheckCommand command = VersionCheckCommand( + mockPackagesDir, mockFileSystem, + processRunner: processRunner, gitDir: gitDir, httpClient: mockClient); + + runner = CommandRunner( + 'version_check_command', 'Test for $VersionCheckCommand'); + runner.addCommand(command); + + createFakePlugin('plugin', includeChangeLog: true, includeVersion: true); + gitDiffResponse = 'packages/plugin/pubspec.yaml'; + gitShowResponses = { + 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', + 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', + }; + final List result = await runCapturingPrint(runner, + ['version-check', '--base-sha=master', '--against-pub']); + + expect( + result, + containsAllInOrder([ + 'No version check errors found!', + ]), + ); + }); }); group('Pre 1.0', () { From cf26cd477a90d5bd9b1caa716c67cdb51d11bb63 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 12:32:57 -0700 Subject: [PATCH 02/14] close client when not used --- script/tool/lib/src/version_check_command.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 6f03bf5de297..88d1e1d0c32a 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -98,6 +98,7 @@ class VersionCheckCommand extends PluginCommand { 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' 'This command requires "pub" and "flutter" to be in your path.'; + /// The http client used to query pub server. final http.Client httpClient; @override @@ -131,11 +132,13 @@ class VersionCheckCommand extends PluginCommand { } Version masterVersion; if (argResults[_againstPubFlag] as bool) { + final http.Client client = + httpClient ?? http.Client(); final PubVersionFinder pubVersionFinder = PubVersionFinder( - package: pubspecFile.parent.basename, - httpClient: httpClient ?? http.Client()); + package: pubspecFile.parent.basename, httpClient: client); final PubVersionFinderResponse pubVersionFinderResponse = await pubVersionFinder.getPackageVersion(); + client.close(); switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: masterVersion = pubVersionFinderResponse.versions.first; From 46ca9ae4cb504421d7cc2b2e1ff12e9c1949d8d2 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 12:37:54 -0700 Subject: [PATCH 03/14] close client --- script/tool/lib/src/common.dart | 4 ++++ script/tool/lib/src/version_check_command.dart | 6 ++---- script/tool/test/version_check_test.dart | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index 27ce8008851b..4b1170b18fb6 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -551,6 +551,8 @@ class ProcessRunner { } /// Finding version of [package] that is published on pub. +/// +/// Note: you should manually close the [httpClient] when done using the finder. class PubVersionFinder { /// Constructor. @@ -566,6 +568,8 @@ class PubVersionFinder { final String pubHost; /// The http client, can override for testing. + /// + /// You should manually close this client when done using this finder. final http.Client httpClient; /// Get the package version on pub. diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 88d1e1d0c32a..10b1f4e753c2 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -132,13 +132,11 @@ class VersionCheckCommand extends PluginCommand { } Version masterVersion; if (argResults[_againstPubFlag] as bool) { - final http.Client client = - httpClient ?? http.Client(); final PubVersionFinder pubVersionFinder = PubVersionFinder( - package: pubspecFile.parent.basename, httpClient: client); + package: pubspecFile.parent.basename, httpClient: httpClient ?? http.Client()); final PubVersionFinderResponse pubVersionFinderResponse = await pubVersionFinder.getPackageVersion(); - client.close(); + pubVersionFinder.httpClient.close(); switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: masterVersion = pubVersionFinderResponse.versions.first; diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_test.dart index 365ddb3be487..6df89c37ee16 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_test.dart @@ -563,7 +563,6 @@ void main() { 'No version check errors found!', ]), ); - expect(gitDirCommands.length, equals(2)); }); test('denies invalid against pub', () async { From 7262ef3ea40569398276e01eb697a5a3dfbc73ac Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 12:41:06 -0700 Subject: [PATCH 04/14] add log --- script/tool/lib/src/version_check_command.dart | 6 ++++-- script/tool/test/version_check_test.dart | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 10b1f4e753c2..cbd4a2cb9cd9 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -132,18 +132,20 @@ class VersionCheckCommand extends PluginCommand { } Version masterVersion; if (argResults[_againstPubFlag] as bool) { + final String pacakgeName = pubspecFile.parent.basename; final PubVersionFinder pubVersionFinder = PubVersionFinder( - package: pubspecFile.parent.basename, httpClient: httpClient ?? http.Client()); + package: pacakgeName, httpClient: httpClient ?? http.Client()); final PubVersionFinderResponse pubVersionFinderResponse = await pubVersionFinder.getPackageVersion(); pubVersionFinder.httpClient.close(); switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: masterVersion = pubVersionFinderResponse.versions.first; + print('$indentation$pacakgeName: Current largest version on pub: $masterVersion'); break; case PubVersionFinderResult.fail: printErrorAndExit(errorMessage: ''' -${indentation}Error fetching version on pub for ${pubspecFile.parent.basename}. +${indentation}Error fetching version on pub for ${pacakgeName}. ${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_test.dart index 6df89c37ee16..68b2ab3ebbe2 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_test.dart @@ -560,6 +560,7 @@ void main() { expect( output, containsAllInOrder([ + '${indentation}plugin: Current largest version on pub: 1.0.0', 'No version check errors found!', ]), ); From 3f2d76d709853d819cb6f1f074c83f099e7ce163 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 14:16:45 -0700 Subject: [PATCH 05/14] fix tests --- script/tool/lib/src/common.dart | 5 - .../tool/lib/src/version_check_command.dart | 32 +-- script/tool/test/util.dart | 14 +- script/tool/test/version_check_test.dart | 219 +++++++++++------- 4 files changed, 162 insertions(+), 108 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index 4b1170b18fb6..aed2d700bbbc 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -584,11 +584,6 @@ class PubVersionFinder { result: PubVersionFinderResult.noPackageFound, httpResponse: response); } else if (response.statusCode != 200) { - print(''' -Error fetching version on pub for $package. -HTTP Status ${response.statusCode} -HTTP response: ${response.body} -'''); return PubVersionFinderResponse( versions: null, result: PubVersionFinderResult.fail, diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index cbd4a2cb9cd9..9517f64e201b 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -130,7 +130,7 @@ class VersionCheckCommand extends PluginCommand { 'intentionally has no version should be marked ' '"publish_to: none".'); } - Version masterVersion; + Version sourceVersion; if (argResults[_againstPubFlag] as bool) { final String pacakgeName = pubspecFile.parent.basename; final PubVersionFinder pubVersionFinder = PubVersionFinder( @@ -140,8 +140,8 @@ class VersionCheckCommand extends PluginCommand { pubVersionFinder.httpClient.close(); switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: - masterVersion = pubVersionFinderResponse.versions.first; - print('$indentation$pacakgeName: Current largest version on pub: $masterVersion'); + sourceVersion = pubVersionFinderResponse.versions.first; + print('$indentation$pacakgeName: Current largest version on pub: $sourceVersion'); break; case PubVersionFinderResult.fail: printErrorAndExit(errorMessage: ''' @@ -151,33 +151,39 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); break; case PubVersionFinderResult.noPackageFound: - masterVersion = null; + sourceVersion = null; break; } } else { - masterVersion = await gitVersionFinder.getPackageVersion(pubspecPath); + sourceVersion = await gitVersionFinder.getPackageVersion(pubspecPath); } - if (masterVersion == null) { - print('${indentation}Unable to find pubspec in master. ' - 'Safe to ignore if the project is new.'); + if (sourceVersion == null) { + String safeToIgnoreMessage; + if (argResults[_againstPubFlag] as bool) { + safeToIgnoreMessage = '${indentation}Unable to find package on pub server.'; + } else { + safeToIgnoreMessage = '${indentation}Unable to find pubspec in master.'; + } + print('$safeToIgnoreMessage Safe to ignore if the project is new.'); continue; } - if (masterVersion == headVersion) { + if (sourceVersion == headVersion) { print('${indentation}No version change.'); continue; } final Map allowedNextVersions = - getAllowedNextVersions(masterVersion, headVersion); + getAllowedNextVersions(sourceVersion, headVersion); if (!allowedNextVersions.containsKey(headVersion)) { + final String source = (argResults[_againstPubFlag] as bool) ? 'pub' : 'master'; final String error = '${indentation}Incorrectly updated version.\n' - '${indentation}HEAD: $headVersion, master: $masterVersion.\n' + '${indentation}HEAD: $headVersion, $source: $sourceVersion.\n' '${indentation}Allowed versions: $allowedNextVersions'; printErrorAndExit(errorMessage: error); } else { - print('$indentation$headVersion -> $masterVersion'); + print('$indentation$headVersion -> $sourceVersion'); } final bool isPlatformInterface = @@ -264,7 +270,7 @@ The first version listed in CHANGELOG.md is $fromChangeLog. printErrorAndExit(errorMessage: ''' When bumping the version for release, the NEXT section should be incorporated into the new version's release notes. - '''); +'''); } } diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 4dc019c968d5..8d8182af231f 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -193,19 +193,29 @@ void cleanupPackages() { }); } +typedef _ErrorHandler = void Function(Error error); + /// Run the command [runner] with the given [args] and return /// what was printed. +/// A custom [errorHandler] can be used to handle the runner error as desired without throwing. Future> runCapturingPrint( - CommandRunner runner, List args) async { + CommandRunner runner, List args, {_ErrorHandler errorHandler}) async { final List prints = []; final ZoneSpecification spec = ZoneSpecification( print: (_, __, ___, String message) { prints.add(message); }, ); - await Zone.current + try { + await Zone.current .fork(specification: spec) .run>(() => runner.run(args)); + } on Error catch (e) { + if (errorHandler == null) { + rethrow; + } + errorHandler(e); + } return prints; } diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_test.dart index 68b2ab3ebbe2..2bc2f0a2ce59 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_test.dart @@ -42,6 +42,14 @@ class MockGitDir extends Mock implements GitDir {} class MockProcessResult extends Mock implements io.ProcessResult {} +const String _redColorMessagePrefix = '\x1B[31m'; +const String _redColorMessagePostfix = '\x1B[0m'; + +// Some error message was printed in a "Colorized" red message. So `\x1B[31m` and `\x1B[0m` needs to be included. +String _redColorString(String string) { + return '$_redColorMessagePrefix$string$_redColorMessagePostfix'; +} + void main() { const String indentation = ' '; group('$VersionCheckCommand', () { @@ -170,6 +178,7 @@ void main() { expect( output, containsAllInOrder([ + '${indentation}Unable to find pubspec in master. Safe to ignore if the project is new.', 'No version check errors found!', ]), ); @@ -326,25 +335,27 @@ void main() { * Some changes. '''; createFakeCHANGELOG(pluginDirectory, changelog); - final Future> output = runCapturingPrint( - runner, ['version-check', '--base-sha=master']); - await expectLater( + bool hasError = false; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); + + expect( output, - throwsA(const TypeMatcher()), + containsAllInOrder([ + _redColorString(''' +versions for plugin in CHANGELOG.md and pubspec.yaml do not match. +The version in pubspec.yaml is 1.0.1. +The first version listed in CHANGELOG.md is 1.0.2. +'''), + ]), ); - try { - final List outputValue = await output; - await expectLater( - outputValue, - containsAllInOrder([ - ''' - versions for plugin in CHANGELOG.md and pubspec.yaml do not match. - The version in pubspec.yaml is 1.0.1. - The first version listed in CHANGELOG.md is 1.0.2. - ''', - ]), - ); - } on ToolExit catch (_) {} }); test('Success if CHANGELOG and pubspec versions match', () async { @@ -393,25 +404,29 @@ void main() { * Some other changes. '''; createFakeCHANGELOG(pluginDirectory, changelog); - final Future> output = runCapturingPrint( - runner, ['version-check', '--base-sha=master']); - await expectLater( + bool hasError = false; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); + + expect( output, - throwsA(const TypeMatcher()), - ); - try { - final List outputValue = await output; - await expectLater( - outputValue, - containsAllInOrder([ + containsAllInOrder([ + _redColorString( ''' - versions for plugin in CHANGELOG.md and pubspec.yaml do not match. - The version in pubspec.yaml is 1.0.0. - The first version listed in CHANGELOG.md is 1.0.1. - ''', - ]), - ); - } on ToolExit catch (_) {} +versions for plugin in CHANGELOG.md and pubspec.yaml do not match. +The version in pubspec.yaml is 1.0.0. +The first version listed in CHANGELOG.md is 1.0.1. +''', + ) + ]), + ); }); test('Allow NEXT as a placeholder for gathering CHANGELOG entries', @@ -468,25 +483,28 @@ void main() { * Some other changes. '''; createFakeCHANGELOG(pluginDirectory, changelog); - final Future> output = runCapturingPrint( - runner, ['version-check', '--base-sha=master']); - await expectLater( + bool hasError = false; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); + + expect( output, - throwsA(const TypeMatcher()), - ); - try { - final List outputValue = await output; - await expectLater( - outputValue, - containsAllInOrder([ + containsAllInOrder([ + _redColorString( ''' - versions for plugin in CHANGELOG.md and pubspec.yaml do not match. - The version in pubspec.yaml is 1.0.0. - The first version listed in CHANGELOG.md is 1.0.1. - ''', - ]), - ); - } on ToolExit catch (_) {} +When bumping the version for release, the NEXT section should be incorporated +into the new version's release notes. +''', + ) + ]), + ); }); test('Fail if the version changes without replacing NEXT', () async { @@ -507,25 +525,30 @@ void main() { * Some other changes. '''; createFakeCHANGELOG(pluginDirectory, changelog); - final Future> output = runCapturingPrint( - runner, ['version-check', '--base-sha=master']); - await expectLater( + bool hasError = false; + final List output = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); + + expect( output, - throwsA(const TypeMatcher()), - ); - try { - final List outputValue = await output; - await expectLater( - outputValue, - containsAllInOrder([ + containsAllInOrder([ + 'Found NEXT; validating next version in the CHANGELOG.', + _redColorString( ''' - versions for plugin in CHANGELOG.md and pubspec.yaml do not match. - The version in pubspec.yaml is 1.0.0. - The first version listed in CHANGELOG.md is 1.0.1. - ''', - ]), - ); - } on ToolExit catch (_) {} +versions for plugin in CHANGELOG.md and pubspec.yaml do not match. +The version in pubspec.yaml is 1.0.1. +The first version listed in CHANGELOG.md is 1.0.0. +''', + ) + ]), + ); }); test('allows valid against pub', () async { @@ -591,12 +614,28 @@ void main() { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', }; - final Future> result = runCapturingPrint(runner, - ['version-check', '--base-sha=master', '--against-pub']); - await expectLater( + bool hasError = false; + final List result = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); + + expect( result, - throwsA(const TypeMatcher()), + containsAllInOrder([ + _redColorString( + ''' +${indentation}Incorrectly updated version. +${indentation}HEAD: 2.0.0, pub: 0.0.2. +${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: NextVersionType.MINOR, 0.0.3: NextVersionType.PATCH}''', + ) + ]), ); }); @@ -620,26 +659,29 @@ void main() { 'master:packages/plugin/pubspec.yaml': 'version: 1.0.0', 'HEAD:packages/plugin/pubspec.yaml': 'version: 2.0.0', }; - final Future> result = runCapturingPrint(runner, - ['version-check', '--base-sha=master', '--against-pub']); + bool hasError = false; + final List result = await runCapturingPrint(runner, [ + 'version-check', + '--base-sha=master', + '--against-pub' + ], errorHandler: (Error e) { + expect(e, isA()); + hasError = true; + }); + expect(hasError, isTrue); - await expectLater( + expect( result, - throwsA(const TypeMatcher()), - ); - try { - final List outputValue = await result; - await expectLater( - outputValue, - containsAllInOrder([ + containsAllInOrder([ + _redColorString( ''' -${indentation}Error fetching version on pub for plugin}. -${indentation}HTTP Status 404} -${indentation}HTTP response: xx} +${indentation}Error fetching version on pub for plugin. +${indentation}HTTP Status 400 +${indentation}HTTP response: xx ''', - ]), - ); - } on ToolExit catch (_) {} + ) + ]), + ); }); test('when checking against pub, allow any version if http status is 404.', @@ -667,6 +709,7 @@ ${indentation}HTTP response: xx} expect( result, containsAllInOrder([ + '${indentation}Unable to find package on pub server. Safe to ignore if the project is new.', 'No version check errors found!', ]), ); From 497292e1d202a9d17706a8d30a94e759d70e2376 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 30 Apr 2021 14:20:00 -0700 Subject: [PATCH 06/14] add next flag --- script/tool/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index e489a29d0cf7..cf76efc667f6 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,3 +1,7 @@ +## NEXT + +- Add `against-pub` flag for version-check. + ## 0.1.0 - **NOTE**: This is no longer intended as a general-purpose package, and is now From e5d149e588120355b246fa94af1acd8ff4f86d1f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Sat, 1 May 2021 13:01:16 -0700 Subject: [PATCH 07/14] publish check query pub --- .../tool/lib/src/publish_check_command.dart | 90 +++++++-- .../tool/lib/src/version_check_command.dart | 8 +- .../tool/test/publish_check_command_test.dart | 187 ++++++++++++++++++ 3 files changed, 270 insertions(+), 15 deletions(-) diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index 0fb9dbad60a8..07df1b01d5e4 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -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'; @@ -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, @@ -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' + 'The possible values are:\n' + ' $_resultNeedsPublish: There is at least one package need to be published. They also passed all publish checks. \n' + ' $_resultNoPublish: There are no packages need to be published. Either no pubspec change detected or the version has already been published. \n' + ' $_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'; @@ -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 run() async { final List failedPackages = []; + 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; } } @@ -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(); + 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. + if (argResults[_logStatusFlag] as bool) { + print(resultToLog); + } + if (failedPackages.isNotEmpty) { + throw ToolExit(1); + } } Pubspec _tryParsePubspec(Directory package) { @@ -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 _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 _checkIfAlreadyPublished( + {String packageName, Version version}) async { + final PubVersionFinder pubVersionFinder = PubVersionFinder( + package: packageName, httpClient: httpClient ?? http.Client()); + final PubVersionFinderResponse pubVersionFinderResponse = + await pubVersionFinder.getPackageVersion(); + pubVersionFinder.httpClient.close(); + return pubVersionFinderResponse.result == PubVersionFinderResult.success && + pubVersionFinderResponse.versions.contains(version); + } +} + +enum _PublishCheckResult { + _needsPublish, + + _noPublish, + + error, } diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 9517f64e201b..07708ea9318f 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -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} '''); diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart index 0a9d36f2ea6f..5975074c2730 100644 --- a/script/tool/test/publish_check_command_test.dart +++ b/script/tool/test/publish_check_command_test.dart @@ -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'; @@ -126,6 +129,190 @@ void main() { expect(runner.run(['publish-check']), throwsA(isA())); }); + + test( + '--log-status: Log no_publish at the end if there are no packages need to be published. ', + () async { + const Map httpResponseA = { + 'name': 'a', + 'versions': [ + '0.0.1', + '0.1.0', + ], + }; + + const Map httpResponseB = { + 'name': 'b', + 'versions': [ + '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( + '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 output = await runCapturingPrint( + runner, ['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 httpResponseA = { + 'name': 'a', + 'versions': [ + '0.0.1', + '0.1.0', + ], + }; + + const Map httpResponseB = { + 'name': 'b', + 'versions': [ + '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( + '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 output = await runCapturingPrint( + runner, ['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 httpResponseA = { + 'name': 'a', + 'versions': [ + '0.0.1', + '0.1.0', + ], + }; + + const Map httpResponseB = { + 'name': 'b', + 'versions': [ + '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( + '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 output = await runCapturingPrint( + runner, ['publish-check', '--log-status'], + errorHandler: (Error error) { + expect(error, isA()); + hasError = true; + }); + expect(hasError, isTrue); + expect(output.last, 'error'); + }); }); } From 453f8bf63a1cd26b9ec8db85072462de770d8619 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Sat, 1 May 2021 13:04:58 -0700 Subject: [PATCH 08/14] add changelog entry --- script/tool/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index cf76efc667f6..fcdc67cf76f1 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,6 +1,7 @@ ## NEXT - Add `against-pub` flag for version-check. +- Add `log-status` flag for publish-check. ## 0.1.0 From 3880f2e4738711e473071a6ad88af5e1c9f501de Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 4 May 2021 14:05:19 -0700 Subject: [PATCH 09/14] revuiew --- .../tool/lib/src/publish_check_command.dart | 66 +++++++++++++++---- .../tool/test/publish_check_command_test.dart | 22 +++---- 2 files changed, 62 insertions(+), 26 deletions(-) diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index 07df1b01d5e4..ed9b705ad59f 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'dart:io' as io; import 'package:colorize/colorize.dart'; @@ -29,23 +30,28 @@ 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' + argParser.addFlag(_machineFlag, + help: 'Only prints a final status as a defined string.\n' 'The possible values are:\n' - ' $_resultNeedsPublish: There is at least one package need to be published. They also passed all publish checks. \n' - ' $_resultNoPublish: There are no packages need to be published. Either no pubspec change detected or the version has already been published. \n' + ' $_resultNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' + ' $_resultNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' ' $_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 _machineFlag = 'machine'; static const String _resultNeedsPublish = 'needs-publish'; static const String _resultNoPublish = 'no-publish'; static const String _resultError = 'error'; + final List _validStatus = [ + _resultNeedsPublish, + _resultNoPublish, + _resultError + ]; + @override final String name = 'publish-check'; @@ -58,6 +64,19 @@ class PublishCheckCommand extends PluginCommand { @override Future run() async { + final ZoneSpecification logSwitchSpecification = ZoneSpecification( + print: (Zone self, ZoneDelegate parent, Zone zone, String message) { + final bool logMachineMessage = argResults[_machineFlag] as bool; + final bool isMachineMessage = _validStatus.contains(message); + if (logMachineMessage == isMachineMessage) { + parent.print(zone, message); + } + }); + + await runZoned(_runCommand, zoneSpecification: logSwitchSpecification); + } + + Future _runCommand() async { final List failedPackages = []; String resultToLog = _resultNoPublish; @@ -94,7 +113,7 @@ class PublishCheckCommand extends PluginCommand { } // This has to be last output of this command because it is promised in the help section. - if (argResults[_logStatusFlag] as bool) { + if (argResults[_machineFlag] as bool) { print(resultToLog); } if (failedPackages.isNotEmpty) { @@ -127,8 +146,11 @@ class PublishCheckCommand extends PluginCommand { final Completer stdOutCompleter = Completer(); process.stdout.listen( (List event) { - io.stdout.add(event); - outputBuffer.write(String.fromCharCodes(event)); + final String output = String.fromCharCodes(event); + if (output.isNotEmpty) { + print(output); + outputBuffer.write(output); + } }, onDone: () => stdOutCompleter.complete(), ); @@ -136,8 +158,11 @@ class PublishCheckCommand extends PluginCommand { final Completer stdInCompleter = Completer(); process.stderr.listen( (List event) { - io.stderr.add(event); - outputBuffer.write(String.fromCharCodes(event)); + final String output = String.fromCharCodes(event); + if (output.isNotEmpty) { + print(Colorize(output)..red()); + outputBuffer.write(output); + } }, onDone: () => stdInCompleter.complete(), ); @@ -197,9 +222,24 @@ class PublishCheckCommand extends PluginCommand { package: packageName, httpClient: httpClient ?? http.Client()); final PubVersionFinderResponse pubVersionFinderResponse = await pubVersionFinder.getPackageVersion(); + bool published; + switch (pubVersionFinderResponse.result) { + case PubVersionFinderResult.success: + published = pubVersionFinderResponse.versions.contains(version); + break; + case PubVersionFinderResult.fail: + printErrorAndExit(errorMessage: ''' +Error fetching version on pub for $packageName. +HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} +HTTP response: ${pubVersionFinderResponse.httpResponse.body} +'''); + break; + case PubVersionFinderResult.noPackageFound: + published = false; + break; + } pubVersionFinder.httpClient.close(); - return pubVersionFinderResponse.result == PubVersionFinderResult.success && - pubVersionFinderResponse.versions.contains(version); + return published; } } diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart index 5975074c2730..ef0bc11b43e8 100644 --- a/script/tool/test/publish_check_command_test.dart +++ b/script/tool/test/publish_check_command_test.dart @@ -131,7 +131,7 @@ void main() { }); test( - '--log-status: Log no_publish at the end if there are no packages need to be published. ', + '--machine: Log no_publish at the end if there are no packages need to be published. ', () async { const Map httpResponseA = { 'name': 'a', @@ -151,8 +151,6 @@ void main() { }; 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') { @@ -185,13 +183,13 @@ void main() { ); final List output = await runCapturingPrint( - runner, ['publish-check', '--log-status']); + runner, ['publish-check', '--machine']); - expect(output.last, 'no-publish'); + expect(output[0], 'no-publish'); }); test( - '--log-status: Log needs-publish at the end if there is at least 1 plugin needs to be published. ', + '--machine: Log needs-publish at the end if there is at least 1 plugin needs to be published. ', () async { const Map httpResponseA = { 'name': 'a', @@ -210,8 +208,6 @@ void main() { }; 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') { @@ -244,13 +240,13 @@ void main() { ); final List output = await runCapturingPrint( - runner, ['publish-check', '--log-status']); + runner, ['publish-check', '--machine']); - expect(output.last, 'needs-publish'); + expect(output[0], 'needs-publish'); }); test( - '--log-status: Log error at the end if there is at least 1 plugin contains error.', + '--machine: Log error at the end if there is at least 1 plugin contains error.', () async { const Map httpResponseA = { 'name': 'a', @@ -305,13 +301,13 @@ void main() { bool hasError = false; final List output = await runCapturingPrint( - runner, ['publish-check', '--log-status'], + runner, ['publish-check', '--machine'], errorHandler: (Error error) { expect(error, isA()); hasError = true; }); expect(hasError, isTrue); - expect(output.last, 'error'); + expect(output[0], 'error'); }); }); } From 4cd1f65aac8d4ec21a81d3a5b197e057a949e761 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 5 May 2021 11:13:54 -0700 Subject: [PATCH 10/14] review --- script/tool/CHANGELOG.md | 4 +- script/tool/lib/src/common.dart | 36 +++++++-------- .../tool/lib/src/publish_check_command.dart | 46 ++++++++++--------- .../tool/lib/src/version_check_command.dart | 24 ++++++---- script/tool/test/common_test.dart | 16 +++---- 5 files changed, 64 insertions(+), 62 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index b6f6aa56216b..9696e55d0662 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,7 +1,7 @@ ## NEXT -- Add `against-pub` flag for version-check. -- Add `log-status` flag for publish-check. +- Add `against-pub` flag for version-check, which allows the command to check version with pub. +- Add `machine` flag for publish-check, which replaces outputs to something parsable by machines. ## 0.1.0+1 diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index aed2d700bbbc..d1d49ae417e3 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -551,29 +551,27 @@ class ProcessRunner { } /// Finding version of [package] that is published on pub. -/// -/// Note: you should manually close the [httpClient] when done using the finder. class PubVersionFinder { - /// Constructor. - PubVersionFinder({@required this.package, this.pubHost = defaultPubHost, @required this.httpClient}); + /// + /// Note: you should manually close the [httpClient] when done using the finder. + PubVersionFinder({this.pubHost = defaultPubHost, @required this.httpClient}); /// The default pub host to use. static const String defaultPubHost = 'https://pub.dev'; - /// The package name. - final String package; - /// The pub host url, defaults to `https://pub.dev`. final String pubHost; - /// The http client, can override for testing. + /// The http client. /// /// You should manually close this client when done using this finder. final http.Client httpClient; /// Get the package version on pub. - Future getPackageVersion() async { + Future getPackageVersion( + {@required String package}) async { + assert(package != null && package.isNotEmpty); final Uri pubHostUri = Uri.parse(pubHost); final Uri url = pubHostUri.replace(path: '/packages/$package.json'); final http.Response response = await httpClient.get(url); @@ -591,15 +589,10 @@ class PubVersionFinder { } final List versions = (json.decode(response.body)['versions'] as List) - .map( - (final dynamic versionString) => Version.parse(versionString as String)) + .map((final dynamic versionString) => + Version.parse(versionString as String)) .toList(); - versions.sort((Version a, Version b) { - /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. - return b.compareTo(a); - }); - return PubVersionFinderResponse( versions: versions, result: PubVersionFinderResult.success, @@ -610,7 +603,12 @@ class PubVersionFinder { /// Represents a response for [PubVersionFinder]. class PubVersionFinderResponse { /// Constructor. - const PubVersionFinderResponse({this.versions, this.result, this.httpResponse}); + PubVersionFinderResponse({this.versions, this.result, this.httpResponse}) { + versions.sort((Version a, Version b) { + /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. + return b.compareTo(a); + }); + } /// The versions found in [PubVersionFinder]. /// @@ -625,7 +623,7 @@ class PubVersionFinderResponse { final http.Response httpResponse; } -/// An enum represents the result of [PubVersionFinder]. +/// An enum representing the result of [PubVersionFinder]. enum PubVersionFinderResult { /// The version finder successfully found a version. success, @@ -637,7 +635,7 @@ enum PubVersionFinderResult { /// The version finder failed to locate the package. /// - /// This is usually OK when the package is new. + /// This indicates the package is new. noPackageFound, } diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index ed9b705ad59f..f058aac06cdf 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -22,7 +22,9 @@ class PublishCheckCommand extends PluginCommand { FileSystem fileSystem, { ProcessRunner processRunner = const ProcessRunner(), this.httpClient, - }) : super(packagesDir, fileSystem, processRunner: processRunner) { + }) : _pubVersionFinder = + PubVersionFinder(httpClient: httpClient ?? http.Client()), + super(packagesDir, fileSystem, processRunner: processRunner) { argParser.addFlag( _allowPrereleaseFlag, help: 'Allows the pre-release SDK warning to pass.\n' @@ -33,23 +35,23 @@ class PublishCheckCommand extends PluginCommand { argParser.addFlag(_machineFlag, help: 'Only prints a final status as a defined string.\n' 'The possible values are:\n' - ' $_resultNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' - ' $_resultNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' - ' $_resultError: Some error has occurred.', + ' $_machineMessageNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' + ' $_machineMessageNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' + ' $_machineMessageError: Some error has occurred.', defaultsTo: false, negatable: true); } static const String _allowPrereleaseFlag = 'allow-pre-release'; static const String _machineFlag = 'machine'; - static const String _resultNeedsPublish = 'needs-publish'; - static const String _resultNoPublish = 'no-publish'; - static const String _resultError = 'error'; + static const String _machineMessageNeedsPublish = 'needs-publish'; + static const String _machineMessageNoPublish = 'no-publish'; + static const String _machineMessageError = 'error'; final List _validStatus = [ - _resultNeedsPublish, - _resultNoPublish, - _resultError + _machineMessageNeedsPublish, + _machineMessageNoPublish, + _machineMessageError ]; @override @@ -62,6 +64,8 @@ class PublishCheckCommand extends PluginCommand { /// The custom http client used to query versions on pub. final http.Client httpClient; + final PubVersionFinder _pubVersionFinder; + @override Future run() async { final ZoneSpecification logSwitchSpecification = ZoneSpecification( @@ -79,23 +83,24 @@ class PublishCheckCommand extends PluginCommand { Future _runCommand() async { final List failedPackages = []; - String resultToLog = _resultNoPublish; + String resultToLog = _machineMessageNoPublish; await for (final Directory plugin in getPlugins()) { final _PublishCheckResult result = await _passesPublishCheck(plugin); switch (result) { case _PublishCheckResult._needsPublish: if (failedPackages.isEmpty) { - resultToLog = _resultNeedsPublish; + resultToLog = _machineMessageNeedsPublish; } break; case _PublishCheckResult._noPublish: break; - case _PublishCheckResult.error: + case _PublishCheckResult._error: failedPackages.add(plugin); - resultToLog = _resultError; + resultToLog = _machineMessageError; break; } } + _pubVersionFinder.httpClient.close(); if (failedPackages.isNotEmpty) { final String error = @@ -112,7 +117,6 @@ class PublishCheckCommand extends PluginCommand { print(passedMessage); } - // This has to be last output of this command because it is promised in the help section. if (argResults[_machineFlag] as bool) { print(resultToLog); } @@ -191,7 +195,7 @@ class PublishCheckCommand extends PluginCommand { final Pubspec pubspec = _tryParsePubspec(package); if (pubspec == null) { print('no pubspec'); - return _PublishCheckResult.error; + return _PublishCheckResult._error; } else if (pubspec.publishTo == 'none') { print('Package $packageName is marked as unpublishable. Skipping.'); return _PublishCheckResult._noPublish; @@ -211,23 +215,22 @@ class PublishCheckCommand extends PluginCommand { return _PublishCheckResult._needsPublish; } else { print('Unable to publish $packageName'); - return _PublishCheckResult.error; + return _PublishCheckResult._error; } } // Check if `packageName` already has `version` published on pub. Future _checkIfAlreadyPublished( {String packageName, Version version}) async { - final PubVersionFinder pubVersionFinder = PubVersionFinder( - package: packageName, httpClient: httpClient ?? http.Client()); final PubVersionFinderResponse pubVersionFinderResponse = - await pubVersionFinder.getPackageVersion(); + await _pubVersionFinder.getPackageVersion(package: packageName); bool published; switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: published = pubVersionFinderResponse.versions.contains(version); break; case PubVersionFinderResult.fail: + print(_machineMessageError); printErrorAndExit(errorMessage: ''' Error fetching version on pub for $packageName. HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} @@ -238,7 +241,6 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body} published = false; break; } - pubVersionFinder.httpClient.close(); return published; } } @@ -248,5 +250,5 @@ enum _PublishCheckResult { _noPublish, - error, + _error, } diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 07708ea9318f..d3f16731e5ff 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -76,7 +76,9 @@ class VersionCheckCommand extends PluginCommand { ProcessRunner processRunner = const ProcessRunner(), GitDir gitDir, this.httpClient, - }) : super(packagesDir, fileSystem, + }) : _pubVersionFinder = + PubVersionFinder(httpClient: httpClient ?? http.Client()), + super(packagesDir, fileSystem, processRunner: processRunner, gitDir: gitDir) { argParser.addFlag( _againstPubFlag, @@ -101,6 +103,8 @@ class VersionCheckCommand extends PluginCommand { /// The http client used to query pub server. final http.Client httpClient; + final PubVersionFinder _pubVersionFinder; + @override Future run() async { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); @@ -133,15 +137,13 @@ class VersionCheckCommand extends PluginCommand { Version sourceVersion; if (argResults[_againstPubFlag] as bool) { final String packageName = pubspecFile.parent.basename; - final PubVersionFinder pubVersionFinder = PubVersionFinder( - package: packageName, httpClient: httpClient ?? http.Client()); final PubVersionFinderResponse pubVersionFinderResponse = - await pubVersionFinder.getPackageVersion(); - pubVersionFinder.httpClient.close(); + await _pubVersionFinder.getPackageVersion(package: packageName); switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: sourceVersion = pubVersionFinderResponse.versions.first; - print('$indentation$packageName: Current largest version on pub: $sourceVersion'); + print( + '$indentation$packageName: Current largest version on pub: $sourceVersion'); break; case PubVersionFinderResult.fail: printErrorAndExit(errorMessage: ''' @@ -160,9 +162,11 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} if (sourceVersion == null) { String safeToIgnoreMessage; if (argResults[_againstPubFlag] as bool) { - safeToIgnoreMessage = '${indentation}Unable to find package on pub server.'; + safeToIgnoreMessage = + '${indentation}Unable to find package on pub server.'; } else { - safeToIgnoreMessage = '${indentation}Unable to find pubspec in master.'; + safeToIgnoreMessage = + '${indentation}Unable to find pubspec in master.'; } print('$safeToIgnoreMessage Safe to ignore if the project is new.'); continue; @@ -177,7 +181,8 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} getAllowedNextVersions(sourceVersion, headVersion); if (!allowedNextVersions.containsKey(headVersion)) { - final String source = (argResults[_againstPubFlag] as bool) ? 'pub' : 'master'; + final String source = + (argResults[_againstPubFlag] as bool) ? 'pub' : 'master'; final String error = '${indentation}Incorrectly updated version.\n' '${indentation}HEAD: $headVersion, $source: $sourceVersion.\n' '${indentation}Allowed versions: $allowedNextVersions'; @@ -199,6 +204,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} await for (final Directory plugin in getPlugins()) { await _checkVersionsMatch(plugin); } + _pubVersionFinder.httpClient.close(); print('No version check errors found!'); } diff --git a/script/tool/test/common_test.dart b/script/tool/test/common_test.dart index ac5e04cf20f6..bb90427626f3 100644 --- a/script/tool/test/common_test.dart +++ b/script/tool/test/common_test.dart @@ -306,15 +306,13 @@ file2/file2.cc }); group('$PubVersionFinder', () { - test('Package does not exist.', () async { final MockClient mockClient = MockClient((http.Request request) async { return http.Response('', 404); }); - final PubVersionFinder finder = - PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinder finder = PubVersionFinder(httpClient: mockClient); final PubVersionFinderResponse response = - await finder.getPackageVersion(); + await finder.getPackageVersion(package: 'some_package'); expect(response.versions, isNull); expect(response.result, PubVersionFinderResult.noPackageFound); @@ -326,10 +324,9 @@ file2/file2.cc final MockClient mockClient = MockClient((http.Request request) async { return http.Response('', 400); }); - final PubVersionFinder finder = - PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinder finder = PubVersionFinder(httpClient: mockClient); final PubVersionFinderResponse response = - await finder.getPackageVersion(); + await finder.getPackageVersion(package: 'some_package'); expect(response.versions, isNull); expect(response.result, PubVersionFinderResult.fail); @@ -358,10 +355,9 @@ file2/file2.cc final MockClient mockClient = MockClient((http.Request request) async { return http.Response(json.encode(httpResponse), 200); }); - final PubVersionFinder finder = - PubVersionFinder(package: 'some_package', httpClient: mockClient); + final PubVersionFinder finder = PubVersionFinder(httpClient: mockClient); final PubVersionFinderResponse response = - await finder.getPackageVersion(); + await finder.getPackageVersion(package: 'some_package'); expect(response.versions, [ Version.parse('2.0.0'), From 32592410b13200d75c3ecb773a6937ac44dabfcf Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 5 May 2021 21:34:32 -0700 Subject: [PATCH 11/14] review --- script/tool/lib/src/common.dart | 10 +- .../tool/lib/src/publish_check_command.dart | 94 ++++++++++++------- .../tool/test/publish_check_command_test.dart | 50 ++++++++-- 3 files changed, 109 insertions(+), 45 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index d1d49ae417e3..4c07cd3b6c16 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -604,10 +604,12 @@ class PubVersionFinder { class PubVersionFinderResponse { /// Constructor. PubVersionFinderResponse({this.versions, this.result, this.httpResponse}) { - versions.sort((Version a, Version b) { - /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. - return b.compareTo(a); - }); + if (versions != null && versions.isNotEmpty) { + versions.sort((Version a, Version b) { + /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. + return b.compareTo(a); + }); + } } /// The versions found in [PubVersionFinder]. diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index f058aac06cdf..aed018fa5935 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -33,25 +33,27 @@ class PublishCheckCommand extends PluginCommand { defaultsTo: false, ); argParser.addFlag(_machineFlag, - help: 'Only prints a final status as a defined string.\n' - 'The possible values are:\n' - ' $_machineMessageNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' - ' $_machineMessageNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' - ' $_machineMessageError: Some error has occurred.', + help: 'Switch outputs to a machine readable JSON. \n' + 'The JSON contains a "status" field indicating the final status of the command, the possible values are:\n' + ' $_statusNeedsPublish: There is at least one package need to be published. They also passed all publish checks.\n' + ' $_statusMessageNoPublish: There are no packages needs to be published. Either no pubspec change detected or all versions have already been published.\n' + ' $_statusMessageError: Some error has occurred.', defaultsTo: false, negatable: true); } static const String _allowPrereleaseFlag = 'allow-pre-release'; static const String _machineFlag = 'machine'; - static const String _machineMessageNeedsPublish = 'needs-publish'; - static const String _machineMessageNoPublish = 'no-publish'; - static const String _machineMessageError = 'error'; + static const String _statusNeedsPublish = 'needs-publish'; + static const String _statusMessageNoPublish = 'no-publish'; + static const String _statusMessageError = 'error'; + static const String _statusKey = 'status'; + static const String _humanMessageKey = 'humanMessage'; final List _validStatus = [ - _machineMessageNeedsPublish, - _machineMessageNoPublish, - _machineMessageError + _statusNeedsPublish, + _statusMessageNoPublish, + _statusMessageError ]; @override @@ -66,13 +68,19 @@ class PublishCheckCommand extends PluginCommand { final PubVersionFinder _pubVersionFinder; + // The output JSON when the _machineFlag is on. + final Map _machineOutput = {}; + + final List _humanMessages = []; + @override Future run() async { final ZoneSpecification logSwitchSpecification = ZoneSpecification( print: (Zone self, ZoneDelegate parent, Zone zone, String message) { final bool logMachineMessage = argResults[_machineFlag] as bool; - final bool isMachineMessage = _validStatus.contains(message); - if (logMachineMessage == isMachineMessage) { + if (logMachineMessage && message != _prettyJson(_machineOutput)) { + _humanMessages.add(message); + } else { parent.print(zone, message); } }); @@ -83,20 +91,20 @@ class PublishCheckCommand extends PluginCommand { Future _runCommand() async { final List failedPackages = []; - String resultToLog = _machineMessageNoPublish; + String status = _statusMessageNoPublish; await for (final Directory plugin in getPlugins()) { final _PublishCheckResult result = await _passesPublishCheck(plugin); switch (result) { - case _PublishCheckResult._needsPublish: + case _PublishCheckResult._notPublished: if (failedPackages.isEmpty) { - resultToLog = _machineMessageNeedsPublish; + status = _statusNeedsPublish; } break; - case _PublishCheckResult._noPublish: + case _PublishCheckResult._published: break; case _PublishCheckResult._error: failedPackages.add(plugin); - resultToLog = _machineMessageError; + status = _statusMessageError; break; } } @@ -118,8 +126,11 @@ class PublishCheckCommand extends PluginCommand { } if (argResults[_machineFlag] as bool) { - print(resultToLog); + setStatus(status); + _machineOutput[_humanMessageKey] = _humanMessages; + print(_prettyJson(_machineOutput)); } + if (failedPackages.isNotEmpty) { throw ToolExit(1); } @@ -198,21 +209,25 @@ class PublishCheckCommand extends PluginCommand { return _PublishCheckResult._error; } else if (pubspec.publishTo == 'none') { print('Package $packageName is marked as unpublishable. Skipping.'); - return _PublishCheckResult._noPublish; + return _PublishCheckResult._published; } final Version version = pubspec.version; - final bool alreadyPublished = await _checkIfAlreadyPublished( - packageName: packageName, version: version); - if (alreadyPublished) { + final _PublishCheckResult alreadyPublishedResult = + await _checkIfAlreadyPublished( + packageName: packageName, version: version); + if (alreadyPublishedResult == _PublishCheckResult._published) { print( 'Package $packageName version: $version has already be published on pub.'); - return _PublishCheckResult._noPublish; + return alreadyPublishedResult; + } else if (alreadyPublishedResult == _PublishCheckResult._error) { + print('Check pub version failed $packageName'); + return _PublishCheckResult._error; } if (await _hasValidPublishCheckRun(package)) { print('Package $packageName is able to be published.'); - return _PublishCheckResult._needsPublish; + return _PublishCheckResult._notPublished; } else { print('Unable to publish $packageName'); return _PublishCheckResult._error; @@ -220,35 +235,46 @@ class PublishCheckCommand extends PluginCommand { } // Check if `packageName` already has `version` published on pub. - Future _checkIfAlreadyPublished( + Future<_PublishCheckResult> _checkIfAlreadyPublished( {String packageName, Version version}) async { final PubVersionFinderResponse pubVersionFinderResponse = await _pubVersionFinder.getPackageVersion(package: packageName); - bool published; + _PublishCheckResult result; switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: - published = pubVersionFinderResponse.versions.contains(version); + result = pubVersionFinderResponse.versions.contains(version) + ? _PublishCheckResult._published + : _PublishCheckResult._notPublished; break; case PubVersionFinderResult.fail: - print(_machineMessageError); - printErrorAndExit(errorMessage: ''' + print(''' Error fetching version on pub for $packageName. HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); + result = _PublishCheckResult._error; break; case PubVersionFinderResult.noPackageFound: - published = false; + result = _PublishCheckResult._notPublished; break; } - return published; + return result; + } + + void setStatus(String status) { + assert(_validStatus.contains(status)); + _machineOutput[_statusKey] = status; + } + + String _prettyJson(Map map) { + return const JsonEncoder.withIndent(' ').convert(_machineOutput); } } enum _PublishCheckResult { - _needsPublish, + _notPublished, - _noPublish, + _published, _error, } diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart index ef0bc11b43e8..dee5164d32be 100644 --- a/script/tool/test/publish_check_command_test.dart +++ b/script/tool/test/publish_check_command_test.dart @@ -131,7 +131,7 @@ void main() { }); test( - '--machine: Log no_publish at the end if there are no packages need to be published. ', + '--machine: Log JSON with status:no-publish and correct human message, if there are no packages need to be published. ', () async { const Map httpResponseA = { 'name': 'a', @@ -181,15 +181,25 @@ void main() { processRunner.processesToReturn.add( MockProcess()..exitCodeCompleter.complete(0), ); - final List output = await runCapturingPrint( runner, ['publish-check', '--machine']); - expect(output[0], 'no-publish'); + // ignore: use_raw_strings + expect(output.first, ''' +{ + "status": "no-publish", + "humanMessage": [ + "Checking that no_publish_a can be published.", + "Package no_publish_a version: 0.1.0 has already be published on pub.", + "Checking that no_publish_b can be published.", + "Package no_publish_b version: 0.2.0 has already be published on pub.", + "\\u001b[32mAll packages passed publish check!\\u001b[0m" + ] +}'''); }); test( - '--machine: Log needs-publish at the end if there is at least 1 plugin needs to be published. ', + '--machine: Log JSON with status:needs-publish and correct human message, if there is at least 1 plugin needs to be published.', () async { const Map httpResponseA = { 'name': 'a', @@ -242,11 +252,22 @@ void main() { final List output = await runCapturingPrint( runner, ['publish-check', '--machine']); - expect(output[0], 'needs-publish'); + // ignore: use_raw_strings + expect(output.first, ''' +{ + "status": "needs-publish", + "humanMessage": [ + "Checking that no_publish_a can be published.", + "Package no_publish_a version: 0.1.0 has already be published on pub.", + "Checking that no_publish_b can be published.", + "Package no_publish_b is able to be published.", + "\\u001b[32mAll packages passed publish check!\\u001b[0m" + ] +}'''); }); test( - '--machine: Log error at the end if there is at least 1 plugin contains error.', + '--machine: Log correct JSON, if there is at least 1 plugin contains error.', () async { const Map httpResponseA = { 'name': 'a', @@ -307,7 +328,22 @@ void main() { hasError = true; }); expect(hasError, isTrue); - expect(output[0], 'error'); + + // ignore: use_raw_strings + expect(output.first, ''' +{ + "status": "error", + "humanMessage": [ + "Checking that no_publish_a can be published.", + "Failed to parse `pubspec.yaml` at /packages/no_publish_a/pubspec.yaml: ParsedYamlException: line 1, column 1: Not a map\\n ╷\\n1 │ bad-yaml\\n │ ^^^^^^^^\\n ╵}", + "no pubspec", + "Checking that no_publish_b can be published.", + "url https://pub.dev/packages/no_publish_b.json", + "no_publish_b.json", + "Package no_publish_b is able to be published.", + "\\u001b[31mFAIL: The following 1 package(s) failed the publishing check:\\nMemoryDirectory: '/packages/no_publish_a'\\u001b[0m" + ] +}'''); }); }); } From 3242c8b0462c3eadb2665ab1c49b306569957fa7 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 10 May 2021 09:27:37 -0700 Subject: [PATCH 12/14] review --- .../tool/lib/src/publish_check_command.dart | 33 ++++++++++++------- .../tool/test/publish_check_command_test.dart | 6 ++-- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index aed018fa5935..676b76bb6ce5 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -9,6 +9,7 @@ import 'dart:io' as io; import 'package:colorize/colorize.dart'; import 'package:file/file.dart'; import 'package:http/http.dart' as http; +import 'package:meta/meta.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; @@ -112,21 +113,16 @@ class PublishCheckCommand extends PluginCommand { if (failedPackages.isNotEmpty) { final String error = - 'FAIL: The following ${failedPackages.length} package(s) failed the ' + 'The following ${failedPackages.length} package(s) failed the ' 'publishing check:'; final String joinedFailedPackages = failedPackages.join('\n'); - - final Colorize colorizedError = Colorize('$error\n$joinedFailedPackages') - ..red(); - print(colorizedError); + _printImportantStatusMessage('$error\n$joinedFailedPackages', isError: true); } else { - final Colorize passedMessage = - Colorize('All packages passed publish check!')..green(); - print(passedMessage); + _printImportantStatusMessage('All packages passed publish check!', isError: false); } if (argResults[_machineFlag] as bool) { - setStatus(status); + _setStatus(status); _machineOutput[_humanMessageKey] = _humanMessages; print(_prettyJson(_machineOutput)); } @@ -175,7 +171,7 @@ class PublishCheckCommand extends PluginCommand { (List event) { final String output = String.fromCharCodes(event); if (output.isNotEmpty) { - print(Colorize(output)..red()); + _printImportantStatusMessage(output, isError: true); outputBuffer.write(output); } }, @@ -261,7 +257,7 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body} return result; } - void setStatus(String status) { + void _setStatus(String status) { assert(_validStatus.contains(status)); _machineOutput[_statusKey] = status; } @@ -269,6 +265,21 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body} String _prettyJson(Map map) { return const JsonEncoder.withIndent(' ').convert(_machineOutput); } + + void _printImportantStatusMessage(String message, {@required bool isError}) { + final String statusMessage = '${isError ? 'ERROR' : 'SUCCESS'}: $message'; + if (argResults[_machineFlag] as bool) { + print(statusMessage); + } else { + final Colorize colorizedMessage = Colorize(statusMessage); + if (isError) { + colorizedMessage.red(); + } else { + colorizedMessage.green(); + } + print(colorizedMessage); + } + } } enum _PublishCheckResult { diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart index dee5164d32be..eca7caf53403 100644 --- a/script/tool/test/publish_check_command_test.dart +++ b/script/tool/test/publish_check_command_test.dart @@ -193,7 +193,7 @@ void main() { "Package no_publish_a version: 0.1.0 has already be published on pub.", "Checking that no_publish_b can be published.", "Package no_publish_b version: 0.2.0 has already be published on pub.", - "\\u001b[32mAll packages passed publish check!\\u001b[0m" + "SUCCESS: All packages passed publish check!" ] }'''); }); @@ -261,7 +261,7 @@ void main() { "Package no_publish_a version: 0.1.0 has already be published on pub.", "Checking that no_publish_b can be published.", "Package no_publish_b is able to be published.", - "\\u001b[32mAll packages passed publish check!\\u001b[0m" + "SUCCESS: All packages passed publish check!" ] }'''); }); @@ -341,7 +341,7 @@ void main() { "url https://pub.dev/packages/no_publish_b.json", "no_publish_b.json", "Package no_publish_b is able to be published.", - "\\u001b[31mFAIL: The following 1 package(s) failed the publishing check:\\nMemoryDirectory: '/packages/no_publish_a'\\u001b[0m" + "ERROR: The following 1 package(s) failed the publishing check:\\nMemoryDirectory: '/packages/no_publish_a'" ] }'''); }); From eb5ee5c0b1772c0575f493ddf6e5c3ed2bda2ddb Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 10 May 2021 09:27:49 -0700 Subject: [PATCH 13/14] dartfmt --- script/tool/lib/src/publish_check_command.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index 676b76bb6ce5..84503f4540c6 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -116,9 +116,11 @@ class PublishCheckCommand extends PluginCommand { 'The following ${failedPackages.length} package(s) failed the ' 'publishing check:'; final String joinedFailedPackages = failedPackages.join('\n'); - _printImportantStatusMessage('$error\n$joinedFailedPackages', isError: true); + _printImportantStatusMessage('$error\n$joinedFailedPackages', + isError: true); } else { - _printImportantStatusMessage('All packages passed publish check!', isError: false); + _printImportantStatusMessage('All packages passed publish check!', + isError: false); } if (argResults[_machineFlag] as bool) { From f083fe436b883f27a5ccb270ac26a32b36c8f4e1 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 10 May 2021 14:38:26 -0700 Subject: [PATCH 14/14] analyze --- script/tool/lib/src/common.dart | 3 ++- script/tool/lib/src/version_check_command.dart | 2 +- script/tool/test/common_test.dart | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index 33bef084bbb3..1e864fcbc38a 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -620,7 +620,8 @@ class PubVersionFinderResponse { PubVersionFinderResponse({this.versions, this.result, this.httpResponse}) { if (versions != null && versions.isNotEmpty) { versions.sort((Version a, Version b) { - /// TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. + // TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. + // https://github.com/flutter/flutter/issues/82222 return b.compareTo(a); }); } diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index 93391db8f492..475caf5d285d 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -147,7 +147,7 @@ class VersionCheckCommand extends PluginCommand { break; case PubVersionFinderResult.fail: printErrorAndExit(errorMessage: ''' -${indentation}Error fetching version on pub for ${packageName}. +${indentation}Error fetching version on pub for $packageName. ${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); diff --git a/script/tool/test/common_test.dart b/script/tool/test/common_test.dart index 0ff66cbd06a7..d6ac449e7fd3 100644 --- a/script/tool/test/common_test.dart +++ b/script/tool/test/common_test.dart @@ -9,9 +9,9 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common.dart'; +import 'package:git/git.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; -import 'package:git/git.dart'; import 'package:mockito/mockito.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart';