From 95393efe1307a10df9240c50e5c9e72cd4ec94d4 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 6 Apr 2020 12:07:08 -0700 Subject: [PATCH 1/8] adding an apple script to quit safari --- lib/web_ui/dev/felt.dart | 16 +++++++++++++++- lib/web_ui/dev/quit_safari.scpt | Bin 0 -> 760 bytes 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 lib/web_ui/dev/quit_safari.scpt diff --git a/lib/web_ui/dev/felt.dart b/lib/web_ui/dev/felt.dart index 3ba2984777d65..6aef790afec34 100644 --- a/lib/web_ui/dev/felt.dart +++ b/lib/web_ui/dev/felt.dart @@ -10,8 +10,12 @@ import 'package:args/command_runner.dart'; import 'build.dart'; import 'clean.dart'; import 'common.dart'; +import 'environment.dart'; import 'licenses.dart'; import 'test_runner.dart'; +import 'utils.dart'; + +final TestCommand testCommand = TestCommand(); CommandRunner runner = CommandRunner( 'felt', @@ -19,7 +23,7 @@ CommandRunner runner = CommandRunner( ) ..addCommand(CleanCommand()) ..addCommand(LicensesCommand()) - ..addCommand(TestCommand()) + ..addCommand(testCommand) ..addCommand(BuildCommand()); void main(List args) async { @@ -64,6 +68,16 @@ void _cleanup() { directory.deleteSync(recursive: true); } } + + // Many tabs left open after Safari runs, quit safari. + if(testCommand.browser == 'safari') { + print('INFO: Safari tests run. Quit Safari.'); + startProcess( + 'osascript', + ['dev/quit_safari.scpt'], + workingDirectory: environment.webUiRootDir.path, + ); + } } void _listenToShutdownSignals() { diff --git a/lib/web_ui/dev/quit_safari.scpt b/lib/web_ui/dev/quit_safari.scpt new file mode 100644 index 0000000000000000000000000000000000000000..350c001ec2fd26439cd0958746a9d9db2ed316a6 GIT binary patch literal 760 zcmb_a%}&BV5dOBU7M7NxCTchlj~+m;+%UxG!FV7%z@|#5R;aXi<`H}mUy1k({HdUf zvtR-Tq8Hoj?0nPdH^1j9zNuGgh125ci9T5XmPZ5u1EydFF6(B<5RkBFPG2r87}t!o zWeeLb3-gjaETrj^W;B)biNm70eM%m?LDK4mAyE(dXkE|@Flef|w;>)DOaitCTl7*3 z^sgdrbOIOxDhy&xERHx=;an*{XeI5qS-7Zfh<8>6*H0vCs-ZeTQAN=PufI*2W8G_s zWx?;RTj5lqC=7g+1npML`Q=OHbxV1bMG0B#bLVSNXz=MEK#}i=Uk`aWxMJk9wS@$2 z#Au>`3skX-gLTf*iWAs_DDi~2J2cng3mk@r?&V;@qz|N!7JMPcnxJ<{?>{4Mb7%Be zfgV@%jAMSXNW;$28zfC=j0_waNrua%*z||X6^Z*rOC(%_SA%CnLxYAon~7eSu@0Wm k!r_lL4I~YIS+gA)A@jNm`}od*sCz2*JG{%zwBLXE2FM|(-v9sr literal 0 HcmV?d00001 From f399a6ac98fb04b73376bdfeb8fb61ac3d70ea3b Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 6 Apr 2020 12:52:25 -0700 Subject: [PATCH 2/8] making changes to utils and adding cleanup after unit tests failed --- lib/web_ui/dev/felt.dart | 31 ++----------------------------- lib/web_ui/dev/test_runner.dart | 15 ++++++++------- lib/web_ui/dev/utils.dart | 29 ++++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/lib/web_ui/dev/felt.dart b/lib/web_ui/dev/felt.dart index 6aef790afec34..d50729836188d 100644 --- a/lib/web_ui/dev/felt.dart +++ b/lib/web_ui/dev/felt.dart @@ -9,8 +9,6 @@ import 'package:args/command_runner.dart'; import 'build.dart'; import 'clean.dart'; -import 'common.dart'; -import 'environment.dart'; import 'licenses.dart'; import 'test_runner.dart'; import 'utils.dart'; @@ -39,7 +37,7 @@ void main(List args) async { final bool result = (await runner.run(args)) as bool; if (result == false) { print('Sub-command returned false: `${args.join(' ')}`'); - _cleanup(); + await cleanup(); io.exit(1); } } on UsageException catch (e) { @@ -48,38 +46,13 @@ void main(List args) async { } catch (e) { rethrow; } finally { - _cleanup(); + await cleanup(browser: testCommand.browser); } // 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); - } - } - - // Many tabs left open after Safari runs, quit safari. - if(testCommand.browser == 'safari') { - print('INFO: Safari tests run. Quit Safari.'); - startProcess( - 'osascript', - ['dev/quit_safari.scpt'], - workingDirectory: environment.webUiRootDir.path, - ); - } -} - void _listenToShutdownSignals() { io.ProcessSignal.sigint.watch().listen((_) { print('Received SIGINT. Shutting down.'); diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 5f5995c3d6465..73c6dd290187d 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -179,7 +179,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'; @@ -190,7 +190,7 @@ class TestCommand extends Command with ArgUtils { Future _runTargetTests(List targets) async { await _runTestBatch(targets, concurrency: 1, expectFailure: false); - _checkExitCode(); + await _checkExitCode(); } Future _runAllTests() async { @@ -236,12 +236,12 @@ class TestCommand extends Command with ArgUtils { concurrency: 1, expectFailure: true, ); - _checkExitCode(); + await _checkExitCode(); } // Run all unit-tests as a single batch. await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false); - _checkExitCode(); + await _checkExitCode(); // Run screenshot tests one at a time. for (FilePath testFilePath in screenshotTestFiles) { @@ -250,7 +250,7 @@ class TestCommand extends Command with ArgUtils { concurrency: 1, expectFailure: false, ); - _checkExitCode(); + await _checkExitCode(); } } else { final List unitTestFiles = []; @@ -269,13 +269,14 @@ class TestCommand extends Command with ArgUtils { } // Run all unit-tests as a single batch. await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false); - _checkExitCode(); + await _checkExitCode(); } } - void _checkExitCode() { + void _checkExitCode() async { if (io.exitCode != 0) { io.stderr.writeln('Process exited with exit code ${io.exitCode}.'); + await cleanup(browser: browser); io.exit(1); } } diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index 4cf53b6651a62..17685a653be1f 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -130,7 +130,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 +162,29 @@ mixin ArgUtils on Command { return value; } } + +/// Cleanup the remaning processes, close open browsers, delete temp files. +void cleanup({String browser = 'chrome'}) 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); + } + } + + // Many tabs left open after Safari runs, quit safari. + if (browser == 'safari') { + print('INFO: Safari tests run. Quit Safari.'); + await runProcess( + 'osascript', + ['dev/quit_safari.scpt'], + workingDirectory: environment.webUiRootDir.path, + ); + } +} From 648f6dfd3bbaf9bd589b816f67d7f777cb253a91 Mon Sep 17 00:00:00 2001 From: nturgut Date: Tue, 7 Apr 2020 16:14:07 -0700 Subject: [PATCH 3/8] switch saai quit to pkill instead of applescript --- lib/web_ui/dev/quit_safari.scpt | Bin 760 -> 0 bytes lib/web_ui/dev/utils.dart | 18 ++++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) delete mode 100644 lib/web_ui/dev/quit_safari.scpt diff --git a/lib/web_ui/dev/quit_safari.scpt b/lib/web_ui/dev/quit_safari.scpt deleted file mode 100644 index 350c001ec2fd26439cd0958746a9d9db2ed316a6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 760 zcmb_a%}&BV5dOBU7M7NxCTchlj~+m;+%UxG!FV7%z@|#5R;aXi<`H}mUy1k({HdUf zvtR-Tq8Hoj?0nPdH^1j9zNuGgh125ci9T5XmPZ5u1EydFF6(B<5RkBFPG2r87}t!o zWeeLb3-gjaETrj^W;B)biNm70eM%m?LDK4mAyE(dXkE|@Flef|w;>)DOaitCTl7*3 z^sgdrbOIOxDhy&xERHx=;an*{XeI5qS-7Zfh<8>6*H0vCs-ZeTQAN=PufI*2W8G_s zWx?;RTj5lqC=7g+1npML`Q=OHbxV1bMG0B#bLVSNXz=MEK#}i=Uk`aWxMJk9wS@$2 z#Au>`3skX-gLTf*iWAs_DDi~2J2cng3mk@r?&V;@qz|N!7JMPcnxJ<{?>{4Mb7%Be zfgV@%jAMSXNW;$28zfC=j0_waNrua%*z||X6^Z*rOC(%_SA%CnLxYAon~7eSu@0Wm k!r_lL4I~YIS+gA)A@jNm`}od*sCz2*JG{%zwBLXE2FM|(-v9sr diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index 17685a653be1f..d1868919d68f3 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -180,11 +180,17 @@ void cleanup({String browser = 'chrome'}) async { // Many tabs left open after Safari runs, quit safari. if (browser == 'safari') { - print('INFO: Safari tests run. Quit Safari.'); - await runProcess( - 'osascript', - ['dev/quit_safari.scpt'], - workingDirectory: environment.webUiRootDir.path, - ); + // 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 run. Quit Safari.'); + await runProcess( + 'sudo', + ['pkill', '-lf', 'Safari'], + workingDirectory: environment.webUiRootDir.path, + ); + } else { + print('INFO: Safari tests run. Please quit Safari tabs.'); + } } } From ca49173161b4c8c55cc35846e2a57b579c33ac18 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 8 Apr 2020 12:39:33 -0700 Subject: [PATCH 4/8] adding toolexception --- lib/web_ui/dev/chrome_installer.dart | 1 + lib/web_ui/dev/common.dart | 18 ---------------- lib/web_ui/dev/environment.dart | 5 +++-- lib/web_ui/dev/exceptions.dart | 30 +++++++++++++++++++++++++++ lib/web_ui/dev/felt.dart | 22 +++++++++++++++----- lib/web_ui/dev/firefox_installer.dart | 1 + lib/web_ui/dev/test_runner.dart | 18 +++++++--------- lib/web_ui/dev/utils.dart | 4 ++-- 8 files changed, 61 insertions(+), 38 deletions(-) create mode 100644 lib/web_ui/dev/exceptions.dart diff --git a/lib/web_ui/dev/chrome_installer.dart b/lib/web_ui/dev/chrome_installer.dart index b6345ea379858..48e10def23fa8 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 3939bf2e9b6cd..6362bb1113892 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) { diff --git a/lib/web_ui/dev/environment.dart b/lib/web_ui/dev/environment.dart index 6af6e6e874271..3214c8a083b3d 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 0000000000000..167d1734e655f --- /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 d50729836188d..4f3957a43b52a 100644 --- a/lib/web_ui/dev/felt.dart +++ b/lib/web_ui/dev/felt.dart @@ -10,6 +10,7 @@ import 'package:args/command_runner.dart'; import 'build.dart'; import 'clean.dart'; import 'licenses.dart'; +import 'exceptions.dart'; import 'test_runner.dart'; import 'utils.dart'; @@ -33,35 +34,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(' ')}`'); await cleanup(); - io.exit(1); + 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 { await cleanup(browser: testCommand.browser); + // 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 _listenToShutdownSignals() { - io.ProcessSignal.sigint.watch().listen((_) { +void _listenToShutdownSignals() async { + io.ProcessSignal.sigint.watch().listen((_) async { print('Received SIGINT. Shutting down.'); + await cleanup(browser: testCommand.browser); 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(browser: testCommand.browser); 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 5a5c951c719dd..e282d2a3df23f 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 73c6dd290187d..052856551f036 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'; @@ -275,9 +276,7 @@ class TestCommand extends Command with ArgUtils { void _checkExitCode() async { if (io.exitCode != 0) { - io.stderr.writeln('Process exited with exit code ${io.exitCode}.'); - await cleanup(browser: browser); - io.exit(1); + throw ToolException('Process exited with exit code ${io.exitCode}.'); } } @@ -291,9 +290,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'); } } @@ -334,9 +332,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. @@ -364,9 +361,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 d1868919d68f3..b91c88259b195 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -183,14 +183,14 @@ void cleanup({String browser = 'chrome'}) 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 run. Quit Safari.'); + print('INFO: Safari tests ran. Quit Safari.'); await runProcess( 'sudo', ['pkill', '-lf', 'Safari'], workingDirectory: environment.webUiRootDir.path, ); } else { - print('INFO: Safari tests run. Please quit Safari tabs.'); + print('INFO: Safari tests ran. Please quit Safari tabs.'); } } } From dde43581dcdc185bb23fe31a1ea35f9f4c0ae287 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 8 Apr 2020 14:46:16 -0700 Subject: [PATCH 5/8] adding a cleanup function option to felt. --- lib/web_ui/dev/common.dart | 11 ---------- lib/web_ui/dev/felt.dart | 10 ++++----- lib/web_ui/dev/test_runner.dart | 20 +++++++++++++++++ lib/web_ui/dev/utils.dart | 39 +++++++++++++++++++-------------- 4 files changed, 47 insertions(+), 33 deletions(-) diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart index 6362bb1113892..344316be76511 100644 --- a/lib/web_ui/dev/common.dart +++ b/lib/web_ui/dev/common.dart @@ -232,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/felt.dart b/lib/web_ui/dev/felt.dart index 4f3957a43b52a..32f3619dc7014 100644 --- a/lib/web_ui/dev/felt.dart +++ b/lib/web_ui/dev/felt.dart @@ -14,15 +14,13 @@ import 'exceptions.dart'; import 'test_runner.dart'; import 'utils.dart'; -final TestCommand testCommand = TestCommand(); - CommandRunner runner = CommandRunner( 'felt', 'Command-line utility for building and testing Flutter web engine.', ) ..addCommand(CleanCommand()) ..addCommand(LicensesCommand()) - ..addCommand(testCommand) + ..addCommand(TestCommand()) ..addCommand(BuildCommand()); void main(List args) async { @@ -51,7 +49,7 @@ void main(List args) async { } catch (e) { rethrow; } finally { - await cleanup(browser: testCommand.browser); + await cleanup(); // The exit code is changed by one of the branches. if(exitCode != -1) { io.exit(exitCode); @@ -65,7 +63,7 @@ void main(List args) async { void _listenToShutdownSignals() async { io.ProcessSignal.sigint.watch().listen((_) async { print('Received SIGINT. Shutting down.'); - await cleanup(browser: testCommand.browser); + await cleanup(); io.exit(1); }); @@ -73,7 +71,7 @@ void _listenToShutdownSignals() async { // See https://docs.microsoft.com/en-us/previous-versions/xdkz3x12(v%3Dvs.140) if (!io.Platform.isWindows) { io.ProcessSignal.sigterm.watch().listen((_) async { - await cleanup(browser: testCommand.browser); + await cleanup(); print('Received SIGTERM. Shutting down.'); io.exit(1); }); diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 052856551f036..38756d926591c 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -152,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 { diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index b91c88259b195..5fa1164011050 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -163,8 +163,27 @@ mixin ArgUtils on Command { } } +/// 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({String browser = 'chrome'}) async { +void cleanup() async { // Cleanup remaining processes if any. if (processesToCleanUp.length > 0) { for (io.Process process in processesToCleanUp) { @@ -178,19 +197,7 @@ void cleanup({String browser = 'chrome'}) async { } } - // Many tabs left open after Safari runs, quit safari. - if (browser == 'safari') { - // 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.'); - } - } + cleanupCallbacks.forEach((element) { + element.call(); + }); } From 98d064ff7ec0d61943cc8413c4288d61354f19af Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 8 Apr 2020 14:57:02 -0700 Subject: [PATCH 6/8] removing unnecassary await's added for the previos version of the PR --- lib/web_ui/dev/test_runner.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 38756d926591c..22e7a2c742c8a 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -211,7 +211,7 @@ class TestCommand extends Command with ArgUtils { Future _runTargetTests(List targets) async { await _runTestBatch(targets, concurrency: 1, expectFailure: false); - await _checkExitCode(); + _checkExitCode(); } Future _runAllTests() async { @@ -257,12 +257,12 @@ class TestCommand extends Command with ArgUtils { concurrency: 1, expectFailure: true, ); - await _checkExitCode(); + _checkExitCode(); } // Run all unit-tests as a single batch. await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false); - await _checkExitCode(); + _checkExitCode(); // Run screenshot tests one at a time. for (FilePath testFilePath in screenshotTestFiles) { @@ -271,7 +271,7 @@ class TestCommand extends Command with ArgUtils { concurrency: 1, expectFailure: false, ); - await _checkExitCode(); + _checkExitCode(); } } else { final List unitTestFiles = []; @@ -290,7 +290,7 @@ class TestCommand extends Command with ArgUtils { } // Run all unit-tests as a single batch. await _runTestBatch(unitTestFiles, concurrency: 10, expectFailure: false); - await _checkExitCode(); + _checkExitCode(); } } From 9021001b482671007fd3085d691397fcdd7da765 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 8 Apr 2020 14:58:16 -0700 Subject: [PATCH 7/8] removing unnecassary async --- lib/web_ui/dev/test_runner.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 22e7a2c742c8a..5b62a064d7b8d 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -294,7 +294,7 @@ class TestCommand extends Command with ArgUtils { } } - void _checkExitCode() async { + void _checkExitCode() { if (io.exitCode != 0) { throw ToolException('Process exited with exit code ${io.exitCode}.'); } From f0f679faaad083f7eaa893a0f0ef9c94a766aa77 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 8 Apr 2020 15:43:44 -0700 Subject: [PATCH 8/8] fix analyze error --- lib/web_ui/dev/utils.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index 5fa1164011050..bf3475e8874fc 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 {