Skip to content

Commit

Permalink
[CP-stable]instead of exiting the tool, print a warning when using --…
Browse files Browse the repository at this point in the history
…flavor with an incompatible device (flutter#143785)

**This pull request is opened against a release branch.<br>
To request a cherry pick of a commit, please fill in the form below.** (Questions with an asterisk are required.)<br>

**To fill in the form, you can edit this PR description and type your answers after the 'My Answer' keywords. <br>
A flutter domain expert will evaluate this cherry pick request shortly after ALL questions are answered.**

* Issue Link: What is the link to the issue this cherry-pick is addressing?<br>

<pre>
  <b>My Answer:</b>
flutter#143574

</pre>

* Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers
        See https://github.com/flutter/flutter/wiki/Hotfix-Documentation-Best-Practices for examples (Bug fix, feature, docs update, ...)<br>

<pre>
  <b>My Answer:</b>

Instead of exiting, the `flutter` CLI tool now prints a warning when the `--flavor` option is used with a target platform that doesn't have flavors support.
</pre>

* impact_description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)<br>

<pre>
  <b>My Answer:</b>
All Flutter devs using the Flavors feature that use their IDE to select a target device.

</pre>

* Workaround: Is there a workaround for this issue?<br>

<pre>
  <b>My Answer:</b> Users can create duplicate debug launch configurations for target platforms that do not support the flavors feature.
</pre>

* Risk: What is the risk level of this cherry-pick?<br>

<pre>
  <b>My Answer:</b>

</pre>

* Test Coverage: Are you confident that your fix is well-tested by automated tests?<br>

<pre>
  <b>My Answer:</b>

</pre>

* Validation Steps: What are the steps to validate that this fix works?<br>

<pre>
  <b>My Answer:</b>

Set up a flutter project and configure at least one flavor for it. In VSCode, create a launch configuration that uses `--flavor` to select the flavor you configured (see flutter#143574 (comment) for an example). In VSCode, select chrome as the target device and try running the application using the configured launch configuration.

</pre>
  • Loading branch information
flutteractionsbot authored Feb 20, 2024
1 parent bae5e49 commit abb292a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 22 deletions.
6 changes: 5 additions & 1 deletion packages/flutter_tools/lib/src/commands/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,11 @@ class RunCommand extends RunCommandBase {
final bool flavorsSupportedOnEveryDevice = devices!
.every((Device device) => device.supportsFlavors);
if (flavor != null && !flavorsSupportedOnEveryDevice) {
throwToolExit('--flavor is only supported for Android, macOS, and iOS devices.');
globals.printWarning(
'--flavor is only supported for Android, macOS, and iOS devices. '
'Flavor-related features may not function properly and could '
'behave differently in a future release.'
);
}
}

Expand Down
45 changes: 24 additions & 21 deletions packages/flutter_tools/test/commands.shard/hermetic/run_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ void main() {
});

group('run', () {
late BufferLogger logger;
late TestDeviceManager testDeviceManager;
late FileSystem fileSystem;

setUp(() {
testDeviceManager = TestDeviceManager(logger: BufferLogger.test());
logger = BufferLogger.test();
testDeviceManager = TestDeviceManager(logger: logger);
fileSystem = MemoryFileSystem.test();
});

Expand All @@ -65,7 +67,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
});

testUsingContext('does not support --no-sound-null-safety by default', () async {
Expand All @@ -89,7 +91,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
});

testUsingContext('supports --no-sound-null-safety with an overridden NonNullSafeBuilds', () async {
Expand All @@ -109,7 +111,7 @@ void main() {
}, overrides: <Type, Generator>{
DeviceManager: () => testDeviceManager,
FileSystem: () => fileSystem,
Logger: () => BufferLogger.test(),
Logger: () => logger,
NonNullSafeBuilds: () => NonNullSafeBuilds.allowed,
ProcessManager: () => FakeProcessManager.any(),
});
Expand Down Expand Up @@ -137,7 +139,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
});

testUsingContext('Walks upward looking for a pubspec.yaml and succeeds if found', () async {
Expand Down Expand Up @@ -165,7 +167,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
});

testUsingContext('Walks upward looking for a pubspec.yaml and exits if missing', () async {
Expand All @@ -185,7 +187,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
});

group('run app', () {
Expand Down Expand Up @@ -431,7 +433,7 @@ void main() {
Cache: () => Cache.test(processManager: FakeProcessManager.any()),
});

testUsingContext('fails when --flavor is used with an unsupported target platform', () async {
testUsingContext('prints warning when --flavor is used with an unsupported target platform', () async {
const List<String> runCommand = <String>[
'run',
'--no-pub',
Expand All @@ -440,24 +442,25 @@ void main() {
'-d',
'all',
];

// Useful for test readability.
// ignore: avoid_redundant_argument_values
final FakeDevice deviceWithoutFlavorSupport = FakeDevice(supportsFlavors: false);
final FakeDevice deviceWithFlavorSupport = FakeDevice(supportsFlavors: true);
testDeviceManager.devices = <Device>[deviceWithoutFlavorSupport, deviceWithFlavorSupport];

await expectLater(
() => createTestCommandRunner(RunCommand()).run(runCommand),
throwsToolExit(
message: '--flavor is only supported for Android, macOS, and iOS devices.',
),
);
await createTestCommandRunner(TestRunCommandThatOnlyValidates()).run(runCommand);

expect(logger.warningText, contains(
'--flavor is only supported for Android, macOS, and iOS devices. '
'Flavor-related features may not function properly and could '
'behave differently in a future release.'
));
}, overrides: <Type, Generator>{
DeviceManager: () => testDeviceManager,
FileSystem: () => fs,
ProcessManager: () => FakeProcessManager.any(),
Cache: () => Cache.test(processManager: FakeProcessManager.any()),
Logger: () => logger,
});

testUsingContext('forwards --uninstall-only to DebuggingOptions', () async {
Expand Down Expand Up @@ -677,7 +680,7 @@ void main() {
ProcessManager: () => FakeProcessManager.any(),
Usage: () => usage,
Stdio: () => FakeStdio(),
Logger: () => AppRunLogger(parent: BufferLogger.test()),
Logger: () => AppRunLogger(parent: logger),
});

testUsingContext('can disable devtools with --no-devtools', () async {
Expand Down Expand Up @@ -705,7 +708,7 @@ void main() {
ProcessManager: () => FakeProcessManager.any(),
Usage: () => usage,
Stdio: () => FakeStdio(),
Logger: () => AppRunLogger(parent: BufferLogger.test()),
Logger: () => AppRunLogger(parent: logger),
});
});
});
Expand Down Expand Up @@ -1014,7 +1017,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
DeviceManager: () => testDeviceManager,
});

Expand All @@ -1035,7 +1038,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
DeviceManager: () => testDeviceManager,
});

Expand All @@ -1059,7 +1062,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
DeviceManager: () => testDeviceManager,
});

Expand All @@ -1079,7 +1082,7 @@ void main() {
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
Logger: () => logger,
DeviceManager: () => testDeviceManager,
});
});
Expand Down

0 comments on commit abb292a

Please sign in to comment.