diff --git a/lib/web_ui/dev/chrome_installer.dart b/lib/web_ui/dev/chrome_installer.dart index b6345ea3798586f1b7e8ba896531d4ed40120b07..48e10def23fa86e42ffd1744e99c5543c7fcdf57 100644 --- a/lib/web_ui/dev/chrome_installer.dart +++ b/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._(); diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart index 3939bf2e9b6cde3d7806a21c0d1716a7c0d0cb38..344316be76511a503a7fdc6c4a7765aee5e764ba 100644 --- a/lib/web_ui/dev/common.dart +++ b/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 processesToCleanUp = List(); - -/// There might be temporary directories created during the tests. -/// -/// Use this list to store those directories and for deleteing them before -/// shutdown. -final List temporaryDirectories = List(); diff --git a/lib/web_ui/dev/environment.dart b/lib/web_ui/dev/environment.dart index 6af6e6e874271c311279fa1f89fb0ea82a0f4a37..3214c8a083b3d340d9e7c50c52bc43d8fb0bbe43 100644 --- a/lib/web_ui/dev/environment.dart +++ b/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 [engineSrcDir, outDir, hostDebugUnoptDir, dartSdkDir, webUiRootDir]) { if (!expectedDirectory.existsSync()) { - io.stderr.writeln('$expectedDirectory does not exist.'); - io.exit(1); + throw ToolException('$expectedDirectory does not exist.'); } } diff --git a/lib/web_ui/dev/exceptions.dart b/lib/web_ui/dev/exceptions.dart new file mode 100644 index 0000000000000000000000000000000000000000..167d1734e655f52b0731732a6ed898aab92bb0b7 --- /dev/null +++ b/lib/web_ui/dev/exceptions.dart @@ -0,0 +1,30 @@ +// 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; +} diff --git a/lib/web_ui/dev/felt.dart b/lib/web_ui/dev/felt.dart index 3ba2984777d65d76e42815828caa14a2b687ad11..32f3619dc701420304866343a5247fee5699981c 100644 --- a/lib/web_ui/dev/felt.dart +++ b/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( 'felt', @@ -31,50 +32,46 @@ void main(List 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); }); diff --git a/lib/web_ui/dev/firefox_installer.dart b/lib/web_ui/dev/firefox_installer.dart index 5a5c951c719dd1a2e960a9d5eb42258f7f634f58..e282d2a3df23fe2fe509b6fcd6e51455e24560c5 100644 --- a/lib/web_ui/dev/firefox_installer.dart +++ b/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._(); diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 5f5995c3d6465d35bc5ea65370ef16b40083a4d7..5b62a064d7b8d2c0ff8bccc9b8aad7877d077fa1 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/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 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 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 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 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 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 with ArgUtils { ); if (exitCode != 0) { - io.stderr.writeln( + throw ToolException( 'Failed to compile tests. Compiler exited with exit code $exitCode'); - io.exit(1); } } diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index 4cf53b6651a626a1099a822f405bc169fb691dfa..bf3475e8874fc1ddcfdfb403a87691e2533290a0 100644 --- a/lib/web_ui/dev/utils.dart +++ b/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 on Command { return value; } } + +/// There might be proccesses started during the tests. +/// +/// Use this list to store those Processes, for cleaning up before shutdown. +final List processesToCleanUp = List(); + +/// There might be temporary directories created during the tests. +/// +/// Use this list to store those directories and for deleteing them before +/// shutdown. +final List temporaryDirectories = List(); + +typedef AsyncCallback = Future 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 cleanupCallbacks = List(); + +/// 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(); + }); +}