flutter/shorebird
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(shorebird_cli): always respect build-name and build-number args (#2302)

Browse Source
This commit is contained in:
Bryan Oltman
2024-07-01 11:02:33 -04:00
committed by GitHub
parent 8e0f6ba2f9
commit eecb7b1915
11 changed files with 213 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/shorebird_cli/lib/src/commands/patch/patch_command.dart
+8 -10
View File
@@ -47,18 +47,16 @@ class PatchCommand extends ShorebirdCommand {
abbr: 'p',
help: 'The platform(s) to to build this release for.',
allowed: ReleaseType.values.map((e) => e.cliName).toList(),
// TODO(bryanoltman): uncomment this once https://github.com/dart-lang/args/pull/273 lands
// mandatory: true.
)
..addOption(
'build-number',
help: '''
An identifier used as an internal version number.
Each build must have a unique identifier to differentiate it from previous builds.
It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
On Android it is used as "versionCode".
On Xcode builds it is used as "CFBundleVersion".''',
defaultsTo: '1.0',
CommonArguments.buildNameArg.name,
help: CommonArguments.buildNameArg.description,
defaultsTo: CommonArguments.buildNameArg.defaultValue,
)
..addOption(
CommonArguments.buildNumberArg.name,
help: CommonArguments.buildNumberArg.description,
defaultsTo: CommonArguments.buildNumberArg.defaultValue,
)
..addOption(
'target',
packages/shorebird_cli/lib/src/commands/patch/patcher.dart
+16 -3
View File
@@ -6,6 +6,8 @@ import 'package:args/args.dart';
import 'package:mason_logger/mason_logger.dart';
import 'package:path/path.dart' as p;
import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
import 'package:shorebird_cli/src/common_arguments.dart';
import 'package:shorebird_cli/src/extensions/iterable.dart';
import 'package:shorebird_cli/src/patch_diff_checker.dart';
import 'package:shorebird_cli/src/platform/platform.dart';
import 'package:shorebird_cli/src/release_type.dart';
@@ -139,10 +141,21 @@ https://docs.shorebird.dev/status#link-percentage-ios
return [];
}
// If the user already provided --build-name or --build-number, we don't
// want to override them.
// If the user provided --build-name or --build-number before the --, we
// don't want to override them.
if (argResults.options.containsAnyOf([
CommonArguments.buildNameArg.name,
CommonArguments.buildNumberArg.name,
])) {
return [];
}
// If the user provided --build-name or --build-number after the --, we
// don't want to override them.
if (argResults.rest.any(
(a) => a.startsWith('--build-name') || a.startsWith('--build-number'),
(a) =>
a.startsWith('--${CommonArguments.buildNameArg.name}') ||
a.startsWith('--${CommonArguments.buildNumberArg.name}'),
)) {
return [];
}
packages/shorebird_cli/lib/src/commands/release/release_command.dart
+8 -8
View File
@@ -49,14 +49,14 @@ class ReleaseCommand extends ShorebirdCommand {
help: 'The product flavor to use when building the app.',
)
..addOption(
'build-number',
help: '''
An identifier used as an internal version number.
Each build must have a unique identifier to differentiate it from previous builds.
It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
On Android it is used as "versionCode".
On Xcode builds it is used as "CFBundleVersion".''',
defaultsTo: '1.0',
CommonArguments.buildNameArg.name,
help: CommonArguments.buildNameArg.description,
defaultsTo: CommonArguments.buildNameArg.defaultValue,
)
..addOption(
CommonArguments.buildNumberArg.name,
help: CommonArguments.buildNumberArg.description,
defaultsTo: CommonArguments.buildNumberArg.defaultValue,
)
..addFlag(
'codesign',
packages/shorebird_cli/lib/src/common_arguments.dart
+30
View File
@@ -8,6 +8,7 @@ class ArgumentDescriber {
const ArgumentDescriber({
required this.name,
required this.description,
this.defaultValue,
});
/// Argument name as how the user writes it.
@@ -15,11 +16,40 @@ class ArgumentDescriber {
/// Argument description that will be shown in the help of the command.
final String description;
/// Default value for this argument. Only provide this if the default value
/// holds across all commands.
final String? defaultValue;
}
/// A class that houses the name of arguments that are shared between different
/// commands and layers.
class CommonArguments {
/// The Flutter --build-name argument.
static const buildNameArg = ArgumentDescriber(
name: 'build-name',
description: '''
An identifier used as an internal version number.
Each build must have a unique identifier to differentiate it from previous builds.
It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
On Android it is used as "versionCode".
On Xcode builds it is used as "CFBundleVersion".''',
);
/// The Flutter --build-number argument.
static const buildNumberArg = ArgumentDescriber(
name: 'build-number',
description: '''
A "x.y.z" string used as the version number shown to users.
For each new version of your app, you will provide a version number to differentiate it
from previous versions.
On Android it is used as "versionName".
On Xcode builds it is used as "CFBundleShortVersionString".
On Windows it is used as the major, minor, and patch parts of the product and file
versions.''',
defaultValue: '1.0',
);
/// A multioption argument that defines constants for the built application.
/// These are forwarded to Flutter.
static const dartDefineArg = ArgumentDescriber(
packages/shorebird_cli/lib/src/extensions/arg_results.dart
+26 -14
View File
@@ -105,6 +105,24 @@ extension ForwardedArgs on ArgResults {
bool _isPositionalArgPlatform(String arg) =>
ReleaseType.values.any((target) => target.cliName == arg);
/// All parsed args with the given name. Because of multioptions, there may
/// be multiple values for a single name, so we return a potentially empty
/// [Iterable<String>] instead of a [String?].
Iterable<String> _argsNamed(String name) {
if (!wasParsed(name)) {
return [];
}
final value = this[name];
if (value is List) {
return value.map((a) => '--$name=$a');
} else {
return ['--$name=$value'];
}
}
/// A list of arguments parsed by Shorebird commands that will be forwarded
/// to the underlying Flutter commands (that is, placed after `--`).
List<String> get forwardedArgs {
final List<String> forwarded;
if (rest.isNotEmpty && _isPositionalArgPlatform(rest.first)) {
@@ -113,20 +131,14 @@ extension ForwardedArgs on ArgResults {
forwarded = rest.toList();
}
if (wasParsed(CommonArguments.dartDefineArg.name)) {
forwarded.addAll(
(this[CommonArguments.dartDefineArg.name] as List<String>).map(
(a) => '--${CommonArguments.dartDefineArg.name}=$a',
),
);
}
if (wasParsed(CommonArguments.dartDefineFromFileArg.name)) {
forwarded.addAll(
(this[CommonArguments.dartDefineFromFileArg.name] as List<String>)
.map((a) => '--${CommonArguments.dartDefineFromFileArg.name}=$a'),
);
}
forwarded.addAll(
[
..._argsNamed(CommonArguments.dartDefineArg.name),
..._argsNamed(CommonArguments.dartDefineFromFileArg.name),
..._argsNamed(CommonArguments.buildNameArg.name),
..._argsNamed(CommonArguments.buildNumberArg.name),
],
);
return forwarded;
}
packages/shorebird_cli/lib/src/extensions/iterable.dart
+5
View File
@@ -0,0 +1,5 @@
/// Provides the [containsAnyOf] method to all [Iterable]s.
extension ContainsAnyOf<T> on Iterable<T> {
/// Returns `true` if any of the elements in [elements] are in this iterable.
bool containsAnyOf(Iterable<T> elements) => elements.any(contains);
}
packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart
+1
View File
@@ -136,6 +136,7 @@ void main() {
shorebirdValidator = MockShorebirdValidator();
shorebirdAndroidArtifacts = MockShorebirdAndroidArtifacts();
when(() => argResults.options).thenReturn([]);
when(() => argResults.rest).thenReturn([]);
when(() => argResults.wasParsed(any())).thenReturn(false);
packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart
+1 -1
View File
@@ -129,7 +129,7 @@ void main() {
shorebirdValidator = MockShorebirdValidator();
xcodeBuild = MockXcodeBuild();
when(() => argResults['build-number']).thenReturn('1.0');
when(() => argResults.options).thenReturn([]);
when(() => argResults.rest).thenReturn([]);
when(() => argResults.wasParsed(any())).thenReturn(false);
packages/shorebird_cli/test/src/commands/patch/patcher_test.dart
+49 -5
View File
@@ -4,6 +4,7 @@ import 'package:args/args.dart';
import 'package:mocktail/mocktail.dart';
import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
import 'package:shorebird_cli/src/commands/commands.dart';
import 'package:shorebird_cli/src/common_arguments.dart';
import 'package:shorebird_cli/src/patch_diff_checker.dart';
import 'package:shorebird_cli/src/platform/platform.dart';
import 'package:shorebird_cli/src/release_type.dart';
@@ -45,6 +46,7 @@ void main() {
late ArgResults argResults;
setUp(() {
argResults = MockArgResults();
when(() => argResults.options).thenReturn([]);
});
group('when releaseVersion is not specified', () {
@@ -74,12 +76,26 @@ void main() {
});
group('when a valid --release-version is specified', () {
group('when --build-name and --build-number are specified', () {
group('when --build-name is specified', () {
setUp(() {
when(() => argResults.rest).thenReturn([
'--build-name=foo',
'--build-number=42',
]);
when(() => argResults.rest).thenReturn(['--build-name=foo']);
});
test('returns an empty list', () {
expect(
_TestPatcher(
argResults: argResults,
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'),
isEmpty,
);
});
});
group('when --build-number is specified', () {
setUp(() {
when(() => argResults.rest).thenReturn(['--build-number=42']);
});
test('returns an empty list', () {
@@ -108,6 +124,34 @@ void main() {
);
});
});
group('when build-name and build-number were parsed as options', () {
setUp(() {
when(
() => argResults.wasParsed(CommonArguments.buildNameArg.name),
).thenReturn(true);
when(
() => argResults.wasParsed(CommonArguments.buildNumberArg.name),
).thenReturn(true);
when(() => argResults.options).thenReturn([
'release-version',
'build-name',
'build-number',
'platforms',
]);
});
test('returns an empty list', () {
expect(
_TestPatcher(
argResults: argResults,
flavor: null,
target: null,
).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'),
isEmpty,
);
});
});
});
});
});
packages/shorebird_cli/test/src/extensions/arg_results_test.dart
+51
View File
@@ -98,6 +98,14 @@ void main() {
CommonArguments.dartDefineFromFileArg.name,
help: CommonArguments.dartDefineFromFileArg.description,
)
..addOption(
CommonArguments.buildNameArg.name,
help: CommonArguments.buildNameArg.description,
)
..addOption(
CommonArguments.buildNumberArg.name,
help: CommonArguments.buildNumberArg.description,
)
..addMultiOption(
'platforms',
allowed: ReleaseType.values.map((e) => e.cliName),
@@ -178,5 +186,48 @@ void main() {
);
});
});
group('when build-name and build-number are provided', () {
test('forwards build-name and build-number', () {
final args = [
'--verbose',
'--',
'--build-name=1.2.3',
'--build-number=4',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(2));
expect(
result.forwardedArgs,
containsAll(
[
'--build-name=1.2.3',
'--build-number=4',
],
),
);
});
});
group('when build-name and build-number are before the --', () {
test('forwards build-name and build-number', () {
final args = [
'--verbose',
'--build-name=1.2.3',
'--build-number=4',
];
final result = parser.parse(args);
expect(result.forwardedArgs, hasLength(2));
expect(
result.forwardedArgs,
containsAll(
[
'--build-name=1.2.3',
'--build-number=4',
],
),
);
});
});
});
}
packages/shorebird_cli/test/src/extensions/iterable_test.dart
+18
View File
@@ -0,0 +1,18 @@
import 'package:shorebird_cli/src/extensions/iterable.dart';
import 'package:test/test.dart';
void main() {
group('containsAnyOf', () {
test('returns true when any element is in the iterable', () {
final iterable = [1, 2, 3];
final elements = [3, 4, 5];
expect(iterable.containsAnyOf(elements), isTrue);
});
test('returns false when no element is in the iterable', () {
final iterable = [1, 2, 3];
final elements = [4, 5, 6];
expect(iterable.containsAnyOf(elements), isFalse);
});
});
}