Skip to content

Commit afdc783

Browse files
author
Emmanuel Garcia
authored
Report commands that resulted in success or failure (flutter#34288)
This is added as a dimension cd26
1 parent 3b42341 commit afdc783

File tree

7 files changed

+190
-42
lines changed

7 files changed

+190
-42
lines changed

packages/flutter_tools/lib/src/commands/attach.dart

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import '../resident_runner.dart';
2626
import '../run_cold.dart';
2727
import '../run_hot.dart';
2828
import '../runner/flutter_command.dart';
29-
import '../usage.dart';
3029

3130
/// A Flutter-command that attaches to applications that have been launched
3231
/// without `flutter run`.
@@ -316,10 +315,7 @@ class AttachCommand extends FlutterCommand {
316315
result = await runner.attach();
317316
assert(result != null);
318317
}
319-
if (result == 0) {
320-
flutterUsage.sendEvent('attach', 'success');
321-
} else {
322-
flutterUsage.sendEvent('attach', 'failure');
318+
if (result != 0) {
323319
throwToolExit(null, exitCode: result);
324320
}
325321
} finally {

packages/flutter_tools/lib/src/commands/logs.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ class LogsCommand extends FlutterCommand {
3232
Device device;
3333

3434
@override
35-
Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async {
35+
Future<FlutterCommandResult> verifyThenRunCommand() async {
3636
device = await findTargetDevice();
3737
if (device == null)
3838
throwToolExit(null);
39-
return super.verifyThenRunCommand(commandPath);
39+
return super.verifyThenRunCommand();
4040
}
4141

4242
@override

packages/flutter_tools/lib/src/commands/run.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,21 @@ class RunCommand extends RunCommandBase {
219219

220220
@override
221221
Future<Map<String, String>> get usageValues async {
222-
final bool isEmulator = await devices[0].isLocalEmulator;
223222
String deviceType, deviceOsVersion;
224-
if (devices.length == 1) {
223+
bool isEmulator;
224+
225+
if (devices == null || devices.isEmpty) {
226+
deviceType = 'none';
227+
deviceOsVersion = 'none';
228+
isEmulator = false;
229+
} else if (devices.length == 1) {
225230
deviceType = getNameForTargetPlatform(await devices[0].targetPlatform);
226231
deviceOsVersion = await devices[0].sdkNameAndVersion;
232+
isEmulator = await devices[0].isLocalEmulator;
227233
} else {
228234
deviceType = 'multiple';
229235
deviceOsVersion = 'multiple';
236+
isEmulator = false;
230237
}
231238
final String modeName = getBuildInfo().modeName;
232239
final AndroidProject androidProject = FlutterProject.current().android;

packages/flutter_tools/lib/src/commands/screenshot.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ class ScreenshotCommand extends FlutterCommand {
6464
Device device;
6565

6666
@override
67-
Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async {
67+
Future<FlutterCommandResult> verifyThenRunCommand() async {
6868
device = await findTargetDevice();
6969
if (device == null)
7070
throwToolExit('Must have a connected device');
7171
if (argResults[_kType] == _kDeviceType && !device.supportsScreenshot)
7272
throwToolExit('Screenshot not supported for ${device.name}.');
7373
if (argResults[_kType] != _kDeviceType && argResults[_kObservatoryUri] == null)
7474
throwToolExit('Observatory URI must be specified for screenshot type ${argResults[_kType]}');
75-
return super.verifyThenRunCommand(commandPath);
75+
return super.verifyThenRunCommand();
7676
}
7777

7878
@override

packages/flutter_tools/lib/src/runner/flutter_command.dart

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -408,43 +408,78 @@ abstract class FlutterCommand extends Command<void> {
408408
body: () async {
409409
if (flutterUsage.isFirstRun)
410410
flutterUsage.printWelcome();
411-
final String commandPath = await usagePath;
412411
FlutterCommandResult commandResult;
413412
try {
414-
commandResult = await verifyThenRunCommand(commandPath);
413+
commandResult = await verifyThenRunCommand();
415414
} on ToolExit {
416415
commandResult = const FlutterCommandResult(ExitStatus.fail);
417416
rethrow;
418417
} finally {
419418
final DateTime endTime = systemClock.now();
420419
printTrace(userMessages.flutterElapsedTime(name, getElapsedAsMilliseconds(endTime.difference(startTime))));
421420
printTrace('"flutter $name" took ${getElapsedAsMilliseconds(endTime.difference(startTime))}.');
422-
if (commandPath != null) {
423-
final List<String> labels = <String>[];
424-
if (commandResult?.exitStatus != null)
425-
labels.add(getEnumName(commandResult.exitStatus));
426-
if (commandResult?.timingLabelParts?.isNotEmpty ?? false)
427-
labels.addAll(commandResult.timingLabelParts);
428-
429-
final String label = labels
430-
.where((String label) => !isBlank(label))
431-
.join('-');
432-
flutterUsage.sendTiming(
433-
'flutter',
434-
name,
435-
// If the command provides its own end time, use it. Otherwise report
436-
// the duration of the entire execution.
437-
(commandResult?.endTimeOverride ?? endTime).difference(startTime),
438-
// Report in the form of `success-[parameter1-parameter2]`, all of which
439-
// can be null if the command doesn't provide a FlutterCommandResult.
440-
label: label == '' ? null : label,
441-
);
442-
}
421+
422+
await _sendUsage(commandResult, startTime, endTime);
443423
}
444424
},
445425
);
446426
}
447427

428+
/// Logs data about this command.
429+
///
430+
/// For example, the command path (e.g. `build/apk`) and the result,
431+
/// as well as the time spent running it.
432+
Future<void> _sendUsage(FlutterCommandResult commandResult, DateTime startTime, DateTime endTime) async {
433+
final String commandPath = await usagePath;
434+
435+
if (commandPath == null) {
436+
return;
437+
}
438+
439+
// Send screen.
440+
final Map<String, String> additionalUsageValues = <String, String>{};
441+
final Map<String, String> currentUsageValues = await usageValues;
442+
443+
if (currentUsageValues != null) {
444+
additionalUsageValues.addAll(currentUsageValues);
445+
}
446+
if (commandResult != null) {
447+
switch (commandResult.exitStatus) {
448+
case ExitStatus.success:
449+
additionalUsageValues[kCommandResult] = 'success';
450+
break;
451+
case ExitStatus.warning:
452+
additionalUsageValues[kCommandResult] = 'warning';
453+
break;
454+
case ExitStatus.fail:
455+
additionalUsageValues[kCommandResult] = 'fail';
456+
break;
457+
}
458+
}
459+
flutterUsage.sendCommand(commandPath, parameters: additionalUsageValues);
460+
461+
// Send timing.
462+
final List<String> labels = <String>[];
463+
if (commandResult?.exitStatus != null)
464+
labels.add(getEnumName(commandResult.exitStatus));
465+
if (commandResult?.timingLabelParts?.isNotEmpty ?? false)
466+
labels.addAll(commandResult.timingLabelParts);
467+
468+
final String label = labels
469+
.where((String label) => !isBlank(label))
470+
.join('-');
471+
flutterUsage.sendTiming(
472+
'flutter',
473+
name,
474+
// If the command provides its own end time, use it. Otherwise report
475+
// the duration of the entire execution.
476+
(commandResult?.endTimeOverride ?? endTime).difference(startTime),
477+
// Report in the form of `success-[parameter1-parameter2]`, all of which
478+
// can be null if the command doesn't provide a FlutterCommandResult.
479+
label: label == '' ? null : label,
480+
);
481+
}
482+
448483
/// Perform validation then call [runCommand] to execute the command.
449484
/// Return a [Future] that completes with an exit code
450485
/// indicating whether execution was successful.
@@ -453,7 +488,7 @@ abstract class FlutterCommand extends Command<void> {
453488
/// then call this method to execute the command
454489
/// rather than calling [runCommand] directly.
455490
@mustCallSuper
456-
Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async {
491+
Future<FlutterCommandResult> verifyThenRunCommand() async {
457492
await validateCommand();
458493

459494
// Populate the cache. We call this before pub get below so that the sky_engine
@@ -470,11 +505,6 @@ abstract class FlutterCommand extends Command<void> {
470505

471506
setupApplicationPackages();
472507

473-
if (commandPath != null) {
474-
final Map<String, String> additionalUsageValues = await usageValues;
475-
flutterUsage.sendCommand(commandPath, parameters: additionalUsageValues);
476-
}
477-
478508
return await runCommand();
479509
}
480510

packages/flutter_tools/lib/src/usage.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ const String kCommandPackagesProjectModule = 'cd21';
4747

4848
const String kCommandBuildBundleTargetPlatform = 'cd24';
4949
const String kCommandBuildBundleIsModule = 'cd25';
50-
// Next ID: cd26
50+
51+
const String kCommandResult = 'cd26';
52+
// Next ID: cd27
5153

5254
Usage get flutterUsage => Usage.instance;
5355

packages/flutter_tools/test/runner/flutter_command_test.dart

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,114 @@ void main() {
4949
Cache: () => cache,
5050
});
5151

52+
testUsingContext('reports command that results in success', () async {
53+
// Crash if called a third time which is unexpected.
54+
mockTimes = <int>[1000, 2000];
55+
56+
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
57+
commandFunction: () async {
58+
return const FlutterCommandResult(ExitStatus.success);
59+
}
60+
);
61+
await flutterCommand.run();
62+
63+
expect(
64+
verify(usage.sendCommand(captureAny,
65+
parameters: captureAnyNamed('parameters'))).captured,
66+
<dynamic>[
67+
'dummy',
68+
const <String, String>{'cd26': 'success'}
69+
],
70+
);
71+
},
72+
overrides: <Type, Generator>{
73+
SystemClock: () => clock,
74+
Usage: () => usage,
75+
});
76+
77+
testUsingContext('reports command that results in warning', () async {
78+
// Crash if called a third time which is unexpected.
79+
mockTimes = <int>[1000, 2000];
80+
81+
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
82+
commandFunction: () async {
83+
return const FlutterCommandResult(ExitStatus.warning);
84+
}
85+
);
86+
await flutterCommand.run();
87+
88+
expect(
89+
verify(usage.sendCommand(captureAny,
90+
parameters: captureAnyNamed('parameters'))).captured,
91+
<dynamic>[
92+
'dummy',
93+
const <String, String>{'cd26': 'warning'}
94+
],
95+
);
96+
},
97+
overrides: <Type, Generator>{
98+
SystemClock: () => clock,
99+
Usage: () => usage,
100+
});
101+
102+
testUsingContext('reports command that results in failure', () async {
103+
// Crash if called a third time which is unexpected.
104+
mockTimes = <int>[1000, 2000];
105+
106+
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
107+
commandFunction: () async {
108+
return const FlutterCommandResult(ExitStatus.fail);
109+
}
110+
);
111+
112+
try {
113+
await flutterCommand.run();
114+
} on ToolExit {
115+
expect(
116+
verify(usage.sendCommand(captureAny,
117+
parameters: captureAnyNamed('parameters'))).captured,
118+
<dynamic>[
119+
'dummy',
120+
const <String, String>{'cd26': 'fail'}
121+
],
122+
);
123+
}
124+
},
125+
overrides: <Type, Generator>{
126+
SystemClock: () => clock,
127+
Usage: () => usage,
128+
});
129+
130+
testUsingContext('reports command that results in error', () async {
131+
// Crash if called a third time which is unexpected.
132+
mockTimes = <int>[1000, 2000];
133+
134+
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
135+
commandFunction: () async {
136+
throwToolExit('fail');
137+
return null; // unreachable
138+
}
139+
);
140+
141+
try {
142+
await flutterCommand.run();
143+
fail('Mock should make this fail');
144+
} on ToolExit {
145+
expect(
146+
verify(usage.sendCommand(captureAny,
147+
parameters: captureAnyNamed('parameters'))).captured,
148+
<dynamic>[
149+
'dummy',
150+
const <String, String>{'cd26': 'fail'}
151+
],
152+
);
153+
}
154+
},
155+
overrides: <Type, Generator>{
156+
SystemClock: () => clock,
157+
Usage: () => usage,
158+
});
159+
52160
testUsingContext('report execution timing by default', () async {
53161
// Crash if called a third time which is unexpected.
54162
mockTimes = <int>[1000, 2000];
@@ -61,7 +169,12 @@ void main() {
61169
verify(usage.sendTiming(
62170
captureAny, captureAny, captureAny,
63171
label: captureAnyNamed('label'))).captured,
64-
<dynamic>['flutter', 'dummy', const Duration(milliseconds: 1000), null],
172+
<dynamic>[
173+
'flutter',
174+
'dummy',
175+
const Duration(milliseconds: 1000),
176+
null
177+
],
65178
);
66179
},
67180
overrides: <Type, Generator>{

0 commit comments

Comments
 (0)