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

feat(shorebird_cli): enable autofixing issues with shorebird doctor --fix (#405)

Browse Source
This commit is contained in:
Bryan Oltman
2023-05-02 14:11:23 -04:00
committed by GitHub
parent 1e0d39fc62
commit a584f856be
5 changed files with 310 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/shorebird_cli/lib/src/commands/doctor_command.dart
+66 -7
View File
@@ -18,6 +18,13 @@ class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
super.validators,
}) {
validators = _allValidators(baseValidators: validators);
argParser.addFlag(
'fix',
abbr: 'f',
help: 'Fix issues where possible.',
negatable: false,
);
}
late final List<Validator> _doctorValidators = [
@@ -36,33 +43,85 @@ class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
@override
Future<int> run() async {
final shouldFix = results['fix'] == true;
logger.info('''
Shorebird v$packageVersion
Shorebird Engine • revision ${ShorebirdEnvironment.shorebirdEngineRevision}''');
var numIssues = 0;
final allIssues = <ValidationIssue>[];
final allFixableIssues = <ValidationIssue>[];
for (final validator in validators) {
final failedFixes = <ValidationIssue, dynamic>{};
final progress = logger.progress(validator.description);
final issues = await validator.validate(process);
numIssues += issues.length;
if (issues.isEmpty) {
progress.complete();
} else {
progress.fail();
continue;
}
for (final issue in issues) {
logger.info(' ${issue.displayMessage}');
final fixableIssues = issues.where((issue) => issue.fix != null);
var unresolvedIssues = issues;
if (fixableIssues.isNotEmpty) {
if (shouldFix) {
// If --fix flag was used and there are fixable issues, fix them.
progress.update('Fixing');
for (final issue in fixableIssues) {
try {
await issue.fix!();
} catch (error) {
failedFixes[issue] = error;
}
}
// Re-run the validator to see if there are any remaining issues that
// we couldn't fix.
unresolvedIssues = await validator.validate(process);
if (unresolvedIssues.isEmpty) {
final numFixed = issues.length - unresolvedIssues.length;
final fixAppliedMessage =
'($numFixed fix${numFixed == 1 ? '' : 'es'} applied)';
progress.complete(
'''${validator.description} ${green.wrap(fixAppliedMessage)}''',
);
continue;
}
} else {
allFixableIssues.addAll(issues);
}
}
progress.fail(validator.description);
for (final issue in failedFixes.keys) {
logger.err(
''' An error occurred while attempting to fix ${issue.message}: ${failedFixes[issue]}''',
);
}
for (final issue in unresolvedIssues) {
logger.info(' ${issue.displayMessage}');
}
allIssues.addAll(unresolvedIssues);
}
logger.info('');
if (numIssues == 0) {
if (allIssues.isEmpty) {
logger.info('No issues detected!');
} else {
final numIssues = allIssues.length;
logger.info('$numIssues issue${numIssues == 1 ? '' : 's'} detected.');
if (allFixableIssues.isNotEmpty && !shouldFix) {
final fixableIssueCount = allFixableIssues.length;
logger.info(
'''
$fixableIssueCount issue${fixableIssueCount == 1 ? '' : 's'} can be fixed automatically with ${lightCyan.wrap('shorebird doctor --fix')}.''',
);
}
}
return ExitCode.success.code;
packages/shorebird_cli/lib/src/validators/android_internet_permission_validator.dart
+18 -1
View File
@@ -44,7 +44,7 @@ class AndroidInternetPermissionValidator extends Validator {
(dir) => dir
.listSync()
.whereType<File>()
.any((file) => p.basename(file.path) == 'AndroidManifest.xml'),
.any((file) => p.basename(file.path) == manifestFileName),
)
.map((e) => p.join(e.path, manifestFileName));
@@ -67,6 +67,7 @@ class AndroidInternetPermissionValidator extends Validator {
(String manifestPath) => ValidationIssue(
severity: ValidationIssueSeverity.error,
message: '$manifestPath is missing the INTERNET permission.',
fix: () => _addInternetPermissionToFile(manifestPath),
),
)
.toList();
@@ -90,4 +91,20 @@ class AndroidInternetPermissionValidator extends Validator {
return attribute.qualifiedName == 'android:name' &&
attribute.value == 'android.permission.INTERNET';
}
void _addInternetPermissionToFile(String path) {
final xmlDocument = XmlDocument.parse(File(path).readAsStringSync());
xmlDocument.rootElement.children.add(
XmlElement(
XmlName('uses-permission'),
[
XmlAttribute(
XmlName('android:name'),
'android.permission.INTERNET',
),
],
),
);
File(path).writeAsStringSync(xmlDocument.toXmlString(pretty: true));
}
}
packages/shorebird_cli/lib/src/validators/validators.dart
+10 -1
View File
@@ -1,3 +1,5 @@
import 'dart:async';
import 'package:mason_logger/mason_logger.dart';
import 'package:meta/meta.dart';
import 'package:shorebird_cli/src/shorebird_process.dart';
@@ -30,7 +32,11 @@ extension Display on ValidationIssueSeverity {
/// A (potential) problem with the current Shorebird installation or project.
@immutable
class ValidationIssue {
const ValidationIssue({required this.severity, required this.message});
const ValidationIssue({
required this.severity,
required this.message,
this.fix,
});
/// How important it is to fix this issue.
final ValidationIssueSeverity severity;
@@ -38,6 +44,9 @@ class ValidationIssue {
/// A description of the issue.
final String message;
/// Fixes this issue.
final FutureOr<void> Function()? fix;
/// A console-friendly description of this issue.
String? get displayMessage {
return '${severity.leading} $message';
packages/shorebird_cli/test/src/commands/doctor_command_test.dart
+188 -3
View File
@@ -1,3 +1,4 @@
import 'package:args/args.dart';
import 'package:mason_logger/mason_logger.dart';
import 'package:mocktail/mocktail.dart';
import 'package:shorebird_cli/src/commands/commands.dart';
@@ -6,6 +7,8 @@ import 'package:shorebird_cli/src/shorebird_process.dart';
import 'package:shorebird_cli/src/validators/validators.dart';
import 'package:test/test.dart';
class _MockArgResults extends Mock implements ArgResults {}
class _MockShorebirdVersionValidator extends Mock
implements ShorebirdVersionValidator {}
@@ -23,6 +26,9 @@ class _MockShorebirdProcess extends Mock implements ShorebirdProcess {}
void main() {
group('doctor', () {
const androidValidatorDescription = 'Android';
late ArgResults argResults;
late Logger logger;
late Progress progress;
late DoctorCommand command;
@@ -32,11 +38,14 @@ void main() {
late ShorebirdProcess shorebirdProcess;
setUp(() {
argResults = _MockArgResults();
logger = _MockLogger();
progress = _MockProgress();
ShorebirdEnvironment.shorebirdEngineRevision = 'test-revision';
when(() => argResults['fix']).thenReturn(false);
when(() => logger.progress(any())).thenReturn(progress);
when(() => logger.info(any())).thenReturn(null);
@@ -50,7 +59,7 @@ void main() {
when(() => androidInternetPermissionValidator.id)
.thenReturn('$AndroidInternetPermissionValidator');
when(() => androidInternetPermissionValidator.description)
.thenReturn('Android');
.thenReturn(androidValidatorDescription);
when(() => androidInternetPermissionValidator.validate(any()))
.thenAnswer((_) async => []);
@@ -76,6 +85,7 @@ void main() {
shorebirdFlutterValidator,
],
)
..testArgResults = argResults
..testProcess = shorebirdProcess
..testEngineConfig = const EngineConfig.empty();
});
@@ -113,16 +123,191 @@ void main() {
}
verify(
() => logger.info(any(that: contains('${yellow.wrap('[!]')} oh no!'))),
() => logger.info(any(that: stringContainsInOrder(['[!]', 'oh no!']))),
).called(1);
verify(
() => logger.info(any(that: contains('${red.wrap('[✗]')} OH NO!'))),
() => logger.info(any(that: stringContainsInOrder(['[✗]', 'OH NO!']))),
).called(1);
verify(
() => logger.info(any(that: contains('2 issues detected.'))),
).called(1);
});
test('tells the user we can fix issues if we can', () async {
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async => [
ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
fix: () async {},
),
],
);
await command.run();
verify(
() => logger.info(
any(
that: stringContainsInOrder([
'1 issue can be fixed automatically',
'shorebird doctor --fix',
]),
),
),
).called(1);
});
test('does not tell the user we can fix issues if we cannot', () async {
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async => [
const ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
),
],
);
await command.run();
verifyNever(
() => logger.info(
any(
that: stringContainsInOrder([
'We can fix some of these issues',
'shorebird doctor --fix',
]),
),
),
);
});
test('does not fix issues if --fix flag is not provided', () async {
when(() => argResults['fix']).thenReturn(false);
var fixCalled = false;
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async => [
ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
fix: () => fixCalled = true,
),
],
);
await command.run();
expect(fixCalled, isFalse);
verifyNever(() => progress.update('Fixing'));
verify(() => progress.fail(androidValidatorDescription)).called(1);
verify(
() => androidInternetPermissionValidator.validate(any()),
).called(1);
});
test('fixes issues if the --fix flag is provided', () async {
when(() => argResults['fix']).thenReturn(true);
var fixCalled = false;
final issues = [
ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
fix: () => fixCalled = true,
),
];
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async {
if (issues.isEmpty) return [];
return [issues.removeLast()];
},
);
await command.run();
expect(fixCalled, isTrue);
verify(() => progress.update('Fixing')).called(1);
verify(
() => progress.complete(any(that: contains('1 fix applied'))),
).called(1);
verify(
() => androidInternetPermissionValidator.validate(any()),
).called(2);
});
test('does not print "fixed" if fix fails', () async {
when(() => argResults['fix']).thenReturn(true);
var fixCalled = false;
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async => [
ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
fix: () => fixCalled = true,
),
],
);
await command.run();
expect(fixCalled, isTrue);
verify(() => progress.update('Fixing')).called(1);
verify(() => progress.fail(androidValidatorDescription)).called(1);
verifyNever(
() => progress.complete(any(that: contains('fix applied'))),
);
verifyNever(
() => progress.complete(any(that: contains('fixes applied'))),
);
verify(
() => androidInternetPermissionValidator.validate(any()),
).called(2);
});
test('prints error and continues if fix() throws', () async {
when(() => argResults['fix']).thenReturn(true);
when(
() => androidInternetPermissionValidator.validate(any()),
).thenAnswer(
(_) async => [
ValidationIssue(
severity: ValidationIssueSeverity.warning,
message: 'oh no!',
fix: () => throw Exception('oh no!'),
),
],
);
await command.run();
verify(() => progress.update('Fixing')).called(1);
verify(
() => androidInternetPermissionValidator.validate(any()),
).called(2);
verify(
() => logger.err(
any(
that: stringContainsInOrder([
'An error occurred while attempting to fix',
'oh no!',
]),
),
),
).called(1);
});
});
}
packages/shorebird_cli/test/src/validators/android_internet_permission_validator_test.dart
+28
View File
@@ -82,6 +82,7 @@ void main() {
expect(results, hasLength(1));
expect(results.first.severity, ValidationIssueSeverity.error);
expect(results.first.message, 'No Android project found');
expect(results.first.fix, isNull);
});
test('returns an error if no AndroidManifest.xml files are found',
@@ -101,6 +102,7 @@ void main() {
results.first.message,
startsWith('No AndroidManifest.xml files found in'),
);
expect(results.first.fix, isNull);
});
test(
@@ -165,5 +167,31 @@ void main() {
);
},
);
test('fix() adds permission to manifest file', () async {
final tempDirectory = createTempDir();
writeManifestToPath(
manifestWithNonInternetPermissions,
p.join(tempDirectory.path, 'android', 'app', 'src', 'debug'),
);
var results = await IOOverrides.runZoned(
() => AndroidInternetPermissionValidator().validate(shorebirdProcess),
getCurrentDirectory: () => tempDirectory,
);
expect(results, hasLength(1));
expect(results.first.fix, isNotNull);
await IOOverrides.runZoned(
() => results.first.fix!(),
getCurrentDirectory: () => tempDirectory,
);
results = await IOOverrides.runZoned(
() => AndroidInternetPermissionValidator().validate(shorebirdProcess),
getCurrentDirectory: () => tempDirectory,
);
expect(results, isEmpty);
});
});
}