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

Commit

Permalink
[tools]Build IPA validation UI Polish (#116744)
Browse files Browse the repository at this point in the history
* [tools]some ui polish for build ipa validation

* do not print out a few success validations

* rename installed type to success for more general usage

* forgot nit after reverting custom validation types and re-use doctor types
  • Loading branch information
hellohuanlin committed Dec 15, 2022
1 parent c98978a commit 0916375
Show file tree
Hide file tree
Showing 27 changed files with 221 additions and 134 deletions.
32 changes: 17 additions & 15 deletions dev/devicelab/bin/tasks/ios_content_validation_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,23 @@ Future<void> main() async {
throw TaskResult.failure('Usage archive event not sent');
}

if (!output.contains('Warning: App icon is using the wrong size (e.g. Icon-App-20x20@1x.png).')) {
throw TaskResult.failure('Must validate incorrect app icon image size.');
}

// The project is still using Flutter template icon and launch image.
if (!output.contains('Warning: App icon is set to the default placeholder icon. Replace with unique icons.')) {
throw TaskResult.failure('Must validate template app icon.');
}
if (!output.contains('Warning: Launch image is set to the default placeholder. Replace with unique launch images.')) {
throw TaskResult.failure('Must validate template launch image.');
}

// The project is still using com.example as bundle identifier prefix.
if (!output.contains('Warning: Your application still contains the default "com.example" bundle identifier.')) {
throw TaskResult.failure('Must validate the default bundle identifier prefix');
// The output contains extra time related prefix, so cannot use a single string.
const List<String> expectedValidationMessages = <String>[
'[!] App Settings Validation\n',
' • Version Number: 1.0.0\n',
' • Build Number: 1\n',
' • Display Name: Hello\n',
' • Deployment Target: 11.0\n',
' • Bundle Identifier: com.example.hello\n',
' ! Your application still contains the default "com.example" bundle identifier.\n',
'[!] App Icon and Launch Image Assets Validation\n',
' ! App icon is set to the default placeholder icon. Replace with unique icons.\n',
' ! App icon is using the incorrect size (e.g. Icon-App-20x20@1x.png).\n',
' ! Launch image is set to the default placeholder icon. Replace with unique launch image.\n',
'To update the settings, please refer to https://docs.flutter.dev/deployment/ios\n',
];
if (expectedValidationMessages.any((String message) => !output.contains(message))) {
throw TaskResult.failure('Must have the expected validation message');
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class AndroidStudioValidator extends DoctorValidator {
if (_studio.isValid) {
type = _hasIssues(messages)
? ValidationType.partial
: ValidationType.installed;
: ValidationType.success;
messages.addAll(_studio.validationMessages.map<ValidationMessage>(
(String m) => ValidationMessage(m),
));
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter_tools/lib/src/android/android_workflow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class AndroidValidator extends DoctorValidator {
}

// Success.
return ValidationResult(ValidationType.installed, messages, statusInfo: sdkVersionText);
return ValidationResult(ValidationType.success, messages, statusInfo: sdkVersionText);
}
}

Expand Down Expand Up @@ -327,7 +327,7 @@ class AndroidLicenseValidator extends DoctorValidator {
messages.add(ValidationMessage.error(_userMessages.androidLicensesUnknown(_platform)));
return ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
}
return ValidationResult(ValidationType.installed, messages, statusInfo: sdkVersionText);
return ValidationResult(ValidationType.success, messages, statusInfo: sdkVersionText);
}

Future<bool> _checkJavaVersionNoOutput() async {
Expand Down
109 changes: 88 additions & 21 deletions packages/flutter_tools/lib/src/commands/build_ios.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import '../base/process.dart';
import '../base/utils.dart';
import '../build_info.dart';
import '../convert.dart';
import '../doctor_validator.dart';
import '../globals.dart' as globals;
import '../ios/application_package.dart';
import '../ios/mac.dart';
Expand Down Expand Up @@ -277,7 +278,27 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
.toList();
}

Future<void> _validateIconAssetsAfterArchive(StringBuffer messageBuffer) async {
ValidationResult? _createValidationResult(String title, List<ValidationMessage> messages) {
if (messages.isEmpty) {
return null;
}
final bool anyInvalid = messages.any((ValidationMessage message) => message.type != ValidationMessageType.information);
return ValidationResult(
anyInvalid ? ValidationType.partial : ValidationType.success,
messages,
statusInfo: title,
);
}

ValidationMessage _createValidationMessage({
required bool isValid,
required String message,
}) {
// Use "information" type for valid message, and "hint" type for invalid message.
return isValid ? ValidationMessage(message) : ValidationMessage.hint(message);
}

Future<List<ValidationMessage>> _validateIconAssetsAfterArchive() async {
final BuildableIOSApp app = await buildableIOSApp;

final Map<_ImageAssetFileKey, String> templateInfoMap = _parseImageAssetContentsJson(
Expand All @@ -287,24 +308,35 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
app.projectAppIconDirName,
requiresSize: true);

final List<ValidationMessage> validationMessages = <ValidationMessage>[];

final bool usesTemplate = _isAssetStillUsingTemplateFiles(
templateImageInfoMap: templateInfoMap,
projectImageInfoMap: projectInfoMap,
templateImageDirName: await app.templateAppIconDirNameForImages,
projectImageDirName: app.projectAppIconDirName);

if (usesTemplate) {
messageBuffer.writeln('\nWarning: App icon is set to the default placeholder icon. Replace with unique icons.');
validationMessages.add(_createValidationMessage(
isValid: false,
message: 'App icon is set to the default placeholder icon. Replace with unique icons.',
));
}

final List<String> filesWithWrongSize = _imageFilesWithWrongSize(
imageInfoMap: projectInfoMap,
imageDirName: app.projectAppIconDirName);

if (filesWithWrongSize.isNotEmpty) {
messageBuffer.writeln('\nWarning: App icon is using the wrong size (e.g. ${filesWithWrongSize.first}).');
validationMessages.add(_createValidationMessage(
isValid: false,
message: 'App icon is using the incorrect size (e.g. ${filesWithWrongSize.first}).',
));
}
return validationMessages;
}

Future<void> _validateLaunchImageAssetsAfterArchive(StringBuffer messageBuffer) async {
Future<List<ValidationMessage>> _validateLaunchImageAssetsAfterArchive() async {
final BuildableIOSApp app = await buildableIOSApp;

final Map<_ImageAssetFileKey, String> templateInfoMap = _parseImageAssetContentsJson(
Expand All @@ -314,25 +346,32 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
app.projectLaunchImageDirName,
requiresSize: false);

final List<ValidationMessage> validationMessages = <ValidationMessage>[];

final bool usesTemplate = _isAssetStillUsingTemplateFiles(
templateImageInfoMap: templateInfoMap,
projectImageInfoMap: projectInfoMap,
templateImageDirName: await app.templateLaunchImageDirNameForImages,
projectImageDirName: app.projectLaunchImageDirName);

if (usesTemplate) {
messageBuffer.writeln('\nWarning: Launch image is set to the default placeholder. Replace with unique launch images.');
validationMessages.add(_createValidationMessage(
isValid: false,
message: 'Launch image is set to the default placeholder icon. Replace with unique launch image.',
));
}

return validationMessages;
}

Future<void> _validateXcodeBuildSettingsAfterArchive(StringBuffer messageBuffer) async {
Future<List<ValidationMessage>> _validateXcodeBuildSettingsAfterArchive() async {
final BuildableIOSApp app = await buildableIOSApp;

final String plistPath = app.builtInfoPlistPathAfterArchive;

if (!globals.fs.file(plistPath).existsSync()) {
globals.printError('Invalid iOS archive. Does not contain Info.plist.');
return;
return <ValidationMessage>[];
}

final Map<String, String?> xcodeProjectSettingsMap = <String, String?>{};
Expand All @@ -343,17 +382,32 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
xcodeProjectSettingsMap['Deployment Target'] = globals.plistParser.getStringValueFromFile(plistPath, PlistParser.kMinimumOSVersionKey);
xcodeProjectSettingsMap['Bundle Identifier'] = globals.plistParser.getStringValueFromFile(plistPath, PlistParser.kCFBundleIdentifierKey);

xcodeProjectSettingsMap.forEach((String title, String? info) {
messageBuffer.writeln('$title: ${info ?? "Missing"}');
});
final List<ValidationMessage> validationMessages = xcodeProjectSettingsMap.entries.map((MapEntry<String, String?> entry) {
final String title = entry.key;
final String? info = entry.value;
return _createValidationMessage(
isValid: info != null,
message: '$title: ${info ?? "Missing"}',
);
}).toList();

if (xcodeProjectSettingsMap.values.any((String? element) => element == null)) {
messageBuffer.writeln('\nYou must set up the missing settings.');
final bool hasMissingSettings = xcodeProjectSettingsMap.values.any((String? element) => element == null);
if (hasMissingSettings) {
validationMessages.add(_createValidationMessage(
isValid: false,
message: 'You must set up the missing app settings.'),
);
}

if (xcodeProjectSettingsMap['Bundle Identifier']?.startsWith('com.example') ?? false) {
messageBuffer.writeln('\nWarning: Your application still contains the default "com.example" bundle identifier.');
final bool usesDefaultBundleIdentifier = xcodeProjectSettingsMap['Bundle Identifier']?.startsWith('com.example') ?? false;
if (usesDefaultBundleIdentifier) {
validationMessages.add(_createValidationMessage(
isValid: false,
message: 'Your application still contains the default "com.example" bundle identifier.'),
);
}

return validationMessages;
}

@override
Expand All @@ -362,13 +416,26 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
displayNullSafetyMode(buildInfo);
final FlutterCommandResult xcarchiveResult = await super.runCommand();

final StringBuffer validationMessageBuffer = StringBuffer();
await _validateXcodeBuildSettingsAfterArchive(validationMessageBuffer);
await _validateIconAssetsAfterArchive(validationMessageBuffer);
await _validateLaunchImageAssetsAfterArchive(validationMessageBuffer);

validationMessageBuffer.write('\nTo update the settings, please refer to https://docs.flutter.dev/deployment/ios');
globals.printBox(validationMessageBuffer.toString(), title: 'App Settings');
final List<ValidationResult?> validationResults = <ValidationResult?>[];
validationResults.add(_createValidationResult(
'App Settings Validation',
await _validateXcodeBuildSettingsAfterArchive(),
));
validationResults.add(_createValidationResult(
'App Icon and Launch Image Assets Validation',
await _validateIconAssetsAfterArchive() + await _validateLaunchImageAssetsAfterArchive(),
));

for (final ValidationResult result in validationResults.whereType<ValidationResult>()) {
globals.printStatus('\n${result.coloredLeadingBox} ${result.statusInfo}');
for (final ValidationMessage message in result.messages) {
globals.printStatus(
'${message.coloredIndicator} ${message.message}',
indent: result.leadingBox.length + 1,
);
}
}
globals.printStatus('\nTo update the settings, please refer to https://docs.flutter.dev/deployment/ios\n');

// xcarchive failed or not at expected location.
if (xcarchiveResult.exitStatus != ExitStatus.success) {
Expand Down
12 changes: 6 additions & 6 deletions packages/flutter_tools/lib/src/doctor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class Doctor {
case ValidationType.notAvailable:
lineBuffer.write('is not available.');
break;
case ValidationType.installed:
case ValidationType.success:
lineBuffer.write('is fully installed.');
break;
}
Expand All @@ -320,7 +320,7 @@ class Doctor {
));
buffer.writeln();

if (result.type != ValidationType.installed) {
if (result.type != ValidationType.success) {
missingComponent = true;
}
}
Expand Down Expand Up @@ -400,7 +400,7 @@ class Doctor {
case ValidationType.notAvailable:
issues += 1;
break;
case ValidationType.installed:
case ValidationType.success:
break;
}
if (sendEvent) {
Expand Down Expand Up @@ -558,7 +558,7 @@ class FlutterValidator extends DoctorValidator {

ValidationType valid;
if (messages.every((ValidationMessage message) => message.isInformation)) {
valid = ValidationType.installed;
valid = ValidationType.success;
} else {
// The issues for this validator stem from broken git configuration of the local install;
// in that case, make it clear that it is fine to continue, but freshness check/upgrades
Expand Down Expand Up @@ -707,13 +707,13 @@ class DeviceValidator extends DoctorValidator {
} else if (diagnostics.isNotEmpty) {
installedMessages.addAll(diagnosticMessages);
return ValidationResult(
ValidationType.installed,
ValidationType.success,
installedMessages,
statusInfo: _userMessages.devicesAvailable(devices.length)
);
} else {
return ValidationResult(
ValidationType.installed,
ValidationType.success,
installedMessages,
statusInfo: _userMessages.devicesAvailable(devices.length)
);
Expand Down
14 changes: 7 additions & 7 deletions packages/flutter_tools/lib/src/doctor_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ enum ValidationType {
missing,
partial,
notAvailable,
installed,
success,
}

enum ValidationMessageType {
Expand Down Expand Up @@ -116,7 +116,7 @@ class GroupedValidator extends DoctorValidator {
for (final ValidationResult result in results) {
statusInfo ??= result.statusInfo;
switch (result.type) {
case ValidationType.installed:
case ValidationType.success:
if (mergedType == ValidationType.missing) {
mergedType = ValidationType.partial;
}
Expand All @@ -127,7 +127,7 @@ class GroupedValidator extends DoctorValidator {
break;
case ValidationType.crash:
case ValidationType.missing:
if (mergedType == ValidationType.installed) {
if (mergedType == ValidationType.success) {
mergedType = ValidationType.partial;
}
break;
Expand All @@ -142,7 +142,7 @@ class GroupedValidator extends DoctorValidator {

@immutable
class ValidationResult {
/// [ValidationResult.type] should only equal [ValidationResult.installed]
/// [ValidationResult.type] should only equal [ValidationResult.success]
/// if no [messages] are hints or errors.
const ValidationResult(this.type, this.messages, { this.statusInfo });

Expand Down Expand Up @@ -171,7 +171,7 @@ class ValidationResult {
return '[☠]';
case ValidationType.missing:
return '[✗]';
case ValidationType.installed:
case ValidationType.success:
return '[✓]';
case ValidationType.notAvailable:
case ValidationType.partial:
Expand All @@ -186,7 +186,7 @@ class ValidationResult {
return globals.terminal.color(leadingBox, TerminalColor.red);
case ValidationType.missing:
return globals.terminal.color(leadingBox, TerminalColor.red);
case ValidationType.installed:
case ValidationType.success:
return globals.terminal.color(leadingBox, TerminalColor.green);
case ValidationType.notAvailable:
case ValidationType.partial:
Expand All @@ -202,7 +202,7 @@ class ValidationResult {
return 'crash';
case ValidationType.missing:
return 'missing';
case ValidationType.installed:
case ValidationType.success:
return 'installed';
case ValidationType.notAvailable:
return 'notAvailable';
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/http_host_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class HttpHostValidator extends DoctorValidator {

if (availabilityResults.every((_HostValidationResult result) => result.available)) {
return ValidationResult(
ValidationType.installed,
ValidationType.success,
messages..add(const ValidationMessage('All required HTTP hosts are available')),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ abstract class IntelliJValidator extends DoctorValidator {
}

return ValidationResult(
_hasIssues(messages) ? ValidationType.partial : ValidationType.installed,
_hasIssues(messages) ? ValidationType.partial : ValidationType.success,
messages,
statusInfo: _userMessages.intellijStatusInfo(version),
);
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/linux/linux_doctor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class LinuxDoctorValidator extends DoctorValidator {

@override
Future<ValidationResult> validate() async {
ValidationType validationType = ValidationType.installed;
ValidationType validationType = ValidationType.success;
final List<ValidationMessage> messages = <ValidationMessage>[];

final Map<String, _VersionInfo?> installedVersions = <String, _VersionInfo?>{
Expand Down
Loading

0 comments on commit 0916375

Please sign in to comment.