Revert "Reland [devicelab] Migrate Gallery to BuildTestTask (#78361)" (#79903)

This reverts commit 0c9781e3.

dev/bots/test.dart

@@ -323,18 +323,11 @@ Future<void> _runBuildTests() async {
     ..add(Directory(path.join(flutterRoot, 'dev', 'integration_tests', 'non_nullable')))
     ..add(Directory(path.join(flutterRoot, 'dev', 'integration_tests', 'ui')));
 
-  final List<String> devicelabBuildTasks = <String>[
-    'flutter_gallery__transition_perf',
-    'flutter_gallery_ios__transition_perf',
-  ];
-
   // The tests are randomly distributed into subshards so as to get a uniform
   // distribution of costs, but the seed is fixed so that issues are reproducible.
   final List<ShardRunner> tests = <ShardRunner>[
     for (final FileSystemEntity exampleDirectory in exampleDirectories)
       () => _runExampleProjectBuildTests(exampleDirectory),
-    for (String devicelabBuildTask in devicelabBuildTasks)
-      () => _runDeviceLabBuildTask(devicelabBuildTask),
     ...<ShardRunner>[
       // Web compilation tests.
       () => _flutterBuildDart2js(
@@ -352,26 +345,6 @@ Future<void> _runBuildTests() async {
   await _runShardRunnerIndexOfTotalSubshard(tests);
 }
 
-
-Future<void> _runDeviceLabBuildTask(String task) async {
-  // Run the ios tasks
-  if (!Platform.isMacOS && task.contains('_ios_')) {
-    return;
-  }
-
-  final String targetPlatform = (task.contains('_ios_')) ? 'ios' : 'android';
-
-  await runCommand(dart, <String>[
-    'run', path.join('bin', 'test_runner.dart'),
-    'test',
-    '--task', task,
-    '--task-args', 'build',
-    '--task-args', 'target-platform=$targetPlatform',
-  ],
-  workingDirectory: path.join('dev', 'devicelab'));
-}
-
-
 Future<void> _runExampleProjectBuildTests(FileSystemEntity exampleDirectory) async {
   // Only verify caching with flutter gallery.
   final bool verifyCaching = exampleDirectory.path.contains('flutter_gallery');
@@ -667,7 +640,7 @@ Future<void> _runFrameworkTests() async {
   Future<void> runMisc() async {
     print('${green}Running package tests$reset for directories other than packages/flutter');
     await _pubRunTest(path.join(flutterRoot, 'dev', 'bots'));
-    await _pubRunTest(path.join(flutterRoot, 'dev', 'devicelab'), forceSingleCore: true);
+    await _pubRunTest(path.join(flutterRoot, 'dev', 'devicelab'));
     await _pubRunTest(path.join(flutterRoot, 'dev', 'snippets'));
     await _pubRunTest(path.join(flutterRoot, 'dev', 'tools'), forceSingleCore: true);
     await _runFlutterTest(path.join(flutterRoot, 'dev', 'integration_tests', 'android_semantics_testing'));

dev/devicelab/bin/tasks/flutter_gallery__transition_perf.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.android;
-  await task(createGalleryTransitionTest(args));
+  await task(createGalleryTransitionTest());
 }

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.android;
-  await task(createGalleryTransitionE2ETest(args));
+  await task(createGalleryTransitionE2ETest());
 }

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e_ios.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.ios;
-  await task(createGalleryTransitionE2ETest(args));
+  await task(createGalleryTransitionE2ETest());
 }

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e_ios32.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.ios;
-  await task(createGalleryTransitionE2ETest(args));
+  await task(createGalleryTransitionE2ETest());
 }

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_hybrid.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.android;
-  await task(createGalleryTransitionHybridTest(args));
+  await task(createGalleryTransitionHybridTest());
 }

dev/devicelab/bin/tasks/flutter_gallery__transition_perf_with_semantics.dart

@@ -7,11 +7,11 @@ import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 import 'package:flutter_devicelab/framework/task_result.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.android;
   await task(() async {
-    final TaskResult withoutSemantics = await createGalleryTransitionTest(args)();
-    final TaskResult withSemantics = await createGalleryTransitionTest(args, semanticsEnabled: true)();
+    final TaskResult withoutSemantics = await createGalleryTransitionTest()();
+    final TaskResult withSemantics = await createGalleryTransitionTest(semanticsEnabled: true)();
     if (withSemantics.benchmarkScoreKeys.isEmpty || withoutSemantics.benchmarkScoreKeys.isEmpty) {
       String message = 'Lack of data';
       if (withSemantics.benchmarkScoreKeys.isEmpty) {

dev/devicelab/bin/tasks/flutter_gallery_ios__transition_perf.dart

@@ -6,7 +6,7 @@ import 'package:flutter_devicelab/tasks/gallery.dart';
 import 'package:flutter_devicelab/framework/adb.dart';
 import 'package:flutter_devicelab/framework/framework.dart';
 
-Future<void> main(List<String> args) async {
+Future<void> main() async {
   deviceOperatingSystem = DeviceOperatingSystem.ios;
-  await task(createGalleryTransitionTest(args));
+  await task(createGalleryTransitionTest());
 }

dev/devicelab/bin/tasks/smoke_test_build_test.dart

@@ -14,23 +14,18 @@ Future<void> main(List<String> args) async {
 }
 
 class FakeBuildTestTask extends BuildTestTask {
-  FakeBuildTestTask(List<String> args) : super(args, runFlutterClean: false);
+  FakeBuildTestTask(List<String> args) : super(args, runFlutterClean: false) {
+    deviceOperatingSystem = DeviceOperatingSystem.fake;
+  }
 
   @override
   // In prod, tasks always run some unit of work and the test framework assumes
   // there will be some work done when managing the isolate. To fake this, add a delay.
-  Future<void> build() async {
-    if (targetPlatform != DeviceOperatingSystem.fake) {
-      throw Exception('Only DeviceOperatingSystem.fake is supported');
-    }
-  }
+  Future<void> build() => Future<void>.delayed(const Duration(milliseconds: 500));
 
   @override
   Future<TaskResult> test() async {
-    if (targetPlatform != DeviceOperatingSystem.fake) {
-      throw Exception('Only DeviceOperatingSystem.fake is supported');
-    }
-
+    await Future<void>.delayed(const Duration(milliseconds: 500));
     return TaskResult.success(<String, String>{'benchmark': 'data'});
   }
 }

dev/devicelab/bin/tasks/smoke_test_build_test_target_platform.dart

-// Copyright 2014 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'package:flutter_devicelab/framework/framework.dart';
-import 'package:flutter_devicelab/framework/adb.dart';
-
-import 'smoke_test_build_test.dart';
-
-/// Smoke test of a build test task with [deviceOperatingSystem] set. This should
-/// only pass in tests run with target platform set to fake.
-Future<void> main(List<String> args) async {
-  deviceOperatingSystem = DeviceOperatingSystem.ios;
-  await task(FakeBuildTestTask(args));
-}

dev/devicelab/lib/command/test.dart

@@ -65,6 +65,7 @@ class TestCommand extends Command<void> {
     final List<String> taskArgsRaw = argResults['task-args'] as List<String>;
     // Prepend '--' to convert args to options when passed to task
     final List<String> taskArgs = taskArgsRaw.map((String taskArg) => '--$taskArg').toList();
+    print(taskArgs);
     await runTasks(
       <String>[argResults['task'] as String],
       deviceId: argResults['device-id'] as String,

dev/devicelab/lib/framework/adb.dart

@@ -54,26 +54,6 @@ DeviceDiscovery get devices => DeviceDiscovery();
 /// Device operating system the test is configured to test.
 enum DeviceOperatingSystem { android, androidArm, androidArm64 ,ios, fuchsia, fake }
 
-/// Helper function to allow passing the target platform as a task arg instead
-/// of hardcoding it in the task.
-DeviceOperatingSystem deviceOperatingSystemFromString(String os) {
-  switch (os) {
-    case 'android':
-      return DeviceOperatingSystem.android;
-    case 'android_arm':
-      return DeviceOperatingSystem.androidArm;
-    case 'android_arm64':
-      return DeviceOperatingSystem.androidArm64;
-    case 'fake':
-      return DeviceOperatingSystem.fake;
-    case 'fuchsia':
-      return DeviceOperatingSystem.fuchsia;
-    case 'ios':
-      return DeviceOperatingSystem.ios;
-  }
-  throw UnimplementedError('$os is not defined in function deviceOperatingSystemFromString');
-}
-
 /// Device OS to test on.
 DeviceOperatingSystem deviceOperatingSystem = DeviceOperatingSystem.android;
 

dev/devicelab/lib/framework/framework.dart

@@ -20,7 +20,6 @@ import 'utils.dart';
 /// Identifiers for devices that should never be rebooted.
 final Set<String> noRebootForbidList = <String>{
   '822ef7958bba573829d85eef4df6cbdd86593730', // 32bit iPhone requires manual intervention on reboot.
-  'FAKE_SUCCESS', // Fake device id used in unit tests
 };
 
 /// The maximum number of test runs before a device must be rebooted.
@@ -62,8 +61,8 @@ class _TaskRunner {
   _TaskRunner(this.task) {
     registerExtension('ext.cocoonRunTask',
         (String method, Map<String, String> parameters) async {
-      final Duration taskTimeout = parameters.containsKey('timeoutInMinutes') || parameters.containsKey('timeoutInSeconds')
-        ? Duration(minutes: int.parse(parameters['timeoutInMinutes'] ?? '0'), seconds: int.parse(parameters['timeoutInSeconds'] ?? '0'))
+      final Duration taskTimeout = parameters.containsKey('timeoutInMinutes')
+        ? Duration(minutes: int.parse(parameters['timeoutInMinutes']))
         : null;
       // This is only expected to be passed in unit test runs so they do not
       // kill the Dart process that is running them and waste time running config.
@@ -171,12 +170,8 @@ class _TaskRunner {
       print(stackTrace);
       return TaskResult.failure('Task timed out after $taskTimeout');
     } finally {
-      if (runProcessCleanup) {
-        await checkForRebootRequired();
-        await forceQuitRunningProcesses();
-      } else {
-        print('Skipped checkForRebootRequired and forceQuitRunningProcesses');
-      }
+      await checkForRebootRequired();
+      await forceQuitRunningProcesses();
       _closeKeepAlivePort();
     }
   }

dev/devicelab/lib/tasks/build_test_task.dart

@@ -19,25 +19,16 @@ abstract class BuildTestTask {
     applicationBinaryPath = argResults[kApplicationBinaryPathOption] as String;
     buildOnly = argResults[kBuildOnlyFlag] as bool;
     testOnly = argResults[kTestOnlyFlag] as bool;
-    if (argResults.wasParsed(kTargetPlatformOption)) {
-      // Override deviceOperatingSystem to prevent extra utilities from being used.
-      targetPlatform = deviceOperatingSystemFromString(argResults[kTargetPlatformOption] as String);
-      _originalDeviceOperatingSystem = deviceOperatingSystem;
-      deviceOperatingSystem = DeviceOperatingSystem.fake;
-    } else {
-      targetPlatform = deviceOperatingSystem;
-    }
+
   }
 
   static const String kApplicationBinaryPathOption = 'application-binary-path';
   static const String kBuildOnlyFlag = 'build';
-  static const String kTargetPlatformOption = 'target-platform';
   static const String kTestOnlyFlag = 'test';
 
   final ArgParser argParser = ArgParser()
     ..addOption(kApplicationBinaryPathOption)
     ..addFlag(kBuildOnlyFlag)
-    ..addOption(kTargetPlatformOption)
     ..addFlag(kTestOnlyFlag);
 
   /// Args passed from the test runner via "--task-arg".
@@ -46,15 +37,6 @@ abstract class BuildTestTask {
   /// If true, skip [test].
   bool buildOnly = false;
 
-  /// The [DeviceOperatingSystem] being targeted for this build.
-  ///
-  /// If passed, no connected device checks are run as the current connected device
-  /// will be set as [DeviceOperatingSystem.fake].
-  DeviceOperatingSystem targetPlatform;
-
-  /// Original [deviceOperatingSystem] if [targetPlatform] is given.
-  DeviceOperatingSystem _originalDeviceOperatingSystem;
-
   /// If true, skip [build].
   bool testOnly = false;
 
@@ -77,7 +59,7 @@ abstract class BuildTestTask {
         await flutter('clean');
       }
       section('BUILDING APPLICATION');
-      await flutter('build', options: getBuildArgs());
+      await flutter('build', options: getBuildArgs(deviceOperatingSystem));
     });
 
   }
@@ -86,25 +68,21 @@ abstract class BuildTestTask {
   ///
   /// This assumes that [applicationBinaryPath] exists.
   Future<TaskResult> test() async {
-    // Ensure deviceOperatingSystem is the one set in bin/task.
-    if (deviceOperatingSystem == DeviceOperatingSystem.fake) {
-      deviceOperatingSystem = _originalDeviceOperatingSystem;
-    }
     final Device device = await devices.workingDevice;
     await device.unlock();
     await inDirectory<void>(workingDirectory, () async {
       section('DRIVE START');
-      await flutter('drive', options: getTestArgs(device.deviceId));
+      await flutter('drive', options: getTestArgs(deviceOperatingSystem, device.deviceId));
     });
 
     return parseTaskResult();
   }
 
   /// Args passed to flutter build to build the application under test.
-  List<String> getBuildArgs() => throw UnimplementedError('getBuildArgs is not implemented');
+  List<String> getBuildArgs(DeviceOperatingSystem deviceOperatingSystem) => throw UnimplementedError('getBuildArgs is not implemented');
 
   /// Args passed to flutter drive to test the built application.
-  List<String> getTestArgs(String deviceId) => throw UnimplementedError('getTestArgs is not implemented');
+  List<String> getTestArgs(DeviceOperatingSystem deviceOperatingSystem, String deviceId) => throw UnimplementedError('getTestArgs is not implemented');
 
   /// Logic to construct [TaskResult] from this test's results.
   Future<TaskResult> parseTaskResult() => throw UnimplementedError('parseTaskResult is not implemented');
@@ -128,7 +106,7 @@ abstract class BuildTestTask {
     }
 
     if (!testOnly) {
-      await build();
+      build();
     }
 
     if (buildOnly) {

dev/devicelab/lib/tasks/gallery.dart

@@ -11,17 +11,13 @@ import '../framework/framework.dart';
 import '../framework/host_agent.dart';
 import '../framework/task_result.dart';
 import '../framework/utils.dart';
-import 'build_test_task.dart';
 
-final Directory galleryDirectory = dir('${flutterDirectory.path}/dev/integration_tests/flutter_gallery');
-
-TaskFunction createGalleryTransitionTest(List<String> args, {bool semanticsEnabled = false}) {
-  return GalleryTransitionTest(args, semanticsEnabled: semanticsEnabled, workingDirectory: galleryDirectory,);
+TaskFunction createGalleryTransitionTest({bool semanticsEnabled = false}) {
+  return GalleryTransitionTest(semanticsEnabled: semanticsEnabled);
 }
 
-TaskFunction createGalleryTransitionE2ETest(List<String> args, {bool semanticsEnabled = false}) {
+TaskFunction createGalleryTransitionE2ETest({bool semanticsEnabled = false}) {
   return GalleryTransitionTest(
-    args,
     testFile: semanticsEnabled
         ? 'transitions_perf_e2e_with_semantics'
         : 'transitions_perf_e2e',
@@ -30,23 +26,21 @@ TaskFunction createGalleryTransitionE2ETest(List<String> args, {bool semanticsEn
     transitionDurationFile: null,
     timelineTraceFile: null,
     driverFile: 'transitions_perf_e2e_test',
-    workingDirectory: galleryDirectory,
   );
 }
 
-TaskFunction createGalleryTransitionHybridTest(List<String> args, {bool semanticsEnabled = false}) {
+TaskFunction createGalleryTransitionHybridTest({bool semanticsEnabled = false}) {
   return GalleryTransitionTest(
-    args,
     semanticsEnabled: semanticsEnabled,
     driverFile: semanticsEnabled
         ? 'transitions_perf_hybrid_with_semantics_test'
         : 'transitions_perf_hybrid_test',
-    workingDirectory: galleryDirectory,
   );
 }
 
-class GalleryTransitionTest extends BuildTestTask {
-  GalleryTransitionTest(List<String> args, {
+class GalleryTransitionTest {
+
+  GalleryTransitionTest({
     this.semanticsEnabled = false,
     this.testFile = 'transitions_perf',
     this.needFullTimeline = true,
@@ -54,8 +48,7 @@ class GalleryTransitionTest extends BuildTestTask {
     this.timelineTraceFile = 'transitions.timeline',
     this.transitionDurationFile = 'transition_durations.timeline',
     this.driverFile,
-    Directory workingDirectory,
-  }) : super(args, workingDirectory: workingDirectory);
+  });
 
   final bool semanticsEnabled;
   final bool needFullTimeline;
@@ -65,55 +58,55 @@ class GalleryTransitionTest extends BuildTestTask {
   final String transitionDurationFile;
   final String driverFile;
 
-  final String testOutputDirectory = Platform.environment['FLUTTER_TEST_OUTPUTS_DIR'] ?? '${galleryDirectory.path}/build';
-
-  @override
-  List<String> getBuildArgs() {
-      switch (targetPlatform) {
-        case DeviceOperatingSystem.android:
-          return <String>[
-              'apk',
-              '--no-android-gradle-daemon',
-              '--profile',
-              '-t',
-              'test_driver/$testFile.dart',
-              '--target-platform',
-              'android-arm,android-arm64',
-            ];
-        case DeviceOperatingSystem.ios:
-          return <String>[
-            'ios',
-            // Skip codesign on presubmit checks
-            if (targetPlatform != null)
-              '--no-codesign',
+  Future<TaskResult> call() async {
+    final Device device = await devices.workingDevice;
+    await device.unlock();
+    final String deviceId = device.deviceId;
+    final Directory galleryDirectory = dir('${flutterDirectory.path}/dev/integration_tests/flutter_gallery');
+    await inDirectory<void>(galleryDirectory, () async {
+      String applicationBinaryPath;
+      if (deviceOperatingSystem == DeviceOperatingSystem.android) {
+        section('BUILDING APPLICATION');
+        await flutter(
+          'build',
+          options: <String>[
+            'apk',
+            '--no-android-gradle-daemon',
             '--profile',
             '-t',
             'test_driver/$testFile.dart',
-          ];
-        default:
-          throw Exception('$targetPlatform has no build configuration');
+            '--target-platform',
+            'android-arm,android-arm64',
+          ],
+        );
+        applicationBinaryPath = 'build/app/outputs/flutter-apk/app-profile.apk';
       }
-    }
 
-  @override
-  List<String> getTestArgs(String deviceId) {
-    final String testDriver = driverFile ?? (semanticsEnabled
-      ? '${testFile}_with_semantics_test'
-      : '${testFile}_test');
-    return <String>[
+      final String testDriver = driverFile ?? (semanticsEnabled
+          ? '${testFile}_with_semantics_test'
+          : '${testFile}_test');
+      section('DRIVE START');
+      await flutter('drive', options: <String>[
         '--profile',
         if (needFullTimeline)
           '--trace-startup',
-        '-t', 'test_driver/$testFile.dart',
-        '--use-application-binary="${getApplicationBinaryPath()}"',
-        '--driver', 'test_driver/$testDriver.dart',
-        '-d', deviceId,
-        '--screenshot', hostAgent.dumpDirectory.path,
-      ];
-  }
-
-  @override
-  Future<TaskResult> parseTaskResult() async {
+        if (applicationBinaryPath != null)
+          '--use-application-binary=$applicationBinaryPath'
+        else
+          ...<String>[
+            '-t',
+            'test_driver/$testFile.dart',
+          ],
+        '--driver',
+        'test_driver/$testDriver.dart',
+        '-d',
+        deviceId,
+        '--screenshot',
+        hostAgent.dumpDirectory.path,
+      ]);
+    });
+
+    final String testOutputDirectory = Platform.environment['FLUTTER_TEST_OUTPUTS_DIR'] ?? '${galleryDirectory.path}/build';
     final Map<String, dynamic> summary = json.decode(
       file('$testOutputDirectory/$timelineSummaryFile.json').readAsStringSync(),
     ) as Map<String, dynamic>;
@@ -151,22 +144,6 @@ class GalleryTransitionTest extends BuildTestTask {
       ],
     );
   }
-
-  @override
-  String getApplicationBinaryPath() {
-    if (applicationBinaryPath != null) {
-      return applicationBinaryPath;
-    }
-
-    switch (targetPlatform) {
-      case DeviceOperatingSystem.android:
-        return 'build/app/outputs/flutter-apk/app-profile.apk';
-      case DeviceOperatingSystem.ios:
-        return 'build/ios/iphoneos/Flutter Gallery.app';
-      default:
-        throw UnimplementedError('getApplicationBinaryPath does not support $deviceOperatingSystem');
-    }
-  }
 }
 
 int _countMissedTransitions(Map<String, List<int>> transitions) {

dev/devicelab/test/tasks/build_test_task_test.dart

@@ -11,7 +11,7 @@ void main() {
   final Map<String, String> isolateParams = <String, String>{
     'runFlutterConfig': 'false',
     'runProcessCleanup': 'false',
-    'timeoutInSeconds': '5',
+    'timeoutInMinutes': '1',
   };
 
   test('runs build and test when no args are passed', () async {
@@ -62,23 +62,4 @@ void main() {
     );
     expect(result.message, 'Task failed: Exception: Application binary path is only used for tests');
   });
-
-  test('sets target platform when given', () async {
-    final TaskResult result = await runTask(
-      'smoke_test_build_test_target_platform',
-      taskArgs: <String>['--target-platform=fake'],
-      deviceId: 'FAKE_SUCCESS',
-      isolateParams: isolateParams,
-    );
-    expect(result.message, 'success');
-  });
-
-  test('defaults target platform to task deviceOperatingSystem', () async {
-    final TaskResult result = await runTask(
-      'smoke_test_build_test_target_platform',
-      deviceId: 'FAKE_SUCCESS',
-      isolateParams: isolateParams,
-    );
-    expect(result.message, 'Task failed: Exception: Only DeviceOperatingSystem.fake is supported');
-  });
 }