Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(shorebird_cli): add xcodeVersion method to xcodebuild wrapper #1360

Merged
merged 1 commit into from
Oct 5, 2023
Merged

feat(shorebird_cli): add xcodeVersion method to xcodebuild wrapper #1360

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions packages/shorebird_cli/lib/src/executables/xcodebuild.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:io';

import 'package:mason_logger/mason_logger.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
import 'package:scoped/scoped.dart';
import 'package:shorebird_cli/src/process.dart';

Expand Down Expand Up @@ -106,4 +107,41 @@ class XcodeBuild {
schemes: schemes,
);
}

/// Gets the currently installed version of Xcode.
///
/// Invokes `xcodebuild -version` and parses the output.
/// Output is expected to be of the form:
///
/// $ /usr/bin/xcodebuild -version
/// Xcode 15.0
/// Build version 15A240d
Future<Version> xcodeVersion() async {
const arguments = ['-version'];
final result = await process.run(
executable,
arguments,
);

if (result.exitCode != ExitCode.success.code) {
throw ProcessException(executable, arguments, '${result.stderr}');
}

final lines = LineSplitter.split('${result.stdout}').map((e) => e.trim());
var versionString = lines.firstOrNull?.split(' ').lastOrNull;
if (versionString == null) {
throw FormatException(
'Could not parse Xcode version from output: "${result.stdout}".',
);
}

// [Version.parse] requires a patch number. If Xcode does not report a patch
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this will work for all XCode versions and/or whether it would be easier and less error-prone to try to parse the version as a double and see if it’s greater than 15 since clearly XCode doesn’t follow semver I’m not sure this would be the only issue we encounter (for example if there’s a beta version of XCode do we handle that correctly with this logic). 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we should probably be consistent with the flutter_tool in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good Q about the beta, let me download one and give that a shot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interestingly, Xcode betas don't report that they're beta in xcodebuild -version:

⑆ xcrun xcodebuild -version
Xcode 15.1
Build version 15C5028h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To resolve this, I don't think parsing it as a double would work, as there is occasionally a patch version.

// number (e.g. "12.0"), add a patch number of 0 (e.g. "12.0.0")
final noPachNumberRegex = RegExp(r'^\d+\.\d+$');
if (noPachNumberRegex.hasMatch(versionString)) {
versionString += '.0';
}

return Version.parse(versionString);
}
}
95 changes: 95 additions & 0 deletions packages/shorebird_cli/test/src/executables/xcodebuild_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:io';
import 'package:mason_logger/mason_logger.dart';
import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
import 'package:scoped/scoped.dart';
import 'package:shorebird_cli/src/executables/executables.dart';
import 'package:shorebird_cli/src/process.dart';
Expand All @@ -13,6 +14,7 @@ import '../mocks.dart';
void main() {
group(XcodeBuild, () {
late ShorebirdProcess process;
late ShorebirdProcessResult processResult;
late XcodeBuild xcodeBuild;

R runWithOverrides<R>(R Function() body) {
Expand All @@ -32,6 +34,7 @@ void main() {

setUp(() {
process = MockShorebirdProcess();
processResult = MockShorebirdProcessResult();
xcodeBuild = runWithOverrides(XcodeBuild.new);
});

Expand Down Expand Up @@ -132,5 +135,97 @@ To add iOS, run "flutter create . --platforms ios"''',
).called(1);
});
});

group('xcodeVersion', () {
late ExitCode exitCode;
late String stdout;

setUp(() {
when(() => process.run(XcodeBuild.executable, ['-version']))
.thenAnswer((_) async => processResult);
when(() => processResult.exitCode).thenAnswer((_) => exitCode.code);
when(() => processResult.stdout).thenAnswer((_) => stdout);
});

group('when a non-zero exit code is returned', () {
const errorMessage = 'An unexpected error occurred.';
setUp(() {
stdout = '';
exitCode = ExitCode.cantCreate;
when(() => processResult.stderr).thenReturn(errorMessage);
});

test('throws a ProcessException', () async {
expect(
() => runWithOverrides(xcodeBuild.xcodeVersion),
throwsA(
isA<ProcessException>()
.having((e) => e.message, 'message', errorMessage),
),
);
});
});

group('when stdout contains unexpected output', () {
setUp(() {
exitCode = ExitCode.success;
});

test('throws FormatException if output is empty', () async {
stdout = '';
expect(
() => runWithOverrides(xcodeBuild.xcodeVersion),
throwsA(
isA<FormatException>().having(
(e) => e.message,
'message',
'Could not parse Xcode version from output: "".',
),
),
);
});

test('throws FormatException if output does not contain version',
() async {
stdout = 'unexpected output';
expect(
() => runWithOverrides(xcodeBuild.xcodeVersion),
throwsA(
isA<FormatException>().having(
(e) => e.message,
'message',
'Could not parse "output".',
),
),
);
});
});

group('when stdout contains valid output', () {
setUp(() {
exitCode = ExitCode.success;
});

test('returns correct version with major, minor, and build numbers',
() async {
stdout = '''
Xcode 14.3.1
Build version 14E300c
''';
final version = await runWithOverrides(xcodeBuild.xcodeVersion);
expect(version, Version(14, 3, 1));
});

test('returns correct version with only major and minor numbers',
() async {
stdout = '''
Xcode 15.0
Build version 15A240d
''';
final version = await runWithOverrides(xcodeBuild.xcodeVersion);
expect(version, Version(15, 0, 0));
});
});
});
});
}