Unverified Commit 317e4cb0 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

don't warn on CHROME_EXECUTABLE missing if we've already located it. (#34079)

parent 938e4eb6
......@@ -463,6 +463,19 @@ class ValidationMessage {
@override
String toString() => message;
@override
bool operator ==(Object other) {
if (other.runtimeType != runtimeType) {
return false;
}
final ValidationMessage typedOther = other;
return typedOther.message == message
&& typedOther.type == type;
}
@override
int get hashCode => type.hashCode ^ message.hashCode;
}
class FlutterValidator extends DoctorValidator {
......
......@@ -71,7 +71,7 @@ class WebDevice extends Device {
Future<bool> get isLocalEmulator async => false;
@override
bool isSupported() => flutterWebEnabled;
bool isSupported() => flutterWebEnabled && canFindChrome();
@override
String get name => 'web';
......@@ -81,6 +81,9 @@ class WebDevice extends Device {
@override
Future<String> get sdkNameAndVersion async {
if (!isSupported()) {
return 'unknown';
}
// See https://bugs.chromium.org/p/chromium/issues/detail?id=158372
String version = 'unknown';
if (platform.isWindows) {
......
......@@ -17,10 +17,17 @@ class WebValidator extends DoctorValidator {
final bool canRunChrome = canFindChrome();
final List<ValidationMessage> messages = <ValidationMessage>[];
if (platform.environment.containsKey(kChromeEnvironment)) {
if (!canRunChrome) {
messages.add(ValidationMessage.hint('$chrome is not executable.'));
} else {
messages.add(ValidationMessage('$kChromeEnvironment = $chrome'));
}
} else {
messages.add(ValidationMessage('Chrome at $chrome'));
if (!canRunChrome) {
messages.add(ValidationMessage.hint('$kChromeEnvironment not set'));
} else {
messages.add(ValidationMessage('Chrome at $chrome'));
}
}
if (!canRunChrome) {
return ValidationResult(
......
......@@ -3,7 +3,6 @@
// found in the LICENSE file.
import '../base/context.dart';
import '../base/file_system.dart';
import '../base/platform.dart';
import '../base/process_manager.dart';
import '../doctor.dart';
......@@ -40,12 +39,9 @@ class WebWorkflow extends Workflow {
/// Whether we can locate the chrome executable.
bool canFindChrome() {
final String chrome = findChromeExecutable();
if (platform.isLinux) {
try {
return processManager.canRun(chrome);
} else if (platform.isMacOS) {
return fs.file(chrome).existsSync();
} else if (platform.isWindows) {
return fs.file(chrome).existsSync();
}
} on ArgumentError {
return false;
}
}
......@@ -28,14 +28,13 @@ void main() {
});
testUsingContext('Invokes version command on non-Windows platforms', () async{
when(mockPlatform.isWindows).thenReturn(false);
when(mockPlatform.environment).thenReturn(<String, String>{
kChromeEnvironment: 'chrome.foo'
});
when(mockProcessManager.canRun('chrome.foo')).thenReturn(true);
when(mockProcessManager.run(<String>['chrome.foo', '--version'])).thenAnswer((Invocation invocation) async {
return MockProcessResult(0, 'ABC');
});
final WebDevice webDevice = WebDevice();
expect(webDevice.isSupported(), true);
expect(await webDevice.sdkNameAndVersion, 'ABC');
}, overrides: <Type, Generator>{
Platform: () => mockPlatform,
......@@ -44,6 +43,7 @@ void main() {
testUsingContext('Invokes different version command on windows.', () async {
when(mockPlatform.isWindows).thenReturn(true);
when(mockProcessManager.canRun('chrome.foo')).thenReturn(true);
when(mockProcessManager.run(<String>[
'reg',
'query',
......@@ -55,6 +55,7 @@ void main() {
});
final WebDevice webDevice = WebDevice();
expect(webDevice.isSupported(), true);
expect(await webDevice.sdkNameAndVersion, 'Google Chrome 74.0.0');
}, overrides: <Type, Generator>{
Platform: () => mockPlatform,
......@@ -64,7 +65,10 @@ void main() {
}
class MockChromeLauncher extends Mock implements ChromeLauncher {}
class MockPlatform extends Mock implements Platform {}
class MockPlatform extends Mock implements Platform {
@override
Map<String, String> environment = <String, String>{'FLUTTER_WEB': 'true', kChromeEnvironment: 'chrome.foo'};
}
class MockProcessManager extends Mock implements ProcessManager {}
class MockProcessResult extends Mock implements ProcessResult {
MockProcessResult(this.exitCode, this.stdout);
......
......@@ -2,12 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/doctor.dart';
import 'package:flutter_tools/src/web/chrome.dart';
import 'package:flutter_tools/src/web/web_validator.dart';
import 'package:mockito/mockito.dart';
import 'package:process/process.dart';
import '../src/common.dart';
import '../src/testbed.dart';
......@@ -17,14 +17,16 @@ void main() {
Testbed testbed;
WebValidator webValidator;
MockPlatform mockPlatform;
MockProcessManager mockProcessManager;
setUp(() {
mockProcessManager = MockProcessManager();
testbed = Testbed(setup: () {
fs.file(kMacOSExecutable).createSync(recursive: true);
fs.file('chrome_foo').createSync();
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(true);
return null;
}, overrides: <Type, Generator>{
Platform: () => mockPlatform,
ProcessManager: () => mockProcessManager,
});
webValidator = const WebValidator();
mockPlatform = MockPlatform();
......@@ -39,10 +41,19 @@ void main() {
}));
test('Can notice missing macOS executable ', () => testbed.run(() async {
fs.file(kMacOSExecutable).deleteSync();
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(false);
final ValidationResult result = await webValidator.validate();
expect(result.type, ValidationType.missing);
}));
test('Doesn\'t warn about CHROME_EXECUTABLE unless it cant find chrome ', () => testbed.run(() async {
when(mockProcessManager.canRun(kMacOSExecutable)).thenReturn(false);
final ValidationResult result = await webValidator.validate();
expect(result.messages, <ValidationMessage>[
ValidationMessage.hint('CHROME_EXECUTABLE not set')
]);
expect(result.type, ValidationType.missing);
}));
});
}
......@@ -50,3 +61,5 @@ class MockPlatform extends Mock implements Platform {
@override
Map<String, String> get environment => const <String, String>{};
}
class MockProcessManager extends Mock implements ProcessManager {}
\ No newline at end of file
......@@ -83,6 +83,7 @@ void main() {
}));
test('does not apply on other platforms', () => testbed.run(() {
when(mockProcessManager.canRun('chrome')).thenReturn(false);
expect(workflow.appliesToHostPlatform, false);
expect(workflow.canLaunchDevices, false);
expect(workflow.canListDevices, false);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment