Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 7 additions & 65 deletions script/tool/lib/src/version_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

import 'package:file/file.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';
Expand All @@ -12,7 +11,6 @@ import 'common/git_version_finder.dart';
import 'common/output_utils.dart';
import 'common/package_looping_command.dart';
import 'common/package_state_utils.dart';
import 'common/pub_version_finder.dart';
import 'common/repository_package.dart';

/// Categories of version change types.
Expand Down Expand Up @@ -98,17 +96,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
super.processRunner,
super.platform,
super.gitDir,
http.Client? httpClient,
}) : _pubVersionFinder = PubVersionFinder(
httpClient: httpClient ?? http.Client(),
) {
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.',
);
}) {
argParser.addOption(
_prLabelsArg,
help:
Expand Down Expand Up @@ -140,7 +128,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
);
}

static const String _againstPubFlag = 'against-pub';
static const String _prLabelsArg = 'pr-labels';
static const String _checkForMissingChanges = 'check-for-missing-changes';
static const String _ignorePlatformInterfaceBreaks =
Expand All @@ -161,8 +148,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
static const String _missingChangelogChangeOverrideLabel =
'override: no changelog needed';

final PubVersionFinder _pubVersionFinder;

late final GitVersionFinder _gitVersionFinder;

late final Set<String> _prLabels = _getPRLabels();
Expand All @@ -184,7 +169,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
final String description =
'Checks if the versions of packages have been incremented per pub specification.\n'
'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.';
'This command requires "flutter" to be in your path.';

@override
bool get hasLongOutput => false;
Expand Down Expand Up @@ -303,33 +288,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
: PackageResult.fail(errors);
}

@override
Future<void> completeRun() async {
_pubVersionFinder.httpClient.close();
}

/// Returns the previous published version of [package].
///
/// [packageName] must be the actual name of the package as published (i.e.,
/// the name from pubspec.yaml, not the on disk name if different.)
Future<Version?> _fetchPreviousVersionFromPub(String packageName) async {
final PubVersionFinderResponse pubVersionFinderResponse =
await _pubVersionFinder.getPackageVersion(packageName: packageName);
switch (pubVersionFinderResponse.result) {
case PubVersionFinderResult.success:
return pubVersionFinderResponse.versions.first;
case PubVersionFinderResult.fail:
printError('''
${indentation}Error fetching version on pub for $packageName.
${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
''');
return null;
case PubVersionFinderResult.noPackageFound:
return Version.none;
}
}

