Unverified Commit 0d587527 authored by Elias Yishak's avatar Elias Yishak Committed by GitHub

Clearer text about what happens with `--disable-telemetry` + enable-telemetry command (#125995)

Fixes:
- https://github.com/flutter/flutter/issues/124411

This PR is cleaning up the `--disable-telemetry` help message to make it clear that opting out will opt out of all telemetry collection for flutter and dart commands. It is also adding the opposite flag `--enable-telemetry` which will enable telemetry collection
parent 6db4162a
......@@ -63,31 +63,55 @@ Future<int> run(
StackTrace? firstStackTrace;
return runZoned<Future<int>>(() async {
try {
if (args.contains('--disable-telemetry') &&
args.contains('--enable-telemetry')) {
throwToolExit(
'Both enable and disable telemetry commands were detected '
'when only one can be supplied per invocation.',
exitCode: 1);
}
// Disable analytics if user passes in the `--disable-telemetry` option
// `flutter --disable-telemetry`
//
// Same functionality as `flutter config --no-analytics` for disabling
// except with the `value` hard coded as false
if (args.contains('--disable-telemetry')) {
const bool value = false;
// The tool sends the analytics event *before* toggling the flag
// intentionally to be sure that opt-out events are sent correctly.
AnalyticsConfigEvent(enabled: value).send();
if (!value) {
// Normally, the tool waits for the analytics to all send before the
// tool exits, but only when analytics are enabled. When reporting that
// analytics have been disable, the wait must be done here instead.
await globals.flutterUsage.ensureAnalyticsSent();
}
globals.flutterUsage.enabled = value;
AnalyticsConfigEvent(enabled: false).send();
// Normally, the tool waits for the analytics to all send before the
// tool exits, but only when analytics are enabled. When reporting that
// analytics have been disable, the wait must be done here instead.
await globals.flutterUsage.ensureAnalyticsSent();
globals.flutterUsage.enabled = false;
globals.printStatus('Analytics reporting disabled.');
// TODO(eliasyishak): Set the telemetry for the unified_analytics
// package as well, the above will be removed once we have
// fully transitioned to using the new package
await globals.analytics.setTelemetry(value);
await globals.analytics.setTelemetry(false);
}
// Enable analytics if user passes in the `--enable-telemetry` option
// `flutter --enable-telemetry`
//
// Same functionality as `flutter config --analytics` for enabling
// except with the `value` hard coded as true
if (args.contains('--enable-telemetry')) {
// The tool sends the analytics event *before* toggling the flag
// intentionally to be sure that opt-out events are sent correctly.
AnalyticsConfigEvent(enabled: true).send();
globals.flutterUsage.enabled = true;
globals.printStatus('Analytics reporting enabled.');
await globals.analytics.setTelemetry(true);
}
await runner.run(args);
// Triggering [runZoned]'s error callback does not necessarily mean that
......
......@@ -80,7 +80,12 @@ class FlutterCommandRunner extends CommandRunner<void> {
help: 'Suppress analytics reporting for the current CLI invocation.');
argParser.addFlag('disable-telemetry',
negatable: false,
help: 'Disable telemetry reporting when this command runs.');
help: 'Disable telemetry reporting each time a flutter or dart '
'command runs, until it is re-enabled.');
argParser.addFlag('enable-telemetry',
negatable: false,
help: 'Enable telemetry reporting each time a flutter or dart '
'command runs.');
argParser.addOption('packages',
hide: !verboseHelp,
help: 'Path to your "package_config.json" file.');
......@@ -188,8 +193,10 @@ class FlutterCommandRunner extends CommandRunner<void> {
Future<void> runCommand(ArgResults topLevelResults) async {
final Map<Type, Object?> contextOverrides = <Type, Object?>{};
// If the disable-telemetry flag has been passed, return out
if (topLevelResults.wasParsed('disable-telemetry')) {
// If the flag for enabling or disabling telemetry is passed in,
// we will return out
if (topLevelResults.wasParsed('disable-telemetry') ||
topLevelResults.wasParsed('enable-telemetry')) {
return;
}
......
......@@ -317,11 +317,6 @@ void main() {
});
group('unified_analytics', () {
final Usage legacyAnalytics = TestUsage();
setUp(() {
legacyAnalytics.enabled = false;
});
testUsingContext(
'runner disable telemetry with flag',
() async {
......@@ -346,6 +341,62 @@ void main() {
ProcessManager: () => FakeProcessManager.any(),
},
);
testUsingContext(
'runner enabling telemetry with flag',
() async {
io.setExitFunctionForTests((int exitCode) {});
expect(globals.analytics.telemetryEnabled, false);
expect(globals.analytics.shouldShowMessage, false);
await runner.run(
<String>['--enable-telemetry'],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
shutdownHooks: ShutdownHooks(),
);
expect(globals.analytics.telemetryEnabled, true);
},
overrides: <Type, Generator>{
Analytics: () => FakeAnalytics(fakeTelemetryStatusOverride: false),
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
},
);
testUsingContext(
'throw error when both flags passed',
() async {
io.setExitFunctionForTests((int exitCode) {});
expect(globals.analytics.telemetryEnabled, true);
expect(globals.analytics.shouldShowMessage, true);
final int exitCode = await runner.run(
<String>[
'--disable-telemetry',
'--enable-telemetry',
],
() => <FlutterCommand>[],
// This flutterVersion disables crash reporting.
flutterVersion: '[user-branch]/',
shutdownHooks: ShutdownHooks(),
);
expect(exitCode, 1,
reason: 'Should return 1 due to conflicting options for telemetry');
expect(globals.analytics.telemetryEnabled, true,
reason: 'Should not have changed from initialization');
},
overrides: <Type, Generator>{
Analytics: () => FakeAnalytics(),
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
},
);
});
}
......@@ -489,8 +540,16 @@ class WaitingCrashReporter implements CrashReporter {
/// A fake [Analytics] that will be used to test
/// the --disable-telemetry flag
class FakeAnalytics extends Fake implements Analytics {
bool _fakeTelemetryStatus = true;
bool _fakeShowMessage = true;
FakeAnalytics({bool fakeTelemetryStatusOverride = true})
: _fakeTelemetryStatus = fakeTelemetryStatusOverride,
_fakeShowMessage = fakeTelemetryStatusOverride;
// Both of the members below can be initialized with [fakeTelemetryStatusOverride]
// because if we pass in false for the status, that means we can also
// assume the message has been shown before
bool _fakeTelemetryStatus;
bool _fakeShowMessage;
@override
String get getConsentMessage => 'message';
......
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