Simplify the TestFileArgs constructors (#9005)

Author: srawlins

packages/devtools_app/integration_test/test_infra/run/_in_file_args.dart

@@ -7,10 +7,8 @@ import 'dart:io';
 
 import '_test_app_driver.dart';
 
-enum TestFileArgItems { experimentsOn, appPath }
-
-const _defaultFlutterAppPath = 'test/test_infra/fixtures/flutter_app';
-const _defaultDartCliAppPath = 'test/test_infra/fixtures/empty_app.dart';
+const defaultFlutterAppPath = 'test/test_infra/fixtures/flutter_app';
+const defaultDartCliAppPath = 'test/test_infra/fixtures/empty_app.dart';
 
 /// Test arguments, defined inside the test file as a comment.
 class TestFileArgs {
@@ -22,56 +20,46 @@ class TestFileArgs {
     return TestFileArgs.fromFileContent(content, testAppDevice: testAppDevice);
   }
 
+  /// Returns a [TestFileArgs] parsed from [fileContent].
+  ///
+  /// Separate from [TestFileArgs.new] for easier testing.
   factory TestFileArgs.fromFileContent(
     String fileContent, {
     required TestAppDevice testAppDevice,
   }) {
-    final testFileArgItems = _parseFileContent(fileContent);
-
-    for (final arg in testFileArgItems.keys) {
-      testFileArgItems.putIfAbsent(arg, () => null);
-    }
-
-    return TestFileArgs.parse(testFileArgItems, testAppDevice: testAppDevice);
+    final args = _parseFileContent(fileContent);
+    final appPath =
+        args[_TestFileArgItems.appPath] ??
+        (testAppDevice == TestAppDevice.cli
+            ? defaultDartCliAppPath
+            : defaultFlutterAppPath);
+    return TestFileArgs._parse(args, appPath: appPath);
   }
 
-  TestFileArgs.parse(
-    Map<TestFileArgItems, dynamic> map, {
-    required TestAppDevice testAppDevice,
-  }) : experimentsOn = map[TestFileArgItems.experimentsOn] ?? false,
-       appPath =
-           map[TestFileArgItems.appPath] ??
-           (testAppDevice == TestAppDevice.cli
-               ? _defaultDartCliAppPath
-               : _defaultFlutterAppPath);
+  TestFileArgs._parse(
+    Map<_TestFileArgItems, dynamic> args, {
+    required this.appPath,
+  }) : experimentsOn = args[_TestFileArgItems.experimentsOn] ?? false;
 
-  /// If true, experiments will be enabled in the test.
+  /// Whether experiments are enabled in the test.
   final bool experimentsOn;
 
-  /// Path to the application to connect to.
+  /// The path to the application to connect to.
   final String appPath;
-}
 
-final _argRegex = RegExp(
-  r'^\/\/\s*test-argument\s*:\s*(\w*)\s*=\s*(\S*)\s*$',
-  multiLine: true,
-);
-
-Map<TestFileArgItems, dynamic> _parseFileContent(String fileContent) {
-  final matches = _argRegex.allMatches(fileContent);
-
-  final entries = matches.map<MapEntry<TestFileArgItems, dynamic>>((
-    RegExpMatch m,
-  ) {
-    final name = m.group(1) ?? '';
-    if (name.isEmpty) {
-      throw ArgumentError(
-        'Name of test argument should be provided: [${m.group(0)}].',
-      );
-    }
-    final value = m.group(2) ?? '';
-    return MapEntry(TestFileArgItems.values.byName(name), jsonDecode(value));
-  });
+  /// Parses 'test-argument' comments in [fileContent].
+  static Map<_TestFileArgItems, dynamic> _parseFileContent(String fileContent) {
+    return {
+      for (final m in _argRegex.allMatches(fileContent))
+        _TestFileArgItems.values.byName(m.group(1)!): jsonDecode(m.group(2)!),
+    };
+  }
 
-  return Map.fromEntries(entries);
+  static final _argRegex = RegExp(
+    r'^\/\/\s*test-argument\s*:\s*(\w+)\s*=\s*(\S*)\s*$',
+    multiLine: true,
+  );
 }
+
+/// The different arguments accepted as "file args."
+enum _TestFileArgItems { experimentsOn, appPath }

packages/devtools_app/test/integration_test_infra/in_file_args_test.dart

@@ -9,13 +9,13 @@ import '../../integration_test/test_infra/run/_test_app_driver.dart';
 
 const _testAppPath = 'test/test_infra/fixtures/memory_app';
 
-final _defaultArgs = TestFileArgs.parse(
-  {},
+final _defaultArgs = TestFileArgs.fromFileContent(
+  '',
   testAppDevice: TestAppDevice.flutterTester,
 );
 
-final _defaultArgsForCliDevice = TestFileArgs.parse(
-  {},
+final _defaultArgsForCliDevice = TestFileArgs.fromFileContent(
+  '',
   testAppDevice: TestAppDevice.cli,
 );
 
@@ -24,19 +24,15 @@ final tests = [
     name: 'empty',
     input: '',
     testAppDevice: TestAppDevice.flutterTester,
-    output: TestFileArgs.parse({
-      TestFileArgItems.experimentsOn: _defaultArgs.experimentsOn,
-      TestFileArgItems.appPath: _defaultArgs.appPath,
-    }, testAppDevice: TestAppDevice.flutterTester),
+    expectedExperimentsOn: _defaultArgs.experimentsOn,
+    expectedAppPath: defaultFlutterAppPath,
   ),
   _InFileTestArgsTest(
     name: 'empty',
     input: '',
     testAppDevice: TestAppDevice.cli,
-    output: TestFileArgs.parse({
-      TestFileArgItems.experimentsOn: _defaultArgsForCliDevice.experimentsOn,
-      TestFileArgItems.appPath: _defaultArgsForCliDevice.appPath,
-    }, testAppDevice: TestAppDevice.cli),
+    expectedExperimentsOn: _defaultArgsForCliDevice.experimentsOn,
+    expectedAppPath: defaultDartCliAppPath,
   ),
   _InFileTestArgsTest(
     name: 'non-empty',
@@ -53,10 +49,8 @@ final tests = [
 import 'dart:ui' as ui;
 ''',
     testAppDevice: TestAppDevice.flutterTester,
-    output: TestFileArgs.parse({
-      TestFileArgItems.experimentsOn: true,
-      TestFileArgItems.appPath: _testAppPath,
-    }, testAppDevice: TestAppDevice.flutterTester),
+    expectedExperimentsOn: true,
+    expectedAppPath: _testAppPath,
   ),
 ];
 
@@ -67,8 +61,8 @@ void main() {
         t.input,
         testAppDevice: t.testAppDevice,
       );
-      expect(args.experimentsOn, t.output.experimentsOn);
-      expect(args.appPath, t.output.appPath);
+      expect(args.experimentsOn, t.expectedExperimentsOn);
+      expect(args.appPath, t.expectedAppPath);
     });
   }
 }
@@ -78,11 +72,13 @@ class _InFileTestArgsTest {
     required this.name,
     required this.input,
     required this.testAppDevice,
-    required this.output,
+    required this.expectedExperimentsOn,
+    required this.expectedAppPath,
   });
 
   final String name;
   final String input;
   final TestAppDevice testAppDevice;
-  final TestFileArgs output;
+  final bool expectedExperimentsOn;
+  final String expectedAppPath;
 }