[custom-devices] general improvements, add custom-devices subcommand, better...

[custom-devices] general improvements, add custom-devices subcommand, better error handling (#82043)

packages/flutter_tools/lib/executable.dart

@@ -24,6 +24,7 @@ import 'src/commands/channel.dart';
 import 'src/commands/clean.dart';
 import 'src/commands/config.dart';
 import 'src/commands/create.dart';
+import 'src/commands/custom_devices.dart';
 import 'src/commands/daemon.dart';
 import 'src/commands/devices.dart';
 import 'src/commands/doctor.dart';
@@ -149,6 +150,16 @@ List<FlutterCommand> generateCommands({
   ChannelCommand(verboseHelp: verboseHelp),
   CleanCommand(verbose: verbose),
   ConfigCommand(verboseHelp: verboseHelp),
+  CustomDevicesCommand(
+    customDevicesConfig: globals.customDevicesConfig,
+    operatingSystemUtils: globals.os,
+    terminal: globals.terminal,
+    platform: globals.platform,
+    featureFlags: featureFlags,
+    processManager: globals.processManager,
+    fileSystem: globals.fs,
+    logger: globals.logger
+  ),
   CreateCommand(verboseHelp: verboseHelp),
   DaemonCommand(hidden: !verboseHelp),
   DevicesCommand(verboseHelp: verboseHelp),

packages/flutter_tools/lib/src/base/config.dart

@@ -23,17 +23,59 @@ class Config {
   /// `.flutter_$name` already exists there. On other platforms the
   /// configuration file will always be a file named `.flutter_$name` in the
   /// home directory.
+  ///
+  /// Uses some good default behaviours:
+  /// - deletes the file if it's not valid JSON
+  /// - reports an empty config in that case
+  /// - logs and catches any exceptions
   factory Config(
+    String name, {
+    required FileSystem fileSystem,
+    required Logger logger,
+    required Platform platform
+  }) {
+    return Config._common(
+      name,
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+      managed: false
+    );
+  }
+
+  /// Similiar to the default config constructor, but with some different
+  /// behaviours:
+  /// - will not delete the config if it's not valid JSON
+  /// - will log but also rethrow any exceptions while loading the JSON, so
+  ///   you can actually detect whether something went wrong
+  ///
+  /// Useful if you want some more control.
+  factory Config.managed(
+    String name, {
+    required FileSystem fileSystem,
+    required Logger logger,
+    required Platform platform
+  }) {
+    return Config._common(
+      name,
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+      managed: true
+    );
+  }
+
+  factory Config._common(
     String name, {
     required FileSystem fileSystem,
     required Logger logger,
     required Platform platform,
-    bool deleteFileOnFormatException = true
+    bool managed = false
   }) {
     final String filePath = _configPath(platform, fileSystem, name);
     final File file = fileSystem.file(filePath);
     file.parent.createSync(recursive: true);
-    return Config.createForTesting(file, logger, deleteFileOnFormatException: deleteFileOnFormatException);
+    return Config.createForTesting(file, logger, managed: managed);
   }
 
   /// Constructs a new [Config] object from a file called [name] in
@@ -44,19 +86,19 @@ class Config {
     String name = 'test',
     Directory? directory,
     Logger? logger,
-    bool deleteFileOnFormatException = true
+    bool managed = false
   }) {
     directory ??= MemoryFileSystem.test().directory('/');
     return Config.createForTesting(
       directory.childFile('.${kConfigDir}_$name'),
       logger ?? BufferLogger.test(),
-      deleteFileOnFormatException: deleteFileOnFormatException
+      managed: managed
     );
   }
 
   /// Test only access to the Config constructor.
   @visibleForTesting
-  Config.createForTesting(File file, Logger logger, {bool deleteFileOnFormatException = true}) : _file = file, _logger = logger {
+  Config.createForTesting(File file, Logger logger, {bool managed = false}) : _file = file, _logger = logger {
     if (!_file.existsSync()) {
       return;
     }
@@ -68,20 +110,26 @@ class Config {
       _logger
         ..printError('Failed to decode preferences in ${_file.path}.')
         ..printError(
-            'You may need to reapply any previously saved configuration '
-            'with the "flutter config" command.',
+          'You may need to reapply any previously saved configuration '
+          'with the "flutter config" command.',
         );
 
-      if (deleteFileOnFormatException) {
+      if (managed) {
+        rethrow;
+      } else {
         _file.deleteSync();
       }
     } on Exception catch (err) {
       _logger
         ..printError('Could not read preferences in ${file.path}.\n$err')
         ..printError(
-            'You may need to resolve the error above and reapply any previously '
-            'saved configuration with the "flutter config" command.',
+          'You may need to resolve the error above and reapply any previously '
+          'saved configuration with the "flutter config" command.',
         );
+
+      if (managed) {
+        rethrow;
+      }
     }
   }
 

packages/flutter_tools/lib/src/base/terminal.dart

@@ -115,6 +115,9 @@ abstract class Terminal {
 
   /// Return keystrokes from the console.
   ///
+  /// This is a single-subscription stream. This stream may be closed before
+  /// the application exits.
+  ///
   /// Useful when the console is in [singleCharMode].
   Stream<String> get keystrokes;
 

packages/flutter_tools/lib/src/context_runner.dart

@@ -172,6 +172,11 @@ Future<T> runInContext<T>(
         logger: globals.logger,
         platform: globals.platform,
       ),
+      CustomDevicesConfig: () => CustomDevicesConfig(
+        fileSystem: globals.fs,
+        logger: globals.logger,
+        platform: globals.platform
+      ),
       CrashReporter: () => CrashReporter(
         fileSystem: globals.fs,
         logger: globals.logger,
@@ -201,11 +206,7 @@ Future<T> runInContext<T>(
         ),
         operatingSystemUtils: globals.os,
         terminal: globals.terminal,
-        customDevicesConfig: CustomDevicesConfig(
-          fileSystem: globals.fs,
-          logger: globals.logger,
-          platform: globals.platform
-        ),
+        customDevicesConfig: globals.customDevicesConfig,
         uwptool: UwpTool(
           artifacts: globals.artifacts,
           logger: globals.logger,

packages/flutter_tools/lib/src/custom_devices/custom_devices_config.dart

@@ -21,44 +21,62 @@ class CustomDevicesConfig {
   /// when it's not valid JSON (which other configurations do) and will not
   /// be implicitly created when it doesn't exist.
   CustomDevicesConfig({
+    required Platform platform,
     required FileSystem fileSystem,
     required Logger logger,
-    required Platform platform
-  }) : _fileSystem = fileSystem,
-       _config = Config(
+  }) : _platform = platform,
+       _fileSystem = fileSystem,
+       _logger = logger,
+       _configLoader = (() => Config.managed(
          _kCustomDevicesConfigName,
          fileSystem: fileSystem,
          logger: logger,
          platform: platform,
-         deleteFileOnFormatException: false
-       )
-  {
-    ensureFileExists();
-  }
+       ));
 
   @visibleForTesting
   CustomDevicesConfig.test({
     required FileSystem fileSystem,
+    required Logger logger,
     Directory? directory,
-    required Logger logger
-  }) : _fileSystem = fileSystem,
-       _config = Config.test(
+    Platform? platform,
+  }) : _platform = platform ?? FakePlatform(),
+       _fileSystem = fileSystem,
+       _logger = logger,
+       _configLoader = (() => Config.test(
          name: _kCustomDevicesConfigName,
          directory: directory,
          logger: logger,
-         deleteFileOnFormatException: false
-       )
-  {
-    ensureFileExists();
-  }
+         managed: true
+       ));
 
   static const String _kCustomDevicesConfigName = 'custom_devices.json';
   static const String _kCustomDevicesConfigKey = 'custom-devices';
   static const String _kSchema = r'$schema';
   static const String _kCustomDevices = 'custom-devices';
 
+  final Platform _platform;
   final FileSystem _fileSystem;
-  final Config _config;
+  final Logger _logger;
+  final Config Function() _configLoader;
+
+  // When the custom devices feature is disabled, CustomDevicesConfig is
+  // constructed anyway. So loading the config in the constructor isn't a good
+  // idea. (The Config ctor logs any errors)
+  //
+  // I also didn't want to introduce a FeatureFlags argument to the constructor
+  // and conditionally load the config when the feature is enabled, because
+  // sometimes we need that Config object even when the feature is disabled.
+  // For example inside ensureFileExists, which is used when enabling
+  // the feature.
+  //
+  // Instead, users of this config should handle the feature flags. So for
+  // example don't get [devices] when the feature is disabled.
+  Config? __config;
+  Config get _config {
+    __config ??= _configLoader();
+    return __config!;
+  }
 
   String get _defaultSchema {
     final Uri uri = _fileSystem
@@ -84,28 +102,123 @@ class CustomDevicesConfig {
   void ensureFileExists() {
     if (!_fileSystem.file(_config.configPath).existsSync()) {
       _config.setValue(_kSchema, _defaultSchema);
-      _config.setValue(_kCustomDevices, <dynamic>[CustomDeviceConfig.example.toJson()]);
+      _config.setValue(_kCustomDevices, <dynamic>[
+        CustomDeviceConfig.getExampleForPlatform(_platform).toJson(),
+      ]);
     }
   }
 
+  List<dynamic>? _getDevicesJsonValue() {
+    final dynamic json = _config.getValue(_kCustomDevicesConfigKey);
+
+    if (json == null) {
+      return null;
+    } else if (json is! List) {
+      const String msg = "Could not load custom devices config. config['$_kCustomDevicesConfigKey'] is not a JSON array.";
+      _logger.printError(msg);
+      throw const CustomDeviceRevivalException(msg);
+    }
+
+    return json;
+  }
+
   /// Get the list of [CustomDeviceConfig]s that are listed in the config file
   /// including disabled ones.
   ///
-  /// Returns an empty list when the config could not be loaded.
+  /// Throws an Exception when the config could not be loaded and logs any
+  /// errors.
   List<CustomDeviceConfig> get devices {
-    final dynamic json = _config.getValue(_kCustomDevicesConfigKey);
+    final List<dynamic>? typedListNullable = _getDevicesJsonValue();
+    if (typedListNullable == null) {
+      return <CustomDeviceConfig>[];
+    }
 
-    if (json == null) {
+    final List<dynamic> typedList = typedListNullable;
+    final List<CustomDeviceConfig> revived = <CustomDeviceConfig>[];
+    for (final MapEntry<int, dynamic> entry in typedList.asMap().entries) {
+      try {
+        revived.add(CustomDeviceConfig.fromJson(entry.value));
+      } on CustomDeviceRevivalException catch (e) {
+        final String msg = 'Could not load custom device from config index ${entry.key}: $e';
+        _logger.printError(msg);
+        throw CustomDeviceRevivalException(msg);
+      }
+    }
+
+    return revived;
+  }
+
+  /// Get the list of [CustomDeviceConfig]s that are listed in the config file
+  /// including disabled ones.
+  ///
+  /// Returns an empty list when the config could not be loaded and logs any
+  /// errors.
+  List<CustomDeviceConfig> tryGetDevices() {
+    try {
+      return devices;
+    } on Exception {
+      // any Exceptions are logged by [devices] already.
       return <CustomDeviceConfig>[];
     }
+  }
+
+  /// Set the list of [CustomDeviceConfig]s in the config file.
+  ///
+  /// It should generally be avoided to call this often, since this could mean
+  /// data loss. If you want to add or remove a device from the config,
+  /// consider using [add] or [remove].
+  set devices(List<CustomDeviceConfig> configs) {
+    _config.setValue(
+      _kCustomDevicesConfigKey,
+      configs.map<dynamic>((CustomDeviceConfig c) => c.toJson()).toList()
+    );
+  }
+
+  /// Add a custom device to the config file.
+  ///
+  /// Works even when some of the custom devices in the config file are not
+  /// valid.
+  ///
+  /// May throw a [CustomDeviceRevivalException] if `config['custom-devices']`
+  /// is not a list.
+  void add(CustomDeviceConfig config) {
+    _config.setValue(
+      _kCustomDevicesConfigKey,
+      <dynamic>[
+        ...?_getDevicesJsonValue(),
+        config.toJson()
+      ]
+    );
+  }
+
+  /// Returns true if the config file contains a device with id [deviceId].
+  bool contains(String deviceId) {
+    return devices.any((CustomDeviceConfig device) => device.id == deviceId);
+  }
 
-    final List<dynamic> typedList = json as List<dynamic>;
+  /// Removes the first device with this device id from the config file.
+  ///
+  /// Returns true if the device was successfully removed, false if a device
+  /// with this id could not be found.
+  bool remove(String deviceId) {
+    final List<CustomDeviceConfig> modifiedDevices = devices;
+
+    // we use this instead of filtering so we can detect if we actually removed
+    // anything.
+    final CustomDeviceConfig? device = modifiedDevices
+      .cast<CustomDeviceConfig?>()
+      .firstWhere((CustomDeviceConfig? d) => d!.id == deviceId,
+      orElse: () => null
+    );
+
+    if (device == null) {
+      return false;
+    }
 
-    return typedList.map((dynamic e) => CustomDeviceConfig.fromJson(e)).toList();
+    modifiedDevices.remove(device);
+    devices = modifiedDevices;
+    return true;
   }
 
-  // We don't have a setter for devices here because we don't need it and
-  // it also may overwrite any things done by the user that aren't explicitly
-  // tracked by the JSON-representation. For example comments (not possible right now,
-  // but they'd be useful so maybe in the future) or formatting.
+  String get configPath => _config.configPath;
 }

packages/flutter_tools/lib/src/globals_null_migrated.dart

@@ -26,6 +26,7 @@ import 'base/time.dart';
 import 'base/user_messages.dart';
 import 'build_system/build_system.dart';
 import 'cache.dart';
+import 'custom_devices/custom_devices_config.dart';
 import 'ios/ios_workflow.dart';
 import 'ios/plist_parser.dart';
 import 'ios/xcodeproj.dart';
@@ -218,3 +219,5 @@ FlutterProjectFactory get projectFactory {
     fileSystem: fs,
   );
 }
+
+CustomDevicesConfig get customDevicesConfig => context.get<CustomDevicesConfig>()!;

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

@@ -171,8 +171,12 @@ class FlutterCommandRunner extends CommandRunner<void> {
   Future<void> run(Iterable<String> args) {
     // Have an invocation of 'build' print out it's sub-commands.
     // TODO(ianh): Move this to the Build command itself somehow.
-    if (args.length == 1 && args.first == 'build') {
-      args = <String>['build', '-h'];
+    if (args.length == 1) {
+      if (args.first == 'build') {
+        args = <String>['build', '-h'];
+      } else if (args.first == 'custom-devices') {
+        args = <String>['custom-devices', '-h'];
+      }
     }
 
     return super.run(args);

packages/flutter_tools/static/custom-devices.schema.json

@@ -18,15 +18,27 @@
           },
           "label": {
             "description": "A more descriptive, user-friendly label for the device.",
-            "type": "string"
+            "type": "string",
+            "default": "",
+            "required": false
           },
           "sdkNameAndVersion": {
             "description": "Additional information about the device. For other devices, this is the SDK (for example Android SDK, Windows SDK) name and version.",
-            "type": "string"
+            "type": "string",
+            "default": "",
+            "required": false
           },
-          "disabled": {
-            "description": "If true, this device will be ignored completely by the flutter SDK and none of the commands configured will be called. You can use this as a way to comment out device configs you're still working on, for example.",
-            "type": "boolean"
+          "enabled": {
+            "description": "If false, this device will be ignored completely by the flutter SDK and none of the commands configured will be called. You can use this as a way to comment out device configs you're still working on, for example.",
+            "type": "boolean",
+            "default": true,
+            "required": false
+          },
+          "platform": {
+            "description": "The platform of the target device.",
+            "enum": ["linux-arm64", "linux-x64"],
+            "default": "linux-arm64",
+            "required": false
           },
           "ping": {
             "description": "The command to be invoked to ping the device. Used to find out whether its available or not. Every exit code unequal 0 will be treated as \"unavailable\". On Windows, consider providing a \"pingSuccessRegex\" since Windows' ping will also return 0 on failure. Make sure the command times out internally since it's not guaranteed the flutter SDK will enforce a timeout itself.",
@@ -43,7 +55,7 @@
             "description": "When the output of the ping command matches this regex (and the ping command finished with exit code 0), the ping will be considered successful and the pinged device reachable. If this regex is not provided the ping command will be considered successful when it returned with exit code 0. The regex syntax is the standard dart syntax.",
             "type": ["string", "null"],
             "format": "regex",
-            "default": "ms TTL=",
+            "default": "[<=]\\d+ms",
             "required": false
           },
           "postBuild": {

packages/flutter_tools/test/general.shard/custom_devices/custom_device_test.dart

@@ -99,7 +99,7 @@ void main() {
     id: 'testid',
     label: 'testlabel',
     sdkNameAndVersion: 'testsdknameandversion',
-    disabled: false,
+    enabled: true,
     pingCommand: const <String>['testping'],
     pingSuccessRegex: RegExp('testpingsuccess'),
     postBuildCommand: const <String>['testpostbuild'],
@@ -113,7 +113,7 @@ void main() {
 
   const String testConfigPingSuccessOutput = 'testpingsuccess\n';
   const String testConfigForwardPortSuccessOutput = 'testforwardportsuccess\n';
-  final CustomDeviceConfig disabledTestConfig = testConfig.copyWith(disabled: true);
+  final CustomDeviceConfig disabledTestConfig = testConfig.copyWith(enabled: false);
   final CustomDeviceConfig testConfigNonForwarding = testConfig.copyWith(
     explicitForwardPortCommand: true,
     forwardPortCommand: null,
@@ -577,6 +577,18 @@ void main() {
     expect(screenshotCommandWasExecuted, false);
     expect(screenshotFile.existsSync(), false);
   });
+
+  testWithoutContext('CustomDevice returns correct target platform', () async {
+    final CustomDevice device = CustomDevice(
+      config: testConfig.copyWith(
+        platform: TargetPlatform.linux_x64
+      ),
+      logger: BufferLogger.test(),
+      processManager: FakeProcessManager.empty()
+    );
+
+    expect(await device.targetPlatform, TargetPlatform.linux_x64);
+  });
 }
 
 class FakeBundleBuilder extends Fake implements BundleBuilder {

packages/flutter_tools/test/general.shard/custom_devices/custom_devices_config_test.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.
+
+// @dart = 2.8
+
+import 'package:file/memory.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/custom_devices/custom_device_config.dart';
+import 'package:flutter_tools/src/custom_devices/custom_devices_config.dart';
+
+import '../../src/common.dart';
+import '../../src/custom_devices_common.dart';
+
+Map<String, dynamic> copyJsonObjectWith(
+  Map<String, dynamic> object,
+  Map<String, dynamic> overrides
+) => Map<String, dynamic>.of(object)..addAll(overrides);
+
+void main() {
+  testWithoutContext("CustomDevicesConfig logs no error when 'custom-devices' key is missing in config", () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: null
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    expect(customDevicesConfig.devices, hasLength(0));
+    expect(logger.errorText, hasLength(0));
+  });
+
+  testWithoutContext("CustomDevicesConfig logs error when 'custom-devices' key is not a JSON array", () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <String, dynamic>{
+        'test': 'testvalue'
+      }
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    const String msg = "Could not load custom devices config. config['custom-devices'] is not a JSON array.";
+    expect(() => customDevicesConfig.devices, throwsA(const CustomDeviceRevivalException(msg)));
+    expect(logger.errorText, contains(msg));
+  });
+
+  testWithoutContext('CustomDeviceRevivalException serialization', () {
+    expect(
+      const CustomDeviceRevivalException('testmessage').toString(),
+      equals('testmessage')
+    );
+    expect(
+      const CustomDeviceRevivalException.fromDescriptions('testfielddescription', 'testexpectedvaluedescription').toString(),
+      equals('Expected testfielddescription to be testexpectedvaluedescription.')
+    );
+  });
+
+  testWithoutContext('CustomDevicesConfig can load test config and logs no errors', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        testConfigJson
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    final List<CustomDeviceConfig> devices = customDevicesConfig.devices;
+    expect(logger.errorText, hasLength(0));
+    expect(devices, hasLength(1));
+    expect(devices.first, equals(testConfig));
+  });
+
+  testWithoutContext('CustomDevicesConfig logs error when id is null', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'id': null
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    const String msg = 'Could not load custom device from config index 0: Expected id to be a string.';
+    expect(() => customDevicesConfig.devices, throwsA(const CustomDeviceRevivalException(msg)));
+    expect(logger.errorText, contains(msg));
+  });
+
+  testWithoutContext('CustomDevicesConfig logs error when id is not a string', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'id': 1
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    const String msg = 'Could not load custom device from config index 0: Expected id to be a string.';
+    expect(() => customDevicesConfig.devices, throwsA(const CustomDeviceRevivalException(msg)));
+    expect(logger.errorText, contains(msg));
+  });
+
+  testWithoutContext('CustomDevicesConfig logs error when label is not a string', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'label': 1
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger
+    );
+
+    const String msg = 'Could not load custom device from config index 0: Expected label to be a string.';
+    expect(() => customDevicesConfig.devices, throwsA(const CustomDeviceRevivalException(msg)));
+    expect(logger.errorText, contains(msg));
+  });
+
+  testWithoutContext('CustomDevicesConfig loads config when postBuild is null', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'postBuild': null
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger,
+    );
+
+    expect(customDevicesConfig.devices, hasLength(1));
+  });
+
+  testWithoutContext('CustomDevicesConfig loads config without port forwarding', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'forwardPort': null,
+            'forwardPortSuccessRegex': null
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger,
+    );
+
+    final List<CustomDeviceConfig> devices = customDevicesConfig.devices;
+
+    expect(devices, hasLength(1));
+    expect(devices.first.usesPortForwarding, false);
+  });
+
+  testWithoutContext('CustomDevicesConfig logs error when port forward command is given but not regex', () {
+    final BufferLogger logger = BufferLogger.test();
+    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
+    final Directory directory = fileSystem.directory('custom_devices_config');
+
+    writeCustomDevicesConfigFile(
+      directory,
+      json: <dynamic>[
+        copyJsonObjectWith(
+          testConfigJson,
+          <String, dynamic>{
+            'forwardPortSuccessRegex': null
+          },
+        ),
+      ],
+    );
+
+    final CustomDevicesConfig customDevicesConfig = CustomDevicesConfig.test(
+      fileSystem: fileSystem,
+      directory: directory,
+      logger: logger,
+    );
+
+    const String msg = 'Could not load custom device from config index 0: When forwardPort is given, forwardPortSuccessRegex must be specified too.';
+    expect(() => customDevicesConfig.devices, throwsA(const CustomDeviceRevivalException(msg)));
+    expect(logger.errorText, contains(msg));
+  });
+}

packages/flutter_tools/test/src/custom_devices_common.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.
+
+// @dart = 2.8
+
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/convert.dart';
+import 'package:flutter_tools/src/custom_devices/custom_device_config.dart';
+
+void writeCustomDevicesConfigFile(
+  Directory dir, {
+  List<CustomDeviceConfig> configs,
+  dynamic json
+}) {
+  dir.createSync(recursive: true);
+
+  final File file = dir.childFile('.flutter_custom_devices.json');
+  file.writeAsStringSync(jsonEncode(
+    <String, dynamic>{
+      'custom-devices': configs != null ?
+        configs.map<dynamic>((CustomDeviceConfig c) => c.toJson()).toList() :
+        json
+    },
+  ));
+}
+
+final CustomDeviceConfig testConfig = CustomDeviceConfig(
+  id: 'testid',
+  label: 'testlabel',
+  sdkNameAndVersion: 'testsdknameandversion',
+  enabled: true,
+  pingCommand: const <String>['testping'],
+  pingSuccessRegex: RegExp('testpingsuccess'),
+  postBuildCommand: const <String>['testpostbuild'],
+  installCommand: const <String>['testinstall'],
+  uninstallCommand: const <String>['testuninstall'],
+  runDebugCommand: const <String>['testrundebug'],
+  forwardPortCommand: const <String>['testforwardport'],
+  forwardPortSuccessRegex: RegExp('testforwardportsuccess')
+);
+
+const String testConfigPingSuccessOutput = 'testpingsuccess\n';
+const String testConfigForwardPortSuccessOutput = 'testforwardportsuccess\n';
+final CustomDeviceConfig disabledTestConfig = testConfig.copyWith(enabled: false);
+final CustomDeviceConfig testConfigNonForwarding = testConfig.copyWith(
+  explicitForwardPortCommand: true,
+  forwardPortCommand: null,
+  explicitForwardPortSuccessRegex: true,
+  forwardPortSuccessRegex: null,
+);
+
+const Map<String, dynamic> testConfigJson = <String, dynamic>{
+  'id': 'testid',
+  'label': 'testlabel',
+  'sdkNameAndVersion': 'testsdknameandversion',
+  'enabled': true,
+  'ping': <String>['testping'],
+  'pingSuccessRegex': 'testpingsuccess',
+  'postBuild': <String>['testpostbuild'],
+  'install': <String>['testinstall'],
+  'uninstall': <String>['testuninstall'],
+  'runDebug': <String>['testrundebug'],
+  'forwardPort': <String>['testforwardport'],
+  'forwardPortSuccessRegex': 'testforwardportsuccess'
+};