[web] using pkill to quit safari (#17533)

* adding an apple script to quit safari

* making changes to utils and adding cleanup after unit tests failed

* switch saai quit to pkill instead of applescript

* adding toolexception

* adding a cleanup function option to felt.

* removing unnecassary await's added for the previos version of the PR

* removing unnecassary async

* fix analyze error

lib/web_ui/dev/chrome_installer.dart

@@ -13,6 +13,7 @@ import 'package:yaml/yaml.dart';
 
 import 'common.dart';
 import 'environment.dart';
+import 'exceptions.dart';
 
 class ChromeArgParser extends BrowserArgParser {
   static final ChromeArgParser _singletonInstance = ChromeArgParser._();

lib/web_ui/dev/common.dart

@@ -18,24 +18,6 @@ const int kMaxScreenshotWidth = 1024;
 const int kMaxScreenshotHeight = 1024;
 const double kMaxDiffRateFailure = 0.28 / 100; // 0.28%
 
-class BrowserInstallerException implements Exception {
-  BrowserInstallerException(this.message);
-
-  final String message;
-
-  @override
-  String toString() => message;
-}
-
-class DriverException implements Exception {
-  DriverException(this.message);
-
-  final String message;
-
-  @override
-  String toString() => message;
-}
-
 abstract class PlatformBinding {
   static PlatformBinding get instance {
     if (_instance == null) {
@@ -250,14 +232,3 @@ class DevNull implements StringSink {
 }
 
 bool get isCirrus => io.Platform.environment['CIRRUS_CI'] == 'true';
-
-/// There might be proccesses started during the tests.
-///
-/// Use this list to store those Processes, for cleaning up before shutdown.
-final List<io.Process> processesToCleanUp = List<io.Process>();
-
-/// There might be temporary directories created during the tests.
-///
-/// Use this list to store those directories and for deleteing them before
-/// shutdown.
-final List<io.Directory> temporaryDirectories = List<io.Directory>();

lib/web_ui/dev/environment.dart

@@ -6,6 +6,8 @@
 import 'dart:io' as io;
 import 'package:path/path.dart' as pathlib;
 
+import 'exceptions.dart';
+
 /// Contains various environment variables, such as common file paths and command-line options.
 Environment get environment {
   _environment ??= Environment();
@@ -26,8 +28,7 @@ class Environment {
 
     for (io.Directory expectedDirectory in <io.Directory>[engineSrcDir, outDir, hostDebugUnoptDir, dartSdkDir, webUiRootDir]) {
       if (!expectedDirectory.existsSync()) {
-        io.stderr.writeln('$expectedDirectory does not exist.');
-        io.exit(1);
+        throw ToolException('$expectedDirectory does not exist.');
       }
     }
 

lib/web_ui/dev/exceptions.dart

+// Copyright 2013 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.
+
+class BrowserInstallerException implements Exception {
+  BrowserInstallerException(this.message);
+
+  final String message;
+
+  @override
+  String toString() => message;
+}
+
+class DriverException implements Exception {
+  DriverException(this.message);
+
+  final String message;
+
+  @override
+  String toString() => message;
+}
+
+class ToolException implements Exception {
+  ToolException(this.message);
+
+  final String message;
+
+  @override
+  String toString() => message;
+}

lib/web_ui/dev/felt.dart

@@ -9,9 +9,10 @@ import 'package:args/command_runner.dart';
 
 import 'build.dart';
 import 'clean.dart';
-import 'common.dart';
 import 'licenses.dart';
+import 'exceptions.dart';
 import 'test_runner.dart';
+import 'utils.dart';
 
 CommandRunner runner = CommandRunner<bool>(
   'felt',
@@ -31,50 +32,46 @@ void main(List<String> args) async {
 
   _listenToShutdownSignals();
 
+  int exitCode = -1;
   try {
     final bool result = (await runner.run(args)) as bool;
     if (result == false) {
       print('Sub-command returned false: `${args.join(' ')}`');
-      _cleanup();
-      io.exit(1);
+      await cleanup();
+      exitCode = 1;
     }
   } on UsageException catch (e) {
     print(e);
-    io.exit(64); // Exit code 64 indicates a usage error.
+    exitCode = 64; // Exit code 64 indicates a usage error.
+  } on ToolException catch (e) {
+    io.stderr.writeln(e.message);
+    exitCode = 1;
   } catch (e) {
     rethrow;
   } finally {
-    _cleanup();
+    await cleanup();
+    // The exit code is changed by one of the branches.
+    if(exitCode != -1) {
+      io.exit(exitCode);
+    }
   }
 
   // Sometimes the Dart VM refuses to quit.
   io.exit(io.exitCode);
 }
 
-void _cleanup() {
-  // Cleanup remaining processes if any.
-  if (processesToCleanUp.length > 0) {
-    for (io.Process process in processesToCleanUp) {
-      process.kill();
-    }
-  }
-  // Delete temporary directories.
-  if (temporaryDirectories.length > 0) {
-    for (io.Directory directory in temporaryDirectories) {
-      directory.deleteSync(recursive: true);
-    }
-  }
-}
-
-void _listenToShutdownSignals() {
-  io.ProcessSignal.sigint.watch().listen((_) {
+void _listenToShutdownSignals() async {
+  io.ProcessSignal.sigint.watch().listen((_) async {
     print('Received SIGINT. Shutting down.');
+    await cleanup();
     io.exit(1);
   });
+
   // SIGTERM signals are not generated under Windows.
   // See https://docs.microsoft.com/en-us/previous-versions/xdkz3x12(v%3Dvs.140)
   if (!io.Platform.isWindows) {
-    io.ProcessSignal.sigterm.watch().listen((_) {
+    io.ProcessSignal.sigterm.watch().listen((_) async {
+      await cleanup();
       print('Received SIGTERM. Shutting down.');
       io.exit(1);
     });

lib/web_ui/dev/firefox_installer.dart

@@ -12,6 +12,7 @@ import 'package:yaml/yaml.dart';
 
 import 'common.dart';
 import 'environment.dart';
+import 'exceptions.dart';
 
 class FirefoxArgParser extends BrowserArgParser {
   static final FirefoxArgParser _singletonInstance = FirefoxArgParser._();

lib/web_ui/dev/test_runner.dart

@@ -15,6 +15,7 @@ import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_im
 import 'package:test_core/src/executable.dart'
     as test; // ignore: implementation_imports
 
+import 'exceptions.dart';
 import 'integration_tests_manager.dart';
 import 'supported_browsers.dart';
 import 'test_platform.dart';
@@ -151,6 +152,26 @@ class TestCommand extends Command<bool> with ArgUtils {
     }
 
     await _buildTests(targets: targetFiles);
+
+    // Many tabs will be left open after Safari runs, quit Safari during
+    // cleanup.
+    if (browser == 'safari') {
+      cleanupCallbacks.add(() async {
+        // Only close Safari if felt is running in CI environments. Do not close
+        // Safari for the local testing.
+        if (io.Platform.environment['LUCI_CONTEXT'] != null || isCirrus) {
+          print('INFO: Safari tests ran. Quit Safari.');
+          await runProcess(
+            'sudo',
+            ['pkill', '-lf', 'Safari'],
+            workingDirectory: environment.webUiRootDir.path,
+          );
+        } else {
+          print('INFO: Safari tests ran. Please quit Safari tabs.');
+        }
+      });
+    }
+
     if (runAllTests) {
       await _runAllTests();
     } else {
@@ -179,7 +200,7 @@ class TestCommand extends Command<bool> with ArgUtils {
   bool get runAllTests => targets.isEmpty;
 
   /// The name of the browser to run tests in.
-  String get browser => stringArg('browser');
+  String get browser => (argResults != null) ? stringArg('browser') : 'chrome';
 
   /// Whether [browser] is set to "chrome".
   bool get isChrome => browser == 'chrome';
@@ -275,8 +296,7 @@ class TestCommand extends Command<bool> with ArgUtils {
 
   void _checkExitCode() {
     if (io.exitCode != 0) {
-      io.stderr.writeln('Process exited with exit code ${io.exitCode}.');
-      io.exit(1);
+      throw ToolException('Process exited with exit code ${io.exitCode}.');
     }
   }
 
@@ -290,9 +310,8 @@ class TestCommand extends Command<bool> with ArgUtils {
     );
 
     if (exitCode != 0) {
-      io.stderr
-          .writeln('Failed to run pub get. Exited with exit code $exitCode');
-      io.exit(1);
+      throw ToolException(
+          'Failed to run pub get. Exited with exit code $exitCode');
     }
   }
 
@@ -333,9 +352,8 @@ class TestCommand extends Command<bool> with ArgUtils {
     );
 
     if (exitCode != 0) {
-      io.stderr.writeln(
-          'Failed to compile ${hostDartFile.path}. Compiler exited with exit code $exitCode');
-      io.exit(1);
+      throw ToolException('Failed to compile ${hostDartFile.path}. Compiler '
+          'exited with exit code $exitCode');
     }
 
     // Record the timestamp to avoid rebuilding unless the file changes.
@@ -363,9 +381,8 @@ class TestCommand extends Command<bool> with ArgUtils {
     );
 
     if (exitCode != 0) {
-      io.stderr.writeln(
+      throw ToolException(
           'Failed to compile tests. Compiler exited with exit code $exitCode');
-      io.exit(1);
     }
   }
 

lib/web_ui/dev/utils.dart

@@ -10,7 +10,6 @@ import 'package:args/command_runner.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 
-import 'common.dart';
 import 'environment.dart';
 
 class FilePath {
@@ -130,7 +129,8 @@ class ProcessException implements Exception {
     message
       ..writeln(description)
       ..writeln('Command: $executable ${arguments.join(' ')}')
-      ..writeln('Working directory: ${workingDirectory ?? io.Directory.current.path}')
+      ..writeln(
+          'Working directory: ${workingDirectory ?? io.Directory.current.path}')
       ..writeln('Exit code: $exitCode');
     return '$message';
   }
@@ -161,3 +161,42 @@ mixin ArgUtils<T> on Command<T> {
     return value;
   }
 }
+
+/// There might be proccesses started during the tests.
+///
+/// Use this list to store those Processes, for cleaning up before shutdown.
+final List<io.Process> processesToCleanUp = List<io.Process>();
+
+/// There might be temporary directories created during the tests.
+///
+/// Use this list to store those directories and for deleteing them before
+/// shutdown.
+final List<io.Directory> temporaryDirectories = List<io.Directory>();
+
+typedef AsyncCallback = Future<void> Function();
+
+/// There might be additional cleanup needs to be done after the tools ran.
+///
+/// Add these operations here to make sure that they will run before felt
+/// exit.
+final List<AsyncCallback> cleanupCallbacks = List<AsyncCallback>();
+
+/// Cleanup the remaning processes, close open browsers, delete temp files.
+void cleanup() async {
+  // Cleanup remaining processes if any.
+  if (processesToCleanUp.length > 0) {
+    for (io.Process process in processesToCleanUp) {
+      process.kill();
+    }
+  }
+  // Delete temporary directories.
+  if (temporaryDirectories.length > 0) {
+    for (io.Directory directory in temporaryDirectories) {
+      directory.deleteSync(recursive: true);
+    }
+  }
+
+  cleanupCallbacks.forEach((element) {
+    element.call();
+  });
+}