/// Returns the version of [package] from git at the base comparison hash.
Future<Version?> _getPreviousVersionFromGit(RepositoryPackage package) async {
final File pubspecFile = package.pubspecFile;
Expand All @@ -352,28 +310,12 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
}) async {
// This method isn't called unless `version` is non-null.
final Version currentVersion = pubspec.version!;
Version? previousVersion;
String previousVersionSource;
if (getBoolArg(_againstPubFlag)) {
previousVersionSource = 'pub';
previousVersion = await _fetchPreviousVersionFromPub(pubspec.name);
if (previousVersion == null) {
return _CurrentVersionState.unknown;
}
if (previousVersion != Version.none) {
print(
'$indentation${pubspec.name}: Current largest version on pub: $previousVersion',
);
}
} else {
previousVersionSource = baseSha;
previousVersion =
await _getPreviousVersionFromGit(package) ?? Version.none;
}
final Version previousVersion =
await _getPreviousVersionFromGit(package) ?? Version.none;
if (previousVersion == Version.none) {
print(
'${indentation}Unable to find previous version '
'${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.',
'at git base.',
);
logWarning(
'${indentation}If this package is not new, something has gone wrong.',
Expand All @@ -387,7 +329,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
}

// Check for reverts when doing local validation.
if (!getBoolArg(_againstPubFlag) && currentVersion < previousVersion) {
if (currentVersion < previousVersion) {
// Since this skips validation, try to ensure that it really is likely
// to be a revert rather than a typo by checking that the transition
// from the lower version to the new version would have been valid.
Expand All @@ -414,7 +356,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
} else {
printError(
'${indentation}Incorrectly updated version.\n'
'${indentation}HEAD: $currentVersion, $previousVersionSource: $previousVersion.\n'
'${indentation}HEAD: $currentVersion, $baseSha: $previousVersion.\n'
'${indentation}Allowed versions: $allowedNextVersions',
);
return _CurrentVersionState.invalidChange;
Expand Down
133 changes: 3 additions & 130 deletions script/tool/test/version_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';

import 'package:args/command_runner.dart';
import 'package:file/file.dart';
import 'package:flutter_plugin_tools/src/common/core.dart';
import 'package:flutter_plugin_tools/src/version_check_command.dart';
import 'package:git/git.dart';
import 'package:http/http.dart' as http;
import 'package:http/testing.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:test/test.dart';

Expand Down Expand Up @@ -40,15 +36,11 @@ void testAllowedVersion(
}

void main() {
const indentation = ' ';
group('VersionCheckCommand', () {
late MockPlatform mockPlatform;
late Directory packagesDir;
late CommandRunner<void> runner;
late RecordingProcessRunner gitProcessRunner;
// Ignored if mockHttpResponse is set.
int mockHttpStatus;
Map<String, dynamic>? mockHttpResponse;

setUp(() {
mockPlatform = MockPlatform();
Expand All @@ -57,22 +49,11 @@ void main() {
(:packagesDir, :processRunner, :gitProcessRunner, :gitDir) =
configureBaseCommandMocks(platform: mockPlatform);

// Default to simulating the plugin never having been published.
mockHttpStatus = 404;
mockHttpResponse = null;
final mockClient = MockClient((http.Request request) async {
return http.Response(
json.encode(mockHttpResponse),
mockHttpResponse == null ? mockHttpStatus : 200,
);
});

final command = VersionCheckCommand(
packagesDir,
processRunner: processRunner,
platform: mockPlatform,
gitDir: gitDir,
httpClient: mockClient,
);

runner = CommandRunner<void>(
Expand Down Expand Up @@ -448,7 +429,7 @@ void main() {
Error? commandError;
final List<String> output = await runCapturingPrint(
runner,
<String>['version-check', '--base-sha=main', '--against-pub'],
<String>['version-check', '--base-sha=main'],
errorHandler: (Error e) {
commandError = e;
},
Expand Down Expand Up @@ -594,7 +575,7 @@ void main() {
var hasError = false;
final List<String> output = await runCapturingPrint(
runner,
<String>['version-check', '--base-sha=main', '--against-pub'],
<String>['version-check', '--base-sha=main'],
errorHandler: (Error e) {
expect(e, isA<ToolExit>());
hasError = true;
Expand Down Expand Up @@ -669,7 +650,7 @@ void main() {
var hasError = false;
final List<String> output = await runCapturingPrint(
runner,
<String>['version-check', '--base-sha=main', '--against-pub'],
<String>['version-check', '--base-sha=main'],
errorHandler: (Error e) {
expect(e, isA<ToolExit>());
hasError = true;
Expand Down Expand Up @@ -1652,114 +1633,6 @@ packages/plugin/lib/plugin.dart
});
});

test('allows valid against pub', () async {
mockHttpResponse = <String, dynamic>{
'name': 'some_package',
'versions': <String>['0.0.1', '0.0.2', '1.0.0'],
};

createFakePlugin('plugin', packagesDir, version: '2.0.0');
final List<String> output = await runCapturingPrint(runner, <String>[
'version-check',
'--base-sha=main',
'--against-pub',
]);

expect(
output,
containsAllInOrder(<Matcher>[
contains('plugin: Current largest version on pub: 1.0.0'),
]),
);
});

test('denies invalid against pub', () async {
mockHttpResponse = <String, dynamic>{
'name': 'some_package',
'versions': <String>['0.0.1', '0.0.2'],
};

createFakePlugin('plugin', packagesDir, version: '2.0.0');

var hasError = false;
final List<String> result = await runCapturingPrint(
runner,
<String>['version-check', '--base-sha=main', '--against-pub'],
errorHandler: (Error e) {
expect(e, isA<ToolExit>());
hasError = true;
},
);
expect(hasError, isTrue);

expect(
result,
containsAllInOrder(<Matcher>[
contains(
'''
${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}''',
),
]),
);
});

test(
'throw and print error message if http request failed when checking against pub',
() async {
mockHttpStatus = 400;

createFakePlugin('plugin', packagesDir, version: '2.0.0');
var hasError = false;
final List<String> result = await runCapturingPrint(
runner,
<String>['version-check', '--base-sha=main', '--against-pub'],
errorHandler: (Error e) {
expect(e, isA<ToolExit>());
hasError = true;
},
);
expect(hasError, isTrue);

expect(
result,
containsAllInOrder(<Matcher>[
contains('''
${indentation}Error fetching version on pub for plugin.
${indentation}HTTP Status 400
${indentation}HTTP response: null
'''),
]),
);
},
);

test(
'when checking against pub, allow any version if http status is 404.',
() async {
mockHttpStatus = 404;

createFakePlugin('plugin', packagesDir, version: '2.0.0');
gitProcessRunner.mockProcessesForExecutable['git-show'] =
<FakeProcessInfo>[
FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')),
];
final List<String> result = await runCapturingPrint(runner, <String>[
'version-check',
'--base-sha=main',
'--against-pub',
]);

expect(
result,
containsAllInOrder(<Matcher>[
contains('Unable to find previous version on pub server.'),
]),
);
},
);

group('prelease versions', () {
test(
'allow an otherwise-valid transition that also adds a pre-release component',
Expand Down
Loading