++

testing/scenario_app/bin/run_android_tests.dart

@@ -7,188 +7,80 @@ import 'dart:convert';
 import 'dart:io';
 import 'dart:typed_data';
 
-import 'package:args/args.dart';
 import 'package:dir_contents_diff/dir_contents_diff.dart' show dirContentsDiff;
 import 'package:engine_repo_tools/engine_repo_tools.dart';
 import 'package:path/path.dart';
 import 'package:process/process.dart';
 import 'package:skia_gold_client/skia_gold_client.dart';
 
 import 'utils/adb_logcat_filtering.dart';
+import 'utils/environment.dart';
 import 'utils/logs.dart';
+import 'utils/options.dart';
 import 'utils/process_manager_extension.dart';
 import 'utils/screenshot_transformer.dart';
 
-void _withTemporaryCwd(String path, void Function() callback) {
-  final String originalCwd = Directory.current.path;
-  Directory.current = Directory(path).parent.path;
+// If you update the arguments, update the documentation in the README.md file.
+void main(List<String> args) async {
+  // Get some basic environment information to guide the rest of the program.
+  final Environment environment = Environment(
+    isCi: Platform.environment['LUCI_CONTEXT'] != null,
+    showVerbose: Options.showVerbose(args),
+    logsDir: Platform.environment['FLUTTER_LOGS_DIR'],
+  );
 
-  try {
-    callback();
-  } finally {
-    Directory.current = originalCwd;
+  // Determine if the CWD is within an engine checkout.
+  final Engine? localEngineDir = Engine.tryFindWithin();
+
+  // Show usage if requested.
+  if (Options.showUsage(args)) {
+    stdout.writeln(Options.usage(
+      environment: environment, 
+      localEngineDir: localEngineDir,
+    ));
+    return;
   }
-}
 
-// If you update the arguments, update the documentation in the README.md file.
-void main(List<String> args) async {
-  final Engine? engine = Engine.tryFindWithin();
-  final ArgParser parser = ArgParser()
-    ..addFlag(
-      'help',
-      help: 'Prints usage information',
-      negatable: false,
-    )
-    ..addFlag(
-      'verbose',
-      help: 'Prints verbose output',
-      negatable: false,
-    )
-    ..addOption(
-      'adb',
-      help: 'Path to the adb tool',
-      defaultsTo: engine != null
-          ? join(
-              engine.srcDir.path,
-              'third_party',
-              'android_tools',
-              'sdk',
-              'platform-tools',
-              'adb',
-            )
-          : null,
-    )
-    ..addOption(
-      'ndk-stack',
-      help: 'Path to the ndk-stack tool',
-      defaultsTo: engine != null
-          ? join(
-              engine.srcDir.path,
-              'third_party',
-              'android_tools',
-              'ndk',
-              'prebuilt',
-              () {
-                if (Platform.isLinux) {
-                  return 'linux-x86_64';
-                } else if (Platform.isMacOS) {
-                  return 'darwin-x86_64';
-                } else if (Platform.isWindows) {
-                  return 'windows-x86_64';
-                } else {
-                  throw UnsupportedError('Unsupported platform: ${Platform.operatingSystem}');
-                }
-              }(),
-              'bin',
-              'ndk-stack',
-            )
-          : null,
-    )
-    ..addOption(
-      'out-dir',
-      help: 'Out directory',
-      defaultsTo: engine
-          ?.outputs()
-          .where((Output o) => basename(o.path.path).startsWith('android_'))
-          .firstOrNull
-          ?.path
-          .path,
-    )
-    ..addOption(
-      'smoke-test',
-      help: 'Runs a single test to verify the setup',
-    )
-    ..addFlag(
-      'use-skia-gold',
-      help: 'Use Skia Gold to compare screenshots.',
-      defaultsTo: isLuciEnv,
-    )
-    ..addFlag(
-      'enable-impeller',
-      help: 'Enable Impeller for the Android app.',
-    )
-    ..addOption(
-      'output-contents-golden',
-      help: 'Path to a file that contains the expected filenames of golden files.',
-      defaultsTo: engine != null
-          ? join(
-              engine.srcDir.path,
-              'flutter',
-              'testing',
-              'scenario_app',
-              'android',
-              'expected_golden_output.txt',
-            )
-          : null,
-    )
-    ..addOption(
-      'impeller-backend',
-      help: 'The Impeller backend to use for the Android app.',
-      allowed: <String>['vulkan', 'opengles'],
-      defaultsTo: 'vulkan',
-    )
-    ..addOption(
-      'logs-dir',
-      help: 'The directory to store the logs and screenshots. Defaults to '
-          'the value of the FLUTTER_LOGS_DIR environment variable, if set, '
-          'otherwise it defaults to a path within out-dir.',
-      defaultsTo: Platform.environment['FLUTTER_LOGS_DIR'],
+  // Parse the command line arguments.
+  final Options options;
+  try {
+    options = Options.parse(
+      args,
+      environment: environment,
+      localEngine: localEngineDir,
     );
+  } on FormatException catch (error) {
+    stderr.writeln(error);
+    stderr.writeln(Options.usage(
+      environment: environment,
+      localEngineDir: localEngineDir,
+    ));
+    exitCode = 1;
+    return;
+  }
 
   runZonedGuarded(
     () async {
-      final ArgResults results = parser.parse(args);
-      if (results['help'] as bool) {
-        stdout.writeln(parser.usage);
-        return;
-      }
-
-      if (results['out-dir'] == null) {
-        panic(<String>['--out-dir is required']);
-      }
-      if (results['adb'] == null) {
-        panic(<String>['--adb is required']);
-      }
-
-      final bool verbose = results['verbose'] as bool;
-      final Directory outDir = Directory(results['out-dir'] as String);
-      final File adb = File(results['adb'] as String);
-      final bool useSkiaGold = results['use-skia-gold'] as bool;
-      final String? smokeTest = results['smoke-test'] as String?;
-      final bool enableImpeller = results['enable-impeller'] as bool;
-      final String? contentsGolden = results['output-contents-golden'] as String?;
-      final _ImpellerBackend? impellerBackend = _ImpellerBackend.tryParse(results['impeller-backend'] as String?);
-      if (enableImpeller && impellerBackend == null) {
-        panic(<String>[
-          'invalid graphics-backend',
-          results['impeller-backend'] as String? ?? '<null>'
-        ]);
-      }
-      final Directory logsDir = Directory(results['logs-dir'] as String? ?? join(outDir.path, 'scenario_app', 'logs'));
-      final String? ndkStack = results['ndk-stack'] as String?;
-      if (ndkStack == null) {
-        panic(<String>['--ndk-stack is required']);
-      }
       await _run(
-        verbose: verbose,
-        outDir: outDir,
-        adb: adb,
-        smokeTestFullPath: smokeTest,
-        useSkiaGold: useSkiaGold,
-        enableImpeller: enableImpeller,
-        impellerBackend: impellerBackend,
-        logsDir: logsDir,
-        contentsGolden: contentsGolden,
-        ndkStack: ndkStack,
+        verbose: options.verbose,
+        outDir: Directory(options.outDir),
+        adb: File(options.adb),
+        smokeTestFullPath: options.smokeTest,
+        useSkiaGold: options.useSkiaGold,
+        enableImpeller: options.enableImpeller,
+        impellerBackend: _ImpellerBackend.tryParse(options.impellerBackend),
+        logsDir: Directory(options.logsDir),
+        contentsGolden: options.outputContentsGolden,
+        ndkStack: options.ndkStack,
       );
       exit(0);
     },
     (Object error, StackTrace stackTrace) {
       if (error is! Panic) {
-        stderr.writeln(error);
+        stderr.writeln('Unhandled error: $error');
         stderr.writeln(stackTrace);
       }
-      exit(1);
+      exitCode = 1;
     },
   );
 }
@@ -222,18 +114,6 @@ Future<void> _run({
   required String ndkStack,
 }) async {
   const ProcessManager pm = LocalProcessManager();
-
-  if (!outDir.existsSync()) {
-    panic(<String>[
-      'out-dir does not exist: $outDir',
-      'make sure to build the selected engine variant'
-    ]);
-  }
-
-  if (!adb.existsSync()) {
-    panic(<String>['cannot find adb: $adb', 'make sure to run gclient sync']);
-  }
-
   final String scenarioAppPath = join(outDir.path, 'scenario_app');
   final String logcatPath = join(logsDir.path, 'logcat.txt');
 
@@ -531,3 +411,14 @@ Future<void> _run({
     }
   }
 }
+
+void _withTemporaryCwd(String path, void Function() callback) {
+  final String originalCwd = Directory.current.path;
+  Directory.current = Directory(path).path;
+
+  try {
+    callback();
+  } finally {
+    Directory.current = originalCwd;
+  }
+}

testing/scenario_app/bin/utils/environment.dart

@@ -25,7 +25,7 @@ final class Environment {
   final bool showVerbose;
 
   /// What directory to store logs and screenshots in.
-  final String logsDir;
+  final String? logsDir;
 
   @override
   bool operator ==(Object o) {

testing/scenario_app/bin/utils/options.dart

@@ -9,54 +9,46 @@ import 'environment.dart';
 /// Command line options and parser for the Android `scenario_app` test runner.
 extension type const Options._(ArgResults _args) {
   /// Parses the command line [args] into a set of options.
+  ///
+  /// Throws a [FormatException] if command line arguments are invalid.
   factory Options.parse(
     List<String> args, {
     required Environment environment,
     required Engine? localEngine,
   }) {
     final ArgResults results = _parser(environment, localEngine).parse(args);
     final Options options = Options._(results);
-    
+
     // The 'adb' tool must exist.
     if (results['adb'] == null) {
-      throw ArgumentError('The --adb option must be set.');
+      throw const FormatException('The --adb option must be set.');
     } else if (!io.File(options.adb).existsSync()) {
-      throw ArgumentError('The adb tool does not exist at ${options.adb}.');
+      throw FormatException(
+        'The adb tool does not exist at ${options.adb}.',
+      );
     }
 
     // The 'ndk-stack' tool must exist.
     if (results['ndk-stack'] == null) {
-      throw ArgumentError('The --ndk-stack option must be set.');
+      throw const FormatException('The --ndk-stack option must be set.');
     } else if (!io.File(options.ndkStack).existsSync()) {
-      throw ArgumentError('The ndk-stack tool does not exist at ${options.ndkStack}.');
+      throw FormatException(
+        'The ndk-stack tool does not exist at ${options.ndkStack}.',
+      );
     }
 
     // The 'out-dir' must exist.
     if (results['out-dir'] == null) {
-      throw ArgumentError('The --out-dir option must be set.');
+      throw const FormatException('The --out-dir option must be set.');
     } else if (!io.Directory(options.outDir).existsSync()) {
-      throw ArgumentError('The out directory does not exist at ${options.outDir}.');
+      throw FormatException(
+        'The out directory does not exist at ${options.outDir}.',
+      );
     }
 
     return options;
   }
 
-  /// A subset of command line options used by the `scenario_app` runner.
-  ///
-  /// These options are expected to be parsed before the full set of options.
-  static final ArgParser _miniParser = ArgParser()
-    ..addFlag(
-      'verbose',
-      abbr: 'v',
-      help: 'Enable verbose logging',
-    )
-    ..addFlag(
-      'help',
-      abbr: 'h',
-      help: 'Print usage information',
-      negatable: false,
-    );
-
   /// Whether usage information should be shown based on command line [args].
   ///
   /// This is a shortcut that can be used to determine if the usage information
@@ -73,8 +65,10 @@ extension type const Options._(ArgResults _args) {
   /// }
   /// ```
   static bool showUsage(List<String> args) {
-    final ArgResults results = _miniParser.parse(args);
-    return results['help'] as bool;
+    // If any of the arguments are '--help' or -'h'.
+    return args.isNotEmpty && args.any((String arg) {
+      return arg == '--help' || arg == '-h';
+    });
   }
 
   /// Whether verbose logging should be enabled based on command line [args].
@@ -90,8 +84,10 @@ extension type const Options._(ArgResults _args) {
   /// }
   /// ```
   static bool showVerbose(List<String> args) {
-    final ArgResults results = _miniParser.parse(args);
-    return results['verbose'] as bool;
+    // If any of the arguments are '--verbose' or -'v'.
+    return args.isNotEmpty && args.any((String arg) {
+      return arg == '--verbose' || arg == '-v';
+    });
   }
 
   /// Returns usage information for the `scenario_app` test runner.
@@ -261,7 +257,10 @@ extension type const Options._(ArgResults _args) {
   String get impellerBackend => _args['impeller-backend'] as String;
 
   /// Path to a directory where logs and screenshots are stored.
-  String get logsDir => _args['logs-dir'] as String;
+  String get logsDir {
+    final String? logsDir = _args['logs-dir'] as String?;
+    return logsDir ?? p.join(outDir, 'logs');
+  }
 
   /// Path to the Android Debug Bridge (adb) executable.
   String get adb => _args['adb'] as String;