Commit ec2d5833 authored by Emmanuel Garcia's avatar Emmanuel Garcia Committed by Flutter GitHub Bot

Make subcommands return success instead of null (#48100)

parent a8c5f4e8
......@@ -88,7 +88,6 @@ class AnalyzeCommand extends FlutterCommand {
runner.getRepoRoots(),
runner.getRepoPackages(),
).analyze();
return null;
} else {
await AnalyzeOnce(
argResults,
......@@ -96,7 +95,7 @@ class AnalyzeCommand extends FlutterCommand {
runner.getRepoPackages(),
workingDirectory: workingDirectory,
).analyze();
return null;
}
return FlutterCommandResult.success();
}
}
......@@ -200,7 +200,7 @@ class AssembleCommand extends FlutterCommand {
final Depfile depfile = Depfile(result.inputFiles, result.outputFiles);
depfile.writeToFile(globals.fs.file(depfileFile));
}
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -178,7 +178,7 @@ class AttachCommand extends FlutterCommand {
Artifacts: () => overrideArtifacts,
});
return null;
return FlutterCommandResult.success();
}
Future<void> _attachToDevice(Device device) async {
......
......@@ -119,7 +119,7 @@ class BuildAarCommand extends BuildSubCommand {
outputDirectoryPath: stringArg('output-dir'),
buildNumber: buildNumber,
);
return null;
return FlutterCommandResult.success();
}
/// Returns the [FlutterProject] which is determined from the remaining command-line
......
......@@ -89,6 +89,6 @@ class BuildAotCommand extends BuildSubCommand with TargetPlatformBasedDevelopmen
extraGenSnapshotOptions: stringsArg(FlutterOptions.kExtraGenSnapshotOptions),
dartDefines: dartDefines,
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -112,6 +112,6 @@ class BuildApkCommand extends BuildSubCommand {
target: targetFile,
androidBuildInfo: androidBuildInfo,
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -83,6 +83,6 @@ class BuildAppBundleCommand extends BuildSubCommand {
target: targetFile,
androidBuildInfo: androidBuildInfo,
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -139,6 +139,6 @@ class BuildBundleCommand extends BuildSubCommand {
fileSystemScheme: stringArg('filesystem-scheme'),
fileSystemRoots: stringsArg('filesystem-root'),
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -69,6 +69,6 @@ class BuildFuchsiaCommand extends BuildSubCommand {
buildInfo: buildInfo,
runnerPackageSource: stringArg('runner-source'),
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -92,6 +92,6 @@ class BuildIOSCommand extends BuildSubCommand {
globals.printStatus('Built ${result.output}.');
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -204,8 +204,7 @@ class BuildIOSFrameworkCommand extends BuildSubCommand {
}
globals.printStatus('Frameworks written to ${outputDirectory.path}.');
return null;
return FlutterCommandResult.success();
}
/// Create podspec that will download and unzip remote engine assets so host apps can leverage CocoaPods
......
......@@ -47,6 +47,6 @@ class BuildLinuxCommand extends BuildSubCommand {
throwToolExit('"build linux" only supported on Linux hosts.');
}
await buildLinux(flutterProject.linux, buildInfo, target: targetFile);
return null;
return FlutterCommandResult.success();
}
}
......@@ -51,6 +51,6 @@ class BuildMacosCommand extends BuildSubCommand {
buildInfo: buildInfo,
targetOverride: targetFile,
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -67,6 +67,6 @@ class BuildWebCommand extends BuildSubCommand {
dartDefines,
boolArg('csp')
);
return null;
return FlutterCommandResult.success();
}
}
......@@ -47,6 +47,6 @@ class BuildWindowsCommand extends BuildSubCommand {
throwToolExit('"build windows" only supported on Windows hosts.');
}
await buildWindows(flutterProject.windows, buildInfo, target: targetFile);
return null;
return FlutterCommandResult.success();
}
}
......@@ -42,10 +42,10 @@ class ChannelCommand extends FlutterCommand {
showAll: boolArg('all'),
verbose: globalResults['verbose'] as bool,
);
return null;
return FlutterCommandResult.success();
case 1:
await _switchChannel(argResults.rest[0]);
return null;
return FlutterCommandResult.success();
default:
throw ToolExit('Too many arguments.\n$usage');
}
......
......@@ -101,7 +101,7 @@ class ConfigCommand extends FlutterCommand {
Future<FlutterCommandResult> runCommand() async {
if (boolArg('machine')) {
await handleMachine();
return null;
return FlutterCommandResult.success();
}
if (boolArg('clear-features')) {
......@@ -110,7 +110,7 @@ class ConfigCommand extends FlutterCommand {
globals.config.removeValue(feature.configSetting);
}
}
return null;
return FlutterCommandResult.success();
}
if (argResults.wasParsed('analytics')) {
......@@ -157,7 +157,7 @@ class ConfigCommand extends FlutterCommand {
globals.printStatus('\nYou may need to restart any open editors for them to read new settings.');
}
return null;
return FlutterCommandResult.success();
}
Future<void> handleMachine() async {
......
......@@ -297,7 +297,7 @@ class CreateCommand extends FlutterCommand {
Cache.releaseLockEarly();
await _writeSamplesJson(stringArg('list-samples'));
return null;
return FlutterCommandResult.success();
}
if (argResults.rest.isEmpty) {
......@@ -482,8 +482,7 @@ To edit platform code in an IDE see https://flutter.dev/developing-packages/#edi
globals.printStatus('Your $application code is in $relativeAppMain');
}
}
return null;
return FlutterCommandResult.success();
}
Future<int> _generateModule(Directory directory, Map<String, dynamic> templateContext, { bool overwrite = false }) async {
......
......@@ -74,7 +74,7 @@ class DaemonCommand extends FlutterCommand {
Logger: () => notifyingLogger,
},
);
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -47,6 +47,6 @@ class DevicesCommand extends FlutterCommand {
await Device.printDevices(devices);
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -185,7 +185,7 @@ class DriveCommand extends RunCommandBase {
}
}
return null;
return FlutterCommandResult.success();
}
String _getTestFile() {
......
......@@ -54,7 +54,7 @@ class EmulatorsCommand extends FlutterCommand {
await _listEmulators(searchText);
}
return null;
return FlutterCommandResult.success();
}
Future<void> _launchEmulator(String id) async {
......
......@@ -76,6 +76,6 @@ class FormatCommand extends FlutterCommand {
throwToolExit('Formatting failed: $result', exitCode: result);
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -31,7 +31,7 @@ class GenerateCommand extends FlutterCommand {
globals.printError('Code generation failed.');
break;
}
if (codegenStatus ==CodegenStatus.Succeeded) {
if (codegenStatus == CodegenStatus.Succeeded) {
break;
}
}
......@@ -41,7 +41,7 @@ class GenerateCommand extends FlutterCommand {
return dir.childDirectory('error_cache').existsSync();
}, orElse: () => null);
if (errorCacheParent == null) {
return null;
return FlutterCommandResult.success();
}
final Directory errorCache = errorCacheParent.childDirectory('error_cache');
for (final File errorFile in errorCache.listSync(recursive: true).whereType<File>()) {
......@@ -56,6 +56,6 @@ class GenerateCommand extends FlutterCommand {
globals.printError('Error reading error in ${errorFile.path}');
}
}
return const FlutterCommandResult(ExitStatus.fail);
return FlutterCommandResult.fail();
}
}
......@@ -219,7 +219,7 @@ class IdeConfigCommand extends FlutterCommand {
if (boolArg('update-templates')) {
_handleTemplateUpdate();
return null;
return FlutterCommandResult.success();
}
final String flutterRoot = globals.fs.path.absolute(Cache.flutterRoot);
......@@ -243,7 +243,7 @@ class IdeConfigCommand extends FlutterCommand {
globals.printStatus('Your IntelliJ configuration is now up to date. It is prudent to '
'restart IntelliJ, if running.');
return null;
return FlutterCommandResult.success();
}
int _renderTemplate(String templateName, String dirPath, Map<String, dynamic> context) {
......
......@@ -38,6 +38,6 @@ class InjectPluginsCommand extends FlutterCommand {
globals.printStatus('This project does not use plugins, no GeneratedPluginRegistrants have been created.');
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -45,7 +45,7 @@ class InstallCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts
throwToolExit('Install failed');
}
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -83,6 +83,6 @@ class LogsCommand extends FlutterCommand {
throwToolExit('Error listening to $logReader logs.');
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -60,6 +60,6 @@ class MakeHostAppEditableCommand extends FlutterCommand {
await _project.ios.makeHostAppEditable();
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -131,7 +131,7 @@ class PackagesGetCommand extends FlutterCommand {
await exampleProject.ensureReadyForPlatformSpecificTooling(checkProjects: true);
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -162,7 +162,7 @@ class PackagesTestCommand extends FlutterCommand {
Future<FlutterCommandResult> runCommand() async {
Cache.releaseLockEarly();
await pub.batch(<String>['run', 'test', ...argResults.rest], context: PubContext.runTest, retry: false);
return null;
return FlutterCommandResult.success();
}
}
......@@ -203,7 +203,7 @@ class PackagesPublishCommand extends FlutterCommand {
];
Cache.releaseLockEarly();
await pub.interactively(<String>['publish', ...args]);
return null;
return FlutterCommandResult.success();
}
}
......@@ -234,7 +234,7 @@ class PackagesForwardCommand extends FlutterCommand {
Future<FlutterCommandResult> runCommand() async {
Cache.releaseLockEarly();
await pub.interactively(<String>[_commandName, ...argResults.rest]);
return null;
return FlutterCommandResult.success();
}
}
......@@ -262,6 +262,6 @@ class PackagesPassthroughCommand extends FlutterCommand {
Future<FlutterCommandResult> runCommand() async {
Cache.releaseLockEarly();
await pub.interactively(argResults.rest);
return null;
return FlutterCommandResult.success();
}
}
......@@ -88,6 +88,6 @@ class PrecacheCommand extends FlutterCommand {
} else {
globals.printStatus('Already up-to-date.');
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -97,16 +97,16 @@ class ScreenshotCommand extends FlutterCommand {
switch (stringArg(_kType)) {
case _kDeviceType:
await runScreenshot(outputFile);
return null;
return FlutterCommandResult.success();
case _kSkiaType:
await runSkia(outputFile);
return null;
return FlutterCommandResult.success();
case _kRasterizerType:
await runRasterizer(outputFile);
return null;
return FlutterCommandResult.success();
}
return null;
return FlutterCommandResult.success();
}
Future<void> runScreenshot(File outputFile) async {
......
......@@ -50,7 +50,7 @@ class ShellCompletionCommand extends FlutterCommand {
if (argResults.rest.isEmpty || argResults.rest.first == '-') {
stdout.write(generateCompletionScript(<String>['flutter']));
return null;
return FlutterCommandResult.warning();
}
final File outputFile = globals.fs.file(argResults.rest.first);
......@@ -67,6 +67,6 @@ class ShellCompletionCommand extends FlutterCommand {
throwToolExit('Unable to write shell completion setup script.\n$error', exitCode: 1);
}
return null;
return FlutterCommandResult.success();
}
}
......@@ -270,7 +270,7 @@ class TestCommand extends FastFlutterCommand {
if (result != 0) {
throwToolExit(null);
}
return const FlutterCommandResult(ExitStatus.success);
return FlutterCommandResult.success();
}
Future<void> _buildTestAsset() async {
......
......@@ -22,6 +22,6 @@ class TrainingCommand extends FlutterCommand {
@override
Future<FlutterCommandResult> runCommand() async {
// This command does not do anything yet :).
return null;
return FlutterCommandResult.success();
}
}
......@@ -93,7 +93,7 @@ class UnpackCommand extends FlutterCommand {
if (!success) {
throwToolExit('Failed to unpack desktop artifacts.');
}
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -180,7 +180,7 @@ class UpdatePackagesCommand extends FlutterCommand {
);
}
globals.printStatus('All pubspecs were up to date.');
return null;
return FlutterCommandResult.success();
}
if (upgrade || isPrintPaths || isPrintTransitiveClosure) {
......@@ -289,12 +289,12 @@ class UpdatePackagesCommand extends FlutterCommand {
tree._dependencyTree.forEach((String from, Set<String> to) {
globals.printStatus('$from -> $to');
});
return null;
return FlutterCommandResult.success();
}
if (isPrintPaths) {
showDependencyPaths(from: stringArg('from'), to: stringArg('to'), tree: tree);
return null;
return FlutterCommandResult.success();
}
// Now that we have collected all the data, we can apply our dependency
......@@ -328,7 +328,7 @@ class UpdatePackagesCommand extends FlutterCommand {
final double seconds = timer.elapsedMilliseconds / 1000.0;
globals.printStatus('\nRan \'pub\' $count time${count == 1 ? "" : "s"} and fetched coverage data in ${seconds.toStringAsFixed(1)}s.');
return null;
return FlutterCommandResult.success();
}
void showDependencyPaths({
......
......@@ -50,14 +50,13 @@ class UpgradeCommand extends FlutterCommand {
bool get shouldUpdateCache => false;
@override
Future<FlutterCommandResult> runCommand() async {
await _commandRunner.runCommand(
Future<FlutterCommandResult> runCommand() {
return _commandRunner.runCommand(
boolArg('force'),
boolArg('continue'),
GitTagVersion.determine(),
FlutterVersion.instance,
);
return null;
}
}
......@@ -74,7 +73,7 @@ class UpgradeCommandRunner {
} else {
await runCommandSecondHalf(flutterVersion);
}
return null;
return FlutterCommandResult.success();
}
Future<void> runCommandFirstHalf(
......
......@@ -143,6 +143,6 @@ class VersionCommand extends FlutterCommand {
throwToolExit(null, exitCode: code);
}
return const FlutterCommandResult(ExitStatus.success);
return FlutterCommandResult.success();
}
}
......@@ -171,7 +171,9 @@ class BuildEvent extends UsageEvent {
/// An event that reports the result of a top-level command.
class CommandResultEvent extends UsageEvent {
CommandResultEvent(String commandPath, FlutterCommandResult result)
: super(commandPath, result?.toString() ?? 'unspecified');
: assert(commandPath != null),
assert(result != null),
super(commandPath, result.toString());
@override
void send() {
......
......@@ -50,6 +50,21 @@ class FlutterCommandResult {
this.endTimeOverride,
});
/// A command that succeeded. It is used to log the result of a command invocation.
factory FlutterCommandResult.success() {
return const FlutterCommandResult(ExitStatus.success);
}
/// A command that exited with a warning. It is used to log the result of a command invocation.
factory FlutterCommandResult.warning() {
return const FlutterCommandResult(ExitStatus.warning);
}
/// A command that failed. It is used to log the result of a command invocation.
factory FlutterCommandResult.fail() {
return const FlutterCommandResult(ExitStatus.fail);
}
final ExitStatus exitStatus;
/// Optional data that can be appended to the timing event.
......@@ -516,12 +531,9 @@ abstract class FlutterCommand extends Command<void> {
flutterUsage.printWelcome();
final String commandPath = await usagePath;
_registerSignalHandlers(commandPath, startTime);
FlutterCommandResult commandResult;
FlutterCommandResult commandResult = FlutterCommandResult.fail();
try {
commandResult = await verifyThenRunCommand(commandPath);
} on ToolExit {
commandResult = const FlutterCommandResult(ExitStatus.fail);
rethrow;
} finally {
final DateTime endTime = systemClock.now();
globals.printTrace(userMessages.flutterElapsedTime(name, getElapsedAsMilliseconds(endTime.difference(startTime))));
......@@ -557,15 +569,15 @@ abstract class FlutterCommand extends Command<void> {
if (commandPath == null) {
return;
}
assert(commandResult != null);
// Send command result.
CommandResultEvent(commandPath, commandResult).send();
// Send timing.
final List<String> labels = <String>[
if (commandResult?.exitStatus != null)
if (commandResult.exitStatus != null)
getEnumName(commandResult.exitStatus),
if (commandResult?.timingLabelParts?.isNotEmpty ?? false)
if (commandResult.timingLabelParts?.isNotEmpty ?? false)
...commandResult.timingLabelParts,
];
......@@ -577,7 +589,7 @@ abstract class FlutterCommand extends Command<void> {
name,
// If the command provides its own end time, use it. Otherwise report
// the duration of the entire execution.
(commandResult?.endTimeOverride ?? endTime).difference(startTime),
(commandResult.endTimeOverride ?? endTime).difference(startTime),
// Report in the form of `success-[parameter1-parameter2]`, all of which
// can be null if the command doesn't provide a FlutterCommandResult.
label: label == '' ? null : label,
......
......@@ -75,7 +75,7 @@ void main() {
const GitTagVersion.unknown(),
flutterVersion,
);
expect(await result, null);
expect(await result, FlutterCommandResult.success());
}, overrides: <Type, Generator>{
ProcessManager: () => processManager,
Platform: () => fakePlatform,
......@@ -103,7 +103,7 @@ void main() {
gitTagVersion,
flutterVersion,
);
expect(await result, null);
expect(await result, FlutterCommandResult.success());
}, overrides: <Type, Generator>{
ProcessManager: () => processManager,
Platform: () => fakePlatform,
......@@ -116,7 +116,7 @@ void main() {
gitTagVersion,
flutterVersion,
);
expect(await result, null);
expect(await result, FlutterCommandResult.success());
}, overrides: <Type, Generator>{
ProcessManager: () => processManager,
Platform: () => fakePlatform,
......@@ -130,7 +130,7 @@ void main() {
gitTagVersion,
flutterVersion,
);
expect(await result, null);
expect(await result, FlutterCommandResult.success());
verifyNever(globals.processManager.start(
<String>[
globals.fs.path.join('bin', 'flutter'),
......
......@@ -24,10 +24,12 @@ import '../../src/mocks.dart';
void main() {
Cache.disableLocking();
group('getUsage', () {
group('Usage', () {
Directory tempDir;
Usage mockUsage;
setUp(() {
mockUsage = MockUsage();
tempDir = globals.fs.systemTempDirectory.createTempSync('flutter_tools_packages_test.');
});
......@@ -83,6 +85,26 @@ void main() {
}, overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
});
testUsingContext('logs success', () async {
final String projectPath = await createProject(tempDir,
arguments: <String>['--no-pub', '--template=module']);
await runCommandIn(projectPath,
arguments: <String>['--target-platform=android-arm']);
verify(mockUsage.sendEvent(
'tool-command-result',
'aar',
label: 'success',
value: anyNamed('value'),
parameters: anyNamed('parameters'),
)).called(1);
},
overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
Usage: () => mockUsage,
});
});
group('Gradle', () {
......
......@@ -26,10 +26,12 @@ import '../../src/mocks.dart';
void main() {
Cache.disableLocking();
group('getUsage', () {
group('Usage', () {
Directory tempDir;
Usage mockUsage;
setUp(() {
mockUsage = MockUsage();
tempDir = globals.fs.systemTempDirectory.createTempSync('flutter_tools_packages_test.');
});
......@@ -92,6 +94,25 @@ void main() {
}, overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
});
testUsingContext('logs success', () async {
final String projectPath = await createProject(tempDir,
arguments: <String>['--no-pub', '--template=app']);
await runBuildApkCommand(projectPath);
verify(mockUsage.sendEvent(
'tool-command-result',
'apk',
label: 'success',
value: anyNamed('value'),
parameters: anyNamed('parameters'),
)).called(1);
},
overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
Usage: () => mockUsage,
});
});
group('Gradle', () {
......
......@@ -26,11 +26,13 @@ import '../../src/mocks.dart';
void main() {
Cache.disableLocking();
group('getUsage', () {
group('Usage', () {
Directory tempDir;
Usage mockUsage;
setUp(() {
tempDir = globals.fs.systemTempDirectory.createTempSync('flutter_tools_packages_test.');
mockUsage = MockUsage();
});
tearDown(() {
......@@ -75,6 +77,25 @@ void main() {
}, overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
});
testUsingContext('logs success', () async {
final String projectPath = await createProject(tempDir,
arguments: <String>['--no-pub', '--template=app']);
await runBuildAppBundleCommand(projectPath);
verify(mockUsage.sendEvent(
'tool-command-result',
'appbundle',
label: 'success',
value: anyNamed('value'),
parameters: anyNamed('parameters'),
)).called(1);
},
overrides: <Type, Generator>{
AndroidBuilder: () => FakeAndroidBuilder(),
Usage: () => mockUsage,
});
});
group('Gradle', () {
......
......@@ -342,7 +342,7 @@ class _CrashCommand extends FlutterCommand {
fn3();
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -217,6 +217,14 @@ void main() {
}
});
test('FlutterCommandResult.success()', () async {
expect(FlutterCommandResult.success().exitStatus, ExitStatus.success);
});
test('FlutterCommandResult.warning()', () async {
expect(FlutterCommandResult.warning().exitStatus, ExitStatus.warning);
});
group('signals tests', () {
MockIoProcessSignal mockSignal;
ProcessSignal signalUnderTest;
......@@ -297,7 +305,7 @@ void main() {
'flutter',
'dummy',
const Duration(milliseconds: 1000),
null,
'fail',
],
);
});
......@@ -388,7 +396,7 @@ class FakeCommand extends FlutterCommand {
@override
Future<FlutterCommandResult> runCommand() async {
return null;
return FlutterCommandResult.success();
}
}
......
......@@ -36,7 +36,7 @@ class DummyFlutterCommand extends FlutterCommand {
@override
Future<FlutterCommandResult> runCommand() async {
return commandFunction == null ? null : await commandFunction();
return commandFunction == null ? FlutterCommandResult.fail() : await commandFunction();
}
}
......
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