fix(shorebird_cli): don't run AndroidInternetPermissionValidator if no android project exists (#995)
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
import 'package:mason_logger/mason_logger.dart';
|
||||
import 'package:scoped/scoped.dart';
|
||||
import 'package:shorebird_cli/src/logger.dart';
|
||||
import 'package:shorebird_cli/src/shorebird_environment.dart';
|
||||
import 'package:shorebird_cli/src/validators/validators.dart';
|
||||
|
||||
/// A reference to a [Doctor] instance.
|
||||
@@ -36,15 +35,12 @@ class Doctor {
|
||||
List<Validator> validators, {
|
||||
bool applyFixes = false,
|
||||
}) async {
|
||||
final isInProject =
|
||||
ShorebirdEnvironment.getShorebirdYamlFile().existsSync();
|
||||
|
||||
final allIssues = <ValidationIssue>[];
|
||||
final allFixableIssues = <ValidationIssue>[];
|
||||
|
||||
var numIssuesFixed = 0;
|
||||
for (final validator in validators) {
|
||||
if (validator.scope == ValidatorScope.project && !isInProject) {
|
||||
if (!validator.canRunInCurrentContext()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
+12
-20
@@ -22,30 +22,13 @@ class AndroidInternetPermissionValidator extends Validator {
|
||||
'AndroidManifest.xml files contain INTERNET permission';
|
||||
|
||||
@override
|
||||
ValidatorScope get scope => ValidatorScope.project;
|
||||
bool canRunInCurrentContext() => _androidSrcDirectory.existsSync();
|
||||
|
||||
@override
|
||||
Future<List<ValidationIssue>> validate() async {
|
||||
const manifestFileName = 'AndroidManifest.xml';
|
||||
final androidSrcDir = Directory(
|
||||
p.join(
|
||||
Directory.current.path,
|
||||
'android',
|
||||
'app',
|
||||
'src',
|
||||
),
|
||||
);
|
||||
|
||||
if (!androidSrcDir.existsSync()) {
|
||||
return [
|
||||
const ValidationIssue(
|
||||
severity: ValidationIssueSeverity.error,
|
||||
message: 'No Android project found',
|
||||
),
|
||||
];
|
||||
}
|
||||
|
||||
final manifestFiles = androidSrcDir
|
||||
final manifestFiles = _androidSrcDirectory
|
||||
.listSync()
|
||||
.whereType<Directory>()
|
||||
.where(
|
||||
@@ -61,7 +44,7 @@ class AndroidInternetPermissionValidator extends Validator {
|
||||
ValidationIssue(
|
||||
severity: ValidationIssueSeverity.error,
|
||||
message:
|
||||
'No AndroidManifest.xml files found in ${androidSrcDir.path}',
|
||||
'''No AndroidManifest.xml files found in ${_androidSrcDirectory.path}''',
|
||||
),
|
||||
];
|
||||
}
|
||||
@@ -88,6 +71,15 @@ class AndroidInternetPermissionValidator extends Validator {
|
||||
return [];
|
||||
}
|
||||
|
||||
Directory get _androidSrcDirectory => Directory(
|
||||
p.join(
|
||||
Directory.current.path,
|
||||
'android',
|
||||
'app',
|
||||
'src',
|
||||
),
|
||||
);
|
||||
|
||||
bool _androidManifestHasInternetPermission(String path) {
|
||||
final xmlDocument = XmlDocument.parse(File(path).readAsStringSync());
|
||||
return xmlDocument.rootElement.childElements
|
||||
|
||||
@@ -22,7 +22,7 @@ class ShorebirdFlutterValidator extends Validator {
|
||||
String get description => 'Flutter install is correct';
|
||||
|
||||
@override
|
||||
ValidatorScope get scope => ValidatorScope.installation;
|
||||
bool canRunInCurrentContext() => true;
|
||||
|
||||
@override
|
||||
Future<List<ValidationIssue>> validate() async {
|
||||
|
||||
@@ -11,7 +11,7 @@ class ShorebirdVersionValidator extends Validator {
|
||||
String get description => 'Shorebird is up-to-date';
|
||||
|
||||
@override
|
||||
ValidatorScope get scope => ValidatorScope.installation;
|
||||
bool canRunInCurrentContext() => true;
|
||||
|
||||
@override
|
||||
Future<List<ValidationIssue>> validate() async {
|
||||
|
||||
@@ -18,12 +18,6 @@ enum ValidationIssueSeverity {
|
||||
warning,
|
||||
}
|
||||
|
||||
/// The level at which validation is being performed.
|
||||
enum ValidatorScope {
|
||||
project,
|
||||
installation,
|
||||
}
|
||||
|
||||
/// Display helpers for printing [ValidationIssue]s.
|
||||
extension Display on ValidationIssueSeverity {
|
||||
String get leading {
|
||||
@@ -91,6 +85,5 @@ abstract class Validator {
|
||||
/// Not all validators use [process].
|
||||
Future<List<ValidationIssue>> validate();
|
||||
|
||||
/// Whether this validator is project-specific or system-wide.
|
||||
ValidatorScope get scope;
|
||||
bool canRunInCurrentContext();
|
||||
}
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:mason_logger/mason_logger.dart';
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:scoped/scoped.dart';
|
||||
import 'package:shorebird_cli/src/doctor.dart';
|
||||
import 'package:shorebird_cli/src/logger.dart';
|
||||
@@ -55,24 +52,22 @@ void main() {
|
||||
when(() => logger.info(any())).thenReturn(null);
|
||||
|
||||
when(noIssuesValidator.validate).thenAnswer((_) async => []);
|
||||
when(() => noIssuesValidator.scope)
|
||||
.thenReturn(ValidatorScope.installation);
|
||||
when(noIssuesValidator.canRunInCurrentContext).thenReturn(true);
|
||||
when(() => noIssuesValidator.description)
|
||||
.thenReturn('no issues validator');
|
||||
|
||||
when(warningValidator.validate).thenAnswer(
|
||||
(_) async => [validationWarning],
|
||||
);
|
||||
when(() => warningValidator.scope)
|
||||
.thenReturn(ValidatorScope.installation);
|
||||
when(warningValidator.canRunInCurrentContext).thenReturn(true);
|
||||
when(() => warningValidator.description).thenReturn('warning validator');
|
||||
|
||||
when(errorValidator.validate).thenAnswer((_) async => [validationError]);
|
||||
when(() => errorValidator.scope).thenReturn(ValidatorScope.installation);
|
||||
when(errorValidator.canRunInCurrentContext).thenReturn(true);
|
||||
when(() => errorValidator.description).thenReturn('error validator');
|
||||
});
|
||||
|
||||
group('validate', () {
|
||||
group('runValidators', () {
|
||||
test('prints messages when warnings and errors found', () async {
|
||||
final validators = [
|
||||
warningValidator,
|
||||
@@ -98,64 +93,32 @@ void main() {
|
||||
).called(1);
|
||||
});
|
||||
|
||||
group('validator scope', () {
|
||||
const appId = 'test-app-id';
|
||||
late Validator projectScopeValidator;
|
||||
late Validator installationScopeValidator;
|
||||
test('only runs validators that can run in the current context',
|
||||
() async {
|
||||
final validators = [
|
||||
noIssuesValidator,
|
||||
warningValidator,
|
||||
errorValidator,
|
||||
];
|
||||
when(() => warningValidator.canRunInCurrentContext()).thenReturn(false);
|
||||
|
||||
Directory setUpTempDir() {
|
||||
final tempDir = Directory.systemTemp.createTempSync();
|
||||
File(
|
||||
p.join(tempDir.path, 'shorebird.yaml'),
|
||||
).writeAsStringSync('app_id: $appId');
|
||||
return tempDir;
|
||||
}
|
||||
await runWithOverrides(() async => doctor.runValidators(validators));
|
||||
|
||||
setUp(() {
|
||||
projectScopeValidator = _MockValidator();
|
||||
installationScopeValidator = _MockValidator();
|
||||
verify(noIssuesValidator.validate).called(1);
|
||||
verifyNever(warningValidator.validate);
|
||||
verify(errorValidator.validate).called(1);
|
||||
});
|
||||
|
||||
when(projectScopeValidator.validate).thenAnswer((_) async => []);
|
||||
when(() => projectScopeValidator.scope)
|
||||
.thenReturn(ValidatorScope.project);
|
||||
when(() => projectScopeValidator.description)
|
||||
.thenReturn('project-scoped validator');
|
||||
test('tells the user when no issues are found', () async {
|
||||
final validators = [
|
||||
noIssuesValidator,
|
||||
];
|
||||
|
||||
when(installationScopeValidator.validate).thenAnswer((_) async => []);
|
||||
when(() => installationScopeValidator.scope)
|
||||
.thenReturn(ValidatorScope.installation);
|
||||
when(() => installationScopeValidator.description)
|
||||
.thenReturn('installation-scoped validator');
|
||||
});
|
||||
await runWithOverrides(() async => doctor.runValidators(validators));
|
||||
|
||||
test('does not run project-scoped validators in project directory',
|
||||
() async {
|
||||
final validators = [
|
||||
projectScopeValidator,
|
||||
installationScopeValidator
|
||||
];
|
||||
await runWithOverrides(
|
||||
() => doctor.runValidators(validators),
|
||||
);
|
||||
verify(installationScopeValidator.validate).called(1);
|
||||
verifyNever(projectScopeValidator.validate);
|
||||
});
|
||||
|
||||
test('runs project-scoped validators in project directory', () async {
|
||||
final tempDir = setUpTempDir();
|
||||
final validators = [
|
||||
projectScopeValidator,
|
||||
installationScopeValidator
|
||||
];
|
||||
await runWithOverrides(
|
||||
() async => IOOverrides.runZoned(
|
||||
() => doctor.runValidators(validators),
|
||||
getCurrentDirectory: () => tempDir,
|
||||
),
|
||||
);
|
||||
verify(installationScopeValidator.validate).called(1);
|
||||
verify(projectScopeValidator.validate).called(1);
|
||||
});
|
||||
verify(
|
||||
() => logger.info(any(that: contains('No issues detected!'))),
|
||||
).called(1);
|
||||
});
|
||||
|
||||
group('fix', () {
|
||||
@@ -180,10 +143,9 @@ void main() {
|
||||
when(fixableWarningValidator.validate).thenAnswer(
|
||||
(_) async => [fixableValidationWarning],
|
||||
);
|
||||
when(() => fixableWarningValidator.scope)
|
||||
.thenReturn(ValidatorScope.installation);
|
||||
when(() => fixableWarningValidator.description)
|
||||
.thenReturn('fixable warning validator');
|
||||
when(fixableWarningValidator.canRunInCurrentContext).thenReturn(true);
|
||||
});
|
||||
|
||||
test(
|
||||
|
||||
+31
-39
@@ -1,15 +1,10 @@
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:collection/collection.dart';
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:scoped/scoped.dart';
|
||||
import 'package:shorebird_cli/src/process.dart';
|
||||
import 'package:shorebird_cli/src/validators/validators.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
class _MockShorebirdProcess extends Mock implements ShorebirdProcess {}
|
||||
|
||||
void main() {
|
||||
const manifestWithInternetPermission = '''
|
||||
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
|
||||
@@ -39,21 +34,6 @@ void main() {
|
||||
''';
|
||||
|
||||
group(AndroidInternetPermissionValidator, () {
|
||||
late ShorebirdProcess shorebirdProcess;
|
||||
|
||||
R runWithOverrides<R>(R Function() body) {
|
||||
return runScoped(
|
||||
() => body(),
|
||||
values: {
|
||||
processRef.overrideWith(() => shorebirdProcess),
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
setUp(() {
|
||||
shorebirdProcess = _MockShorebirdProcess();
|
||||
});
|
||||
|
||||
Directory createTempDir() => Directory.systemTemp.createTempSync();
|
||||
|
||||
void writeManifestToPath(String manifestContents, String path) {
|
||||
@@ -66,11 +46,32 @@ void main() {
|
||||
expect(AndroidInternetPermissionValidator().description, isNotEmpty);
|
||||
});
|
||||
|
||||
test('is a project-specific validator', () {
|
||||
expect(
|
||||
AndroidInternetPermissionValidator().scope,
|
||||
ValidatorScope.project,
|
||||
);
|
||||
group('canRunInContext', () {
|
||||
test('returns false if no android src directory exists', () {
|
||||
final tempDirectory = createTempDir();
|
||||
|
||||
final result = IOOverrides.runZoned(
|
||||
() => AndroidInternetPermissionValidator().canRunInCurrentContext(),
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
|
||||
expect(result, isFalse);
|
||||
});
|
||||
|
||||
test('returns true if an android src directory exists', () {
|
||||
final tempDirectory = createTempDir();
|
||||
writeManifestToPath(
|
||||
manifestWithInternetPermission,
|
||||
p.join(tempDirectory.path, 'android', 'app', 'src', 'main'),
|
||||
);
|
||||
|
||||
final result = IOOverrides.runZoned(
|
||||
() => AndroidInternetPermissionValidator().canRunInCurrentContext(),
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
|
||||
expect(result, isTrue);
|
||||
});
|
||||
});
|
||||
|
||||
test(
|
||||
@@ -88,7 +89,7 @@ void main() {
|
||||
);
|
||||
|
||||
final results = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(AndroidInternetPermissionValidator().validate),
|
||||
AndroidInternetPermissionValidator().validate,
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
|
||||
@@ -96,15 +97,6 @@ void main() {
|
||||
},
|
||||
);
|
||||
|
||||
test('returns an error if no android project is found', () async {
|
||||
final results = await AndroidInternetPermissionValidator().validate();
|
||||
|
||||
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',
|
||||
() async {
|
||||
final tempDirectory = createTempDir();
|
||||
@@ -112,7 +104,7 @@ void main() {
|
||||
.createSync(recursive: true);
|
||||
|
||||
final results = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(AndroidInternetPermissionValidator().validate),
|
||||
AndroidInternetPermissionValidator().validate,
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
|
||||
@@ -168,7 +160,7 @@ void main() {
|
||||
);
|
||||
|
||||
final results = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(AndroidInternetPermissionValidator().validate),
|
||||
AndroidInternetPermissionValidator().validate,
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
|
||||
@@ -204,7 +196,7 @@ void main() {
|
||||
);
|
||||
|
||||
var results = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(AndroidInternetPermissionValidator().validate),
|
||||
AndroidInternetPermissionValidator().validate,
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
expect(results, hasLength(1));
|
||||
@@ -216,7 +208,7 @@ void main() {
|
||||
);
|
||||
|
||||
results = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(AndroidInternetPermissionValidator().validate),
|
||||
AndroidInternetPermissionValidator().validate,
|
||||
getCurrentDirectory: () => tempDirectory,
|
||||
);
|
||||
expect(results, isEmpty);
|
||||
|
||||
@@ -117,8 +117,8 @@ Tools • Dart 2.19.6 • DevTools 2.20.1
|
||||
expect(validator.description, isNotEmpty);
|
||||
});
|
||||
|
||||
test('is not project-specific', () {
|
||||
expect(validator.scope, ValidatorScope.installation);
|
||||
test('canRunInContext always returns true', () {
|
||||
expect(validator.canRunInCurrentContext(), isTrue);
|
||||
});
|
||||
|
||||
test('returns no issues when the Flutter install is good', () async {
|
||||
|
||||
@@ -38,8 +38,8 @@ void main() {
|
||||
expect(validator.description, isNotEmpty);
|
||||
});
|
||||
|
||||
test('is not project-specific', () {
|
||||
expect(validator.scope, ValidatorScope.installation);
|
||||
test('canRunInContext always returns true', () {
|
||||
expect(validator.canRunInCurrentContext(), isTrue);
|
||||
});
|
||||
|
||||
test('returns no issues when shorebird is up-to-date', () async {
|
||||
|
||||
Reference in New Issue
Block a user