Skip to content

Commit 0d9a1b2

Browse files
jonahwilliamsjonahwilliams
authored
Remove environment variable guards for command line desktop and web (flutter#33867)
1 parent 42d8383 commit 0d9a1b2

File tree

11 files changed

+64
-30
lines changed

11 files changed

+64
-30
lines changed

‎packages/flutter_tools/lib/src/commands/daemon.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ import '../vmservice.dart';
2828

2929
const String protocolVersion = '0.5.2';
3030

31+
/// Whether the tool started from the daemon, as opposed to the command line.
32+
bool get isRunningFromDaemon => _isRunningFromDaemon;
33+
bool _isRunningFromDaemon = false;
34+
3135
/// A server process command. This command will start up a long-lived server.
3236
/// It reads JSON-RPC based commands from stdin, executes them, and returns
3337
/// JSON-RPC based responses and events to stdout.
@@ -49,6 +53,7 @@ class DaemonCommand extends FlutterCommand {
4953
@override
5054
Future<FlutterCommandResult> runCommand() async {
5155
printStatus('Starting device daemon...');
56+
_isRunningFromDaemon = true;
5257

5358
final NotifyingLogger notifyingLogger = NotifyingLogger();
5459

‎packages/flutter_tools/lib/src/desktop.dart

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,32 @@
44

55
import 'dart:async';
66

7+
import 'package:meta/meta.dart';
8+
79
import 'base/io.dart';
810
import 'base/platform.dart';
911
import 'base/process_manager.dart';
12+
import 'commands/daemon.dart' show isRunningFromDaemon;
1013
import 'convert.dart';
1114
import 'device.dart';
1215
import 'version.dart';
1316

14-
// Only launch or display desktop embedding devices if
15-
// `ENABLE_FLUTTER_DESKTOP` environment variable is set to true.
17+
@visibleForTesting
18+
bool debugDisableDesktop = false;
19+
20+
/// Only launch or display desktop embedding devices from the command line
21+
/// or if `ENABLE_FLUTTER_DESKTOP` environment variable is set to true.
1622
bool get flutterDesktopEnabled {
17-
_flutterDesktopEnabled ??= platform.environment['ENABLE_FLUTTER_DESKTOP']?.toLowerCase() == 'true';
18-
return _flutterDesktopEnabled && !FlutterVersion.instance.isStable;
23+
if (debugDisableDesktop) {
24+
return false;
25+
}
26+
if (isRunningFromDaemon) {
27+
final bool platformEnabled = platform
28+
.environment['ENABLE_FLUTTER_DESKTOP']?.toLowerCase() == 'true';
29+
return platformEnabled && !FlutterVersion.instance.isStable;
30+
}
31+
return !FlutterVersion.instance.isStable;
1932
}
20-
bool _flutterDesktopEnabled;
2133

2234
/// Kills a process on linux or macOS.
2335
Future<bool> killProcess(String executable) async {

‎packages/flutter_tools/lib/src/linux/linux_workflow.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ class LinuxWorkflow implements Workflow {
2121
bool get appliesToHostPlatform => platform.isLinux;
2222

2323
@override
24-
bool get canLaunchDevices => flutterDesktopEnabled;
24+
bool get canLaunchDevices => platform.isLinux && flutterDesktopEnabled;
2525

2626
@override
27-
bool get canListDevices => flutterDesktopEnabled;
27+
bool get canListDevices => platform.isLinux && flutterDesktopEnabled;
2828

2929
@override
3030
bool get canListEmulators => false;

‎packages/flutter_tools/lib/src/macos/macos_workflow.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ class MacOSWorkflow implements Workflow {
2121
bool get appliesToHostPlatform => platform.isMacOS;
2222

2323
@override
24-
bool get canLaunchDevices => flutterDesktopEnabled;
24+
bool get canLaunchDevices => platform.isMacOS && flutterDesktopEnabled;
2525

2626
@override
27-
bool get canListDevices => flutterDesktopEnabled;
27+
bool get canListDevices => platform.isMacOS && flutterDesktopEnabled;
2828

2929
@override
3030
bool get canListEmulators => false;

‎packages/flutter_tools/lib/src/web/web_device.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import '../base/process_manager.dart';
1212
import '../build_info.dart';
1313
import '../device.dart';
1414
import '../project.dart';
15-
import '../web/workflow.dart';
1615
import 'chrome.dart';
16+
import 'workflow.dart';
1717

1818
class WebApplicationPackage extends ApplicationPackage {
1919
WebApplicationPackage(this.flutterProject) : super(id: flutterProject.manifest.appName);

‎packages/flutter_tools/lib/src/web/workflow.dart

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,32 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:meta/meta.dart';
6+
57
import '../base/context.dart';
68
import '../base/platform.dart';
79
import '../base/process_manager.dart';
10+
import '../commands/daemon.dart' show isRunningFromDaemon;
811
import '../doctor.dart';
912
import '../version.dart';
1013
import 'chrome.dart';
1114

15+
@visibleForTesting
16+
bool debugDisableWeb = false;
17+
1218
/// Only launch or display web devices if `FLUTTER_WEB`
13-
/// environment variable is set to true.
19+
/// environment variable is set to true from the daemon.
1420
bool get flutterWebEnabled {
15-
_flutterWebEnabled = platform.environment['FLUTTER_WEB']?.toLowerCase() == 'true';
16-
return _flutterWebEnabled && !FlutterVersion.instance.isStable;
21+
if (debugDisableWeb) {
22+
return false;
23+
}
24+
if (isRunningFromDaemon) {
25+
final bool platformEnabled = platform
26+
.environment['FLUTTER_WEB']?.toLowerCase() == 'true';
27+
return platformEnabled && !FlutterVersion.instance.isStable;
28+
}
29+
return !FlutterVersion.instance.isStable;
1730
}
18-
bool _flutterWebEnabled;
1931

2032
/// The web workflow instance.
2133
WebWorkflow get webWorkflow => context.get<WebWorkflow>();

‎packages/flutter_tools/lib/src/windows/windows_workflow.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ class WindowsWorkflow implements Workflow {
2121
bool get appliesToHostPlatform => platform.isWindows;
2222

2323
@override
24-
bool get canLaunchDevices => flutterDesktopEnabled;
24+
bool get canLaunchDevices => platform.isWindows && flutterDesktopEnabled;
2525

2626
@override
27-
bool get canListDevices => flutterDesktopEnabled;
27+
bool get canListDevices => platform.isWindows && flutterDesktopEnabled;
2828

2929
@override
3030
bool get canListEmulators => false;

‎packages/flutter_tools/test/commands/devices_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ void main() {
2020
group('devices', () {
2121
setUpAll(() {
2222
Cache.disableLocking();
23+
// TODO(jonahwilliams): adjust the individual tests so they do not
24+
// depend on the host environment.
25+
debugDisableWebAndDesktop = true;
2326
});
2427

2528
testUsingContext('returns 0 when called', () async {

‎packages/flutter_tools/test/linux/linux_device_test.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ void main() {
2424
final MockProcessManager mockProcessManager = MockProcessManager();
2525

2626
when(notLinux.isLinux).thenReturn(false);
27-
when(notLinux.environment).thenReturn(const <String, String>{});
2827
when(mockProcessManager.run(<String>[
2928
'ps', 'aux',
3029
])).thenAnswer((Invocation invocation) async {

‎packages/flutter_tools/test/src/common.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import 'dart:async';
66

77
import 'package:args/command_runner.dart';
8+
import 'package:flutter_tools/src/desktop.dart';
9+
import 'package:flutter_tools/src/web/workflow.dart';
810
import 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf;
911
import 'package:test_api/test_api.dart' as test_package show TypeMatcher;
1012

@@ -18,6 +20,19 @@ import 'package:flutter_tools/src/runner/flutter_command_runner.dart';
1820

1921
export 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; // Defines a 'package:test' shim.
2022

23+
/// Disable both web and desktop to make testing easier. For example, prevent
24+
/// them from showing up in the devices list if the host happens to be setup
25+
/// properly.
26+
set debugDisableWebAndDesktop(bool value) {
27+
if (value) {
28+
debugDisableDesktop = true;
29+
debugDisableWeb = true;
30+
} else {
31+
debugDisableDesktop = false;
32+
debugDisableWeb = false;
33+
}
34+
}
35+
2136
/// A matcher that compares the type of the actual value to the type argument T.
2237
// TODO(ianh): Remove this once https://github.com/dart-lang/matcher/issues/98 is fixed
2338
Matcher isInstanceOf<T>() => test_package.TypeMatcher<T>();

‎packages/flutter_tools/test/web/workflow_test.dart

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import '../src/testbed.dart';
1717
void main() {
1818
group('WebWorkflow', () {
1919
Testbed testbed;
20-
MockPlatform noEnvironment;
2120
MockPlatform notSupported;
2221
MockPlatform windows;
2322
MockPlatform linux;
@@ -30,7 +29,6 @@ void main() {
3029
setUpAll(() {
3130
unstable = MockFlutterVersion(false);
3231
stable = MockFlutterVersion(true);
33-
noEnvironment = MockPlatform(environment: const <String, String>{});
3432
notSupported = MockPlatform(linux: false, windows: false, macos: false);
3533
windows = MockPlatform(windows: true);
3634
linux = MockPlatform(linux: true);
@@ -46,15 +44,6 @@ void main() {
4644
});
4745
});
4846

49-
test('does not apply if FLUTTER_WEB is not true', ()=> testbed.run(() {
50-
expect(workflow.appliesToHostPlatform, false);
51-
expect(workflow.canLaunchDevices, false);
52-
expect(workflow.canListDevices, false);
53-
expect(workflow.canListEmulators, false);
54-
}, overrides: <Type, Generator>{
55-
Platform: () => noEnvironment,
56-
}));
57-
5847
test('Applies on Linux', () => testbed.run(() {
5948
expect(workflow.appliesToHostPlatform, true);
6049
expect(workflow.canLaunchDevices, true);
@@ -92,7 +81,7 @@ void main() {
9281
Platform: () => notSupported,
9382
}));
9483

95-
test('does not apply on stable brnach', () => testbed.run(() {
84+
test('does not apply on stable branch', () => testbed.run(() {
9685
expect(workflow.appliesToHostPlatform, false);
9786
expect(workflow.canLaunchDevices, false);
9887
expect(workflow.canListDevices, false);
@@ -119,7 +108,6 @@ class MockPlatform extends Mock implements Platform {
119108
this.macos = false,
120109
this.linux = false,
121110
this.environment = const <String, String>{
122-
'FLUTTER_WEB': 'true',
123111
kChromeEnvironment: 'chrome',
124112
}});
125113

0 commit comments

Comments
 (0)