Start chromedriver once for running all integration tests (#5976)

packages/devtools_app/integration_test/run_tests.dart

@@ -4,6 +4,7 @@
 
 import 'dart:io';
 
+import 'test_infra/run/_chrome_driver.dart';
 import 'test_infra/run/_in_file_args.dart';
 import 'test_infra/run/run_test.dart';
 
@@ -18,26 +19,42 @@ const _testSuffix = '_test.dart';
 const _offlineIndicator = 'integration_test/test/offline';
 
 void main(List<String> args) async {
-  final testRunnerArgs = TestRunnerArgs(args, verifyValidTarget: false);
-  if (testRunnerArgs.testTarget != null) {
-    // TODO(kenz): add support for specifying a directory as the target instead
-    // of a single file.
-    await _runTest(testRunnerArgs);
-  } else {
-    // Run all tests since a target test was not provided.
-    final testDirectory = Directory(_testDirectory);
-    final testFiles = testDirectory
-        .listSync(recursive: true)
-        .where((testFile) => testFile.path.endsWith(_testSuffix));
-
-    for (final testFile in testFiles) {
-      final testTarget = testFile.path;
-      final newArgsWithTarget = TestRunnerArgs([
-        ...args,
-        '--${TestRunnerArgs.testTargetArg}=$testTarget',
-      ]);
-      await _runTest(newArgsWithTarget);
+  Exception? exception;
+  final chromedriver = ChromeDriver();
+
+  try {
+    // Start chrome driver before running the flutter integration test.
+    await chromedriver.start();
+
+    final testRunnerArgs = TestRunnerArgs(args, verifyValidTarget: false);
+    if (testRunnerArgs.testTarget != null) {
+      // TODO(kenz): add support for specifying a directory as the target instead
+      // of a single file.
+      await _runTest(testRunnerArgs);
+    } else {
+      // Run all tests since a target test was not provided.
+      final testDirectory = Directory(_testDirectory);
+      final testFiles = testDirectory
+          .listSync(recursive: true)
+          .where((testFile) => testFile.path.endsWith(_testSuffix));
+
+      for (final testFile in testFiles) {
+        final testTarget = testFile.path;
+        final newArgsWithTarget = TestRunnerArgs([
+          ...args,
+          '--${TestRunnerArgs.testTargetArg}=$testTarget',
+        ]);
+        await _runTest(newArgsWithTarget);
+      }
     }
+  } on Exception catch (e) {
+    exception = e;
+  } finally {
+    await chromedriver.stop();
+  }
+
+  if (exception != null) {
+    throw exception;
   }
 }
 

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

@@ -0,0 +1,37 @@
+// Copyright 2023 The Chromium 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 'dart:io';
+
+import '_io_utils.dart';
+import '_utils.dart';
+
+class ChromeDriver with IOMixin {
+  late final Process _process;
+
+  // TODO(kenz): add error messaging if the chromedriver executable is not
+  // found. We can also consider using web installers directly in this script:
+  // https://github.com/flutter/flutter/wiki/Running-Flutter-Driver-tests-with-Web#web-installers-repo.
+  Future<void> start() async {
+    try {
+      debugLog('starting the chromedriver process');
+      _process = await Process.start(
+        'chromedriver',
+        [
+          '--port=4444',
+        ],
+      );
+      listenToProcessOutput(_process, printTag: 'ChromeDriver');
+    } catch (e) {
+      // ignore: avoid-throw-in-catch-block, by design
+      throw Exception('Error starting chromedriver: $e');
+    }
+  }
+
+  Future<void> stop() async {
+    await cancelAllStreamSubscriptions();
+    debugLog('killing the chromedriver process');
+    _process.kill();
+  }
+}

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

@@ -0,0 +1,13 @@
+// Copyright 2023 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: avoid_print
+
+bool debugTestScript = false;
+
+void debugLog(String log) {
+  if (debugTestScript) {
+    print(log);
+  }
+}

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

@@ -14,8 +14,7 @@ import 'package:collection/collection.dart';
 import '_in_file_args.dart';
 import '_io_utils.dart';
 import '_test_app_driver.dart';
-
-bool _debugTestScript = false;
+import '_utils.dart';
 
 /// Runs one test.
 ///
@@ -48,16 +47,6 @@ Future<void> runFlutterIntegrationTest(
     }
   }
 
-  // TODO(kenz): do we need to start chromedriver in headless mode?
-  // Start chrome driver before running the flutter integration test.
-  final chromedriver = ChromeDriver();
-  try {
-    await chromedriver.start();
-  } catch (e) {
-    // ignore: avoid-throw-in-catch-block, by design
-    throw Exception('Error starting chromedriver: $e');
-  }
-
   // Run the flutter integration test.
   final testRunner = TestRunner();
   Exception? exception;
@@ -75,45 +64,19 @@ Future<void> runFlutterIntegrationTest(
     exception = e;
   } finally {
     if (testApp != null) {
-      _debugLog('killing the test app');
+      debugLog('killing the test app');
       await testApp.stop();
     }
 
-    _debugLog('cancelling stream subscriptions');
+    debugLog('cancelling stream subscriptions');
     await testRunner.cancelAllStreamSubscriptions();
-    await chromedriver.cancelAllStreamSubscriptions();
-
-    _debugLog('killing the chromedriver process');
-    chromedriver.kill();
   }
 
   if (exception != null) {
     throw exception;
   }
 }
 
-class ChromeDriver with IOMixin {
-  late final Process _process;
-
-  // TODO(kenz): add error messaging if the chromedriver executable is not
-  // found. We can also consider using web installers directly in this script:
-  // https://github.com/flutter/flutter/wiki/Running-Flutter-Driver-tests-with-Web#web-installers-repo.
-  Future<void> start() async {
-    _debugLog('starting the chromedriver process');
-    _process = await Process.start(
-      'chromedriver',
-      [
-        '--port=4444',
-      ],
-    );
-    listenToProcessOutput(_process, printTag: 'ChromeDriver');
-  }
-
-  void kill() {
-    _process.kill();
-  }
-}
-
 class TestRunner with IOMixin {
   static const _beginExceptionMarker = 'EXCEPTION CAUGHT';
   static const _endExceptionMarker = '═════════════════════════';
@@ -130,7 +93,7 @@ class TestRunner with IOMixin {
     Map<String, Object> testAppArguments = const <String, Object>{},
   }) async {
     Future<void> runTest({required int attemptNumber}) async {
-      _debugLog('starting the flutter drive process');
+      debugLog('starting the flutter drive process');
       final process = await Process.start(
         'flutter',
         [
@@ -210,9 +173,9 @@ class TestRunner with IOMixin {
         timeout,
       ]);
 
-      _debugLog('attempting to kill the flutter drive process');
+      debugLog('attempting to kill the flutter drive process');
       process.kill();
-      _debugLog('flutter drive process has exited');
+      debugLog('flutter drive process has exited');
 
       // Ignore exception handling and retries if the tests passed. This is to
       // avoid bugs with the test runner where the test can fail after the test
@@ -224,7 +187,7 @@ class TestRunner with IOMixin {
               'Integration test timed out on try #$attemptNumber: $testTarget',
             );
           } else {
-            _debugLog(
+            debugLog(
               'Integration test timed out on try #$attemptNumber. Retrying '
               '$testTarget now.',
             );
@@ -276,12 +239,6 @@ class _TestResult {
   }
 }
 
-void _debugLog(String log) {
-  if (_debugTestScript) {
-    print(log);
-  }
-}
-
 class TestRunnerArgs {
   TestRunnerArgs(List<String> args, {bool verifyValidTarget = true}) {
     final argParser = _buildArgParser();