[flutter_tools] teach flutter drive to uninstall if install fails (#67936)

Work towards #39925

Currently flutter run will uninstall and reinstall if the initial install fails and the APK was previously installed. Allow drive to share this same logic by moving it into installApp and out of startApp.

This should reduce the occurrence of the error in the devicelab.

packages/flutter_tools/lib/src/android/android_device.dart

@@ -404,13 +404,40 @@ class AndroidDevice extends Device {
     AndroidApk app, {
     String userIdentifier,
   }) async {
-    if (!app.file.existsSync()) {
-      _logger.printError('"${_fileSystem.path.relative(app.file.path)}" does not exist.');
+    if (!await _isAdbValid()) {
+      return false;
+    }
+    final bool wasInstalled = await isAppInstalled(app, userIdentifier: userIdentifier);
+    if (wasInstalled && await isLatestBuildInstalled(app)) {
+      _logger.printTrace('Latest build already installed.');
+      return true;
+    }
+    _logger.printTrace('Installing APK.');
+    if (await _installApp(app, userIdentifier: userIdentifier)) {
+      return true;
+    }
+    _logger.printTrace('Warning: Failed to install APK.');
+    if (!wasInstalled) {
       return false;
     }
+    _logger.printStatus('Uninstalling old version...');
+    if (!await uninstallApp(app, userIdentifier: userIdentifier)) {
+      _logger.printError('Error: Uninstalling old version failed.');
+      return false;
+    }
+    if (!await _installApp(app, userIdentifier: userIdentifier)) {
+      _logger.printError('Error: Failed to install APK again.');
+      return false;
+    }
+    return true;
+  }
 
-    if (!await _checkForSupportedAdbVersion() ||
-        !await _checkForSupportedAndroidVersion()) {
+  Future<bool> _installApp(
+    AndroidApk app, {
+    String userIdentifier,
+  }) async {
+    if (!app.file.existsSync()) {
+      _logger.printError('"${_fileSystem.path.relative(app.file.path)}" does not exist.');
       return false;
     }
 
@@ -461,8 +488,7 @@ class AndroidDevice extends Device {
     AndroidApk app, {
     String userIdentifier,
   }) async {
-    if (!await _checkForSupportedAdbVersion() ||
-        !await _checkForSupportedAndroidVersion()) {
+    if (!await _isAdbValid()) {
       return false;
     }
 
@@ -487,36 +513,13 @@ class AndroidDevice extends Device {
       _logger.printError('Package uninstall error: $failure');
       return false;
     }
-
     return true;
   }
 
-  Future<bool> _installLatestApp(AndroidApk package, String userIdentifier) async {
-    final bool wasInstalled = await isAppInstalled(package, userIdentifier: userIdentifier);
-    if (wasInstalled) {
-      if (await isLatestBuildInstalled(package)) {
-        _logger.printTrace('Latest build already installed.');
-        return true;
-      }
-    }
-    _logger.printTrace('Installing APK.');
-    if (!await installApp(package, userIdentifier: userIdentifier)) {
-      _logger.printTrace('Warning: Failed to install APK.');
-      if (wasInstalled) {
-        _logger.printStatus('Uninstalling old version...');
-        if (!await uninstallApp(package, userIdentifier: userIdentifier)) {
-          _logger.printError('Error: Uninstalling old version failed.');
-          return false;
-        }
-        if (!await installApp(package, userIdentifier: userIdentifier)) {
-          _logger.printError('Error: Failed to install APK again.');
-          return false;
-        }
-        return true;
-      }
-      return false;
-    }
-    return true;
+  // Whether the adb and Android versions are aligned.
+  bool _adbIsValid;
+  Future<bool> _isAdbValid() async {
+    return _adbIsValid ??= await _checkForSupportedAdbVersion() && await _checkForSupportedAndroidVersion();
   }
 
   AndroidApk _package;
@@ -532,8 +535,7 @@ class AndroidDevice extends Device {
     bool ipv6 = false,
     String userIdentifier,
   }) async {
-    if (!await _checkForSupportedAdbVersion() ||
-        !await _checkForSupportedAndroidVersion()) {
+    if (!await _isAdbValid()) {
       return LaunchResult.failed();
     }
 
@@ -587,7 +589,7 @@ class AndroidDevice extends Device {
     _logger.printTrace("Stopping app '${package.name}' on $name.");
     await stopApp(package, userIdentifier: userIdentifier);
 
-    if (!await _installLatestApp(package, userIdentifier)) {
+    if (!await installApp(package, userIdentifier: userIdentifier)) {
       return LaunchResult.failed();
     }
 

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

@@ -86,7 +86,8 @@ class DriveCommand extends RunCommandBase {
       )
       ..addFlag('build',
         defaultsTo: true,
-        help: 'Build the app before running.',
+        help: '(Deprecated) Build the app before running. To use an existing app, pass the --use-application-binary '
+          'flag with an existing APK',
       )
       ..addOption('driver-port',
         defaultsTo: '4444',
@@ -142,7 +143,6 @@ class DriveCommand extends RunCommandBase {
 
   Device _device;
   Device get device => _device;
-  bool get shouldBuild => boolArg('build');
 
   bool get verboseSystemLogs => boolArg('verbose-system-logs');
   String get userIdentifier => stringArg(FlutterOptions.kDeviceUser);
@@ -469,14 +469,6 @@ Future<LaunchResult> _startApp(
   final ApplicationPackage package = await command.applicationPackages
       .getPackageForPlatform(await command.device.targetPlatform, command.getBuildInfo());
 
-  if (command.shouldBuild) {
-    globals.printTrace('Installing application package.');
-    if (await command.device.isAppInstalled(package, userIdentifier: userIdentifier)) {
-      await command.device.uninstallApp(package, userIdentifier: userIdentifier);
-    }
-    await command.device.installApp(package, userIdentifier: userIdentifier);
-  }
-
   final Map<String, dynamic> platformArgs = <String, dynamic>{};
   if (command.traceStartup) {
     platformArgs['trace-startup'] = command.traceStartup;
@@ -515,7 +507,6 @@ Future<LaunchResult> _startApp(
       purgePersistentCache: command.purgePersistentCache,
     ),
     platformArgs: platformArgs,
-    prebuiltApplication: !command.shouldBuild,
     userIdentifier: userIdentifier,
   );
 

packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart

@@ -562,35 +562,6 @@ void main() {
         FileSystem: () => fs,
         ProcessManager: () => FakeProcessManager.any(),
       });
-
-      testUsingContext('uses prebuilt app if --no-build arg provided', () async {
-        final Device mockDevice = await appStarterSetup();
-
-        final List<String> args = <String>[
-          'drive',
-          '--no-build',
-          '--target=$testApp',
-          '--no-pub',
-        ];
-        try {
-          await createTestCommandRunner(command).run(args);
-        } on ToolExit catch (e) {
-          expect(e.exitCode, 123);
-          expect(e.message, null);
-        }
-        verify(mockDevice.startApp(
-                null,
-                mainPath: anyNamed('mainPath'),
-                route: anyNamed('route'),
-                debuggingOptions: anyNamed('debuggingOptions'),
-                platformArgs: anyNamed('platformArgs'),
-                prebuiltApplication: true,
-                userIdentifier: anyNamed('userIdentifier'),
-        ));
-      }, overrides: <Type, Generator>{
-        FileSystem: () => fs,
-        ProcessManager: () => FakeProcessManager.any(),
-      });
     });
 
     group('debugging options', () {

packages/flutter_tools/test/general.shard/android/android_device_start_test.dart

@@ -89,8 +89,6 @@ void main() {
       processManager.addCommand(const FakeCommand(
         command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', 'FlutterApp'],
       ));
-      processManager.addCommand(kAdbVersionCommand);
-      processManager.addCommand(kStartServer);
       processManager.addCommand(const FakeCommand(
         command: <String>['adb', '-s', '1234', 'install', '-t', '-r', 'app.apk'],
       ));
@@ -193,8 +191,6 @@ void main() {
     processManager.addCommand(const FakeCommand(
       command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'FlutterApp'],
     ));
-    processManager.addCommand(kAdbVersionCommand);
-    processManager.addCommand(kStartServer);
     // TODO(jonahwilliams): investigate why this doesn't work.
     // This doesn't work with the current Android log reader implementation.
     processManager.addCommand(const FakeCommand(

packages/flutter_tools/test/general.shard/android/android_install_test.dart

@@ -51,8 +51,7 @@ void main() {
     AndroidSdk androidSdk,
     ProcessManager processManager,
   }) {
-    androidSdk ??= MockAndroidSdk();
-    when(androidSdk.adbPath).thenReturn('adb');
+    androidSdk ??= FakeAndroidSdk();
     return AndroidDevice('1234',
       logger: logger,
       platform: FakePlatform(operatingSystem: 'linux'),
@@ -108,6 +107,10 @@ void main() {
         command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
         stdout: '[ro.build.version.sdk]: [16]',
       ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'app'],
+        stdout: '\n'
+      ),
       kInstallCommand,
       kStoreShaCommand,
     ]);
@@ -133,6 +136,10 @@ void main() {
       const FakeCommand(
         command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
       ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'app'],
+        stdout: '\n'
+      ),
       kInstallCommand,
       kStoreShaCommand,
     ]);
@@ -158,6 +165,12 @@ void main() {
       const FakeCommand(
         command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
       ),
+      // This command is run before the user is checked and is allowed to fail.
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', 'jane', 'app'],
+        stderr: 'Blah blah',
+        exitCode: 1,
+      ),
       const FakeCommand(
         command: <String>[
           'adb',
@@ -189,6 +202,116 @@ void main() {
     expect(logger.errorText, contains('Error: User "jane" not found. Run "adb shell pm list users" to see list of available identifiers.'));
     expect(processManager.hasRemainingExpectations, false);
   });
+
+  testWithoutContext('Will skip install if the correct version is up to date', () async {
+    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      kAdbVersionCommand,
+      kAdbStartServerCommand,
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
+        stdout: '[ro.build.version.sdk]: [16]',
+      ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'app'],
+        stdout: 'package:app\n'
+      ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'cat', '/data/local/tmp/sky.app.sha1'],
+        stdout: 'example_sha',
+      ),
+    ]);
+    final File apk = fileSystem.file('app.apk')..createSync();
+    fileSystem.file('app.apk.sha1').writeAsStringSync('example_sha');
+    final AndroidApk androidApk = AndroidApk(
+      file: apk,
+      id: 'app',
+      versionCode: 22,
+      launchActivity: 'Main',
+    );
+    final AndroidDevice androidDevice = setUpAndroidDevice(
+      processManager: processManager,
+    );
+
+    expect(await androidDevice.installApp(androidApk, userIdentifier: '10'), true);
+    expect(processManager.hasRemainingExpectations, false);
+  });
+
+  testWithoutContext('Will uninstall if the correct version is not up to date and install fails', () async {
+    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      kAdbVersionCommand,
+      kAdbStartServerCommand,
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
+        stdout: '[ro.build.version.sdk]: [16]',
+      ),
+      const FakeCommand(
+          command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'app'],
+          stdout: 'package:app\n'
+      ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'cat', '/data/local/tmp/sky.app.sha1'],
+        stdout: 'different_example_sha',
+      ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'install', '-t', '-r', '--user', '10', 'app.apk'],
+        exitCode: 1,
+        stderr: '[INSTALL_FAILED_INSUFFICIENT_STORAGE]',
+      ),
+      const FakeCommand(command: <String>['adb', '-s', '1234', 'uninstall', '--user', '10', 'app']),
+      kInstallCommand,
+      const FakeCommand(command: <String>['adb', '-s', '1234', 'shell', 'echo', '-n', 'example_sha', '>', '/data/local/tmp/sky.app.sha1']),
+    ]);
+    final File apk = fileSystem.file('app.apk')..createSync();
+    fileSystem.file('app.apk.sha1').writeAsStringSync('example_sha');
+    final AndroidApk androidApk = AndroidApk(
+      file: apk,
+      id: 'app',
+      versionCode: 22,
+      launchActivity: 'Main',
+    );
+    final AndroidDevice androidDevice = setUpAndroidDevice(
+      processManager: processManager,
+    );
+
+    expect(await androidDevice.installApp(androidApk, userIdentifier: '10'), true);
+    expect(processManager.hasRemainingExpectations, false);
+  });
+
+  testWithoutContext('Will fail to install if the apk was never installed and it fails the first time', () async {
+    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      kAdbVersionCommand,
+      kAdbStartServerCommand,
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'shell', 'getprop'],
+        stdout: '[ro.build.version.sdk]: [16]',
+      ),
+      const FakeCommand(
+          command: <String>['adb', '-s', '1234', 'shell', 'pm', 'list', 'packages', '--user', '10', 'app'],
+          stdout: '\n'
+      ),
+      const FakeCommand(
+        command: <String>['adb', '-s', '1234', 'install', '-t', '-r', '--user', '10', 'app.apk'],
+        exitCode: 1,
+        stderr: '[INSTALL_FAILED_INSUFFICIENT_STORAGE]',
+      ),
+    ]);
+    final File apk = fileSystem.file('app.apk')..createSync();
+    final AndroidApk androidApk = AndroidApk(
+      file: apk,
+      id: 'app',
+      versionCode: 22,
+      launchActivity: 'Main',
+    );
+    final AndroidDevice androidDevice = setUpAndroidDevice(
+      processManager: processManager,
+    );
+
+    expect(await androidDevice.installApp(androidApk, userIdentifier: '10'), false);
+    expect(processManager.hasRemainingExpectations, false);
+  });
 }
 
-class MockAndroidSdk extends Mock implements AndroidSdk {}
+class FakeAndroidSdk extends Fake implements AndroidSdk {
+  @override
+  String get adbPath => 'adb';
+}