From 8d159d2b2faab5c1e5b1033a7210da128f81dd66 Mon Sep 17 00:00:00 2001 From: Nurhan Turgut Date: Wed, 18 Dec 2019 10:21:34 -0800 Subject: [PATCH 1/4] Launch safari browser --- lib/web_ui/dev/common.dart | 9 +++ lib/web_ui/dev/firefox_installer.dart | 2 +- lib/web_ui/dev/safari.dart | 64 ++++++++++++++++++++ lib/web_ui/dev/safari_installation.dart | 78 +++++++++++++++++++++++++ lib/web_ui/dev/safari_launch_test.dart | 25 ++++++++ 5 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 lib/web_ui/dev/safari.dart create mode 100644 lib/web_ui/dev/safari_installation.dart create mode 100644 lib/web_ui/dev/safari_launch_test.dart diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart index 57102e8411519..49e82a96706a0 100644 --- a/lib/web_ui/dev/common.dart +++ b/lib/web_ui/dev/common.dart @@ -48,6 +48,7 @@ abstract class PlatformBinding { String getChromeExecutablePath(io.Directory versionDir); String getFirefoxExecutablePath(io.Directory versionDir); String getFirefoxLatestVersionUrl(); + String getSafariSystemExecutablePath(); } const String _kBaseDownloadUrl = @@ -79,6 +80,10 @@ class _LinuxBinding implements PlatformBinding { @override String getFirefoxLatestVersionUrl() => 'https://download.mozilla.org/?product=firefox-latest&os=linux64&lang=en-US'; + + @override + String getSafariSystemExecutablePath() => + throw UnsupportedError('Safari is not supported on Linux'); } class _MacBinding implements PlatformBinding { @@ -112,6 +117,10 @@ class _MacBinding implements PlatformBinding { @override String getFirefoxLatestVersionUrl() => 'https://download.mozilla.org/?product=firefox-latest&os=osx&lang=en-US'; + + @override + String getSafariSystemExecutablePath() => + '/Applications/Safari.app/Contents/MacOS/Safari'; } class BrowserInstallation { diff --git a/lib/web_ui/dev/firefox_installer.dart b/lib/web_ui/dev/firefox_installer.dart index d9bd9b6989277..1554f7b584eea 100644 --- a/lib/web_ui/dev/firefox_installer.dart +++ b/lib/web_ui/dev/firefox_installer.dart @@ -15,7 +15,7 @@ import 'environment.dart'; class FirefoxArgParser extends BrowserArgParser { static final FirefoxArgParser _singletonInstance = FirefoxArgParser._(); - /// The [ChromeArgParser] singleton. + /// The [FirefoxArgParser] singleton. static FirefoxArgParser get instance => _singletonInstance; String _version; diff --git a/lib/web_ui/dev/safari.dart b/lib/web_ui/dev/safari.dart new file mode 100644 index 0000000000000..de80242e55187 --- /dev/null +++ b/lib/web_ui/dev/safari.dart @@ -0,0 +1,64 @@ +// 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. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; +import 'package:pedantic/pedantic.dart'; + +import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports + +import 'browser.dart'; +import 'safari_installation.dart'; +import 'common.dart'; + +/// A class for running an instance of Safari. +/// +/// Most of the communication with the browser is expected to happen via HTTP, +/// so this exposes a bare-bones API. The browser starts as soon as the class is +/// constructed, and is killed when [close] is called. +/// +/// Any errors starting or running the process are reported through [onExit]. +class Safari extends Browser { + @override + final name = 'Safari'; + + static String version; + + /// Starts a new instance of Safari open to the given [url], which may be a + /// [Uri] or a [String]. + factory Safari(Uri url, {bool debug = false}) { + version = SafariArgParser.instance.version; + + assert(version != null); + return Safari._(() async { + // TODO(nurhan): Configure info log for LUCI. + final BrowserInstallation installation = await getOrInstallSafari( + version, + infoLog: DevNull(), + ); + + var dir = createTempDir(); + + // Safari will only open files (not general URLs) via the command-line + // API, so we create a dummy file to redirect it to the page we actually + // want it to load. + var redirect = path.join(dir, 'redirect.html'); + File(redirect).writeAsStringSync( + ''); + + var process = await Process.start( + installation.executable, [redirect] /* args */); + + unawaited(process.exitCode + .then((_) => Directory(dir).deleteSync(recursive: true))); + + return process; + }); + } + + Safari._(Future startBrowser()) : super(startBrowser); +} diff --git a/lib/web_ui/dev/safari_installation.dart b/lib/web_ui/dev/safari_installation.dart new file mode 100644 index 0000000000000..3df7ec6798dc2 --- /dev/null +++ b/lib/web_ui/dev/safari_installation.dart @@ -0,0 +1,78 @@ +// 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. + +import 'dart:async'; +import 'dart:io' as io; + +import 'package:args/args.dart'; + +import 'common.dart'; + +class SafariArgParser extends BrowserArgParser { + static final SafariArgParser _singletonInstance = SafariArgParser._(); + + /// The [SafariArgParser] singleton. + static SafariArgParser get instance => _singletonInstance; + + String _version; + + SafariArgParser._(); + + @override + void populateOptions(ArgParser argParser) { + argParser + ..addOption( + 'safari-version', + defaultsTo: 'system', + help: 'The Safari version to use while running tests. The Safari ' + 'browswer installed on the system is used as the only option now.' + 'Soon we will add support for using different versions using the ' + 'tech previews.', + ); + } + + @override + void parseOptions(ArgResults argResults) { + _version = argResults['safari-version']; + assert(_version == 'system'); + } + + @override + String get version => _version; +} + +/// Returns the installation of Safari. +/// +/// Currently uses the Safari version installed on the operating system. +/// +/// Latest Safari version for Catalina, Mojave, High Siera is 13. +/// +/// Latest Safari version for Sierra is 12. +// TODO(nurhan): user latest version to download and install the latest +// technology preview. +Future getOrInstallSafari( + String requestedVersion, { + StringSink infoLog, +}) async { + + // These tests are aimed to run only on MacOs machines local or on LUCI. + if (!io.Platform.isMacOS) { + throw UnimplementedError(); + } + + infoLog ??= io.stdout; + + if (requestedVersion == 'system') { + // Since Safari is included in MacOS, always assume there will be one on the + // system. + infoLog.writeln('Using the system version is already installed.'); + return BrowserInstallation( + version: 'system', + executable: PlatformBinding.instance.getSafariSystemExecutablePath(), + ); + } else { + infoLog.writeln('Unsupported version $requestedVersion.'); + throw UnimplementedError(); + } +} diff --git a/lib/web_ui/dev/safari_launch_test.dart b/lib/web_ui/dev/safari_launch_test.dart new file mode 100644 index 0000000000000..32032abaf9903 --- /dev/null +++ b/lib/web_ui/dev/safari_launch_test.dart @@ -0,0 +1,25 @@ +// 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. + +import 'dart:async'; +import 'package:test/test.dart'; + +import 'browser.dart'; +import 'safari.dart'; +import 'test_platform.dart'; + +void main() async { + test('Start Safari Browser', () async { + Browser browser = Safari(Uri.parse('https://www.google.com'), debug: false); + + var completer = Completer(); + + browser.onExit.then((_) { + throw Exception('Safari exited before connecting.'); + }).catchError((error, StackTrace stackTrace) { + if (completer.isCompleted) return; + completer.completeError(error, stackTrace); + }); + }); +} From 49e8729ff9087c59690f5d6d8ddf5868f77bd9c8 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 18 Dec 2019 13:24:41 -0800 Subject: [PATCH 2/4] Adding safari tests to supported browsers --- lib/web_ui/dart_test_safari.yaml | 7 +++++++ lib/web_ui/dev/safari_launch_test.dart | 25 ------------------------- lib/web_ui/dev/supported_browsers.dart | 14 ++++++++++---- 3 files changed, 17 insertions(+), 29 deletions(-) create mode 100644 lib/web_ui/dart_test_safari.yaml delete mode 100644 lib/web_ui/dev/safari_launch_test.dart diff --git a/lib/web_ui/dart_test_safari.yaml b/lib/web_ui/dart_test_safari.yaml new file mode 100644 index 0000000000000..54f2064859b9e --- /dev/null +++ b/lib/web_ui/dart_test_safari.yaml @@ -0,0 +1,7 @@ +# For more information on test and runner configurations: +# +# * https://github.com/dart-lang/test/blob/master/pkgs/test/doc/configuration.md#platforms + +platforms: + - safari + - vm diff --git a/lib/web_ui/dev/safari_launch_test.dart b/lib/web_ui/dev/safari_launch_test.dart deleted file mode 100644 index 32032abaf9903..0000000000000 --- a/lib/web_ui/dev/safari_launch_test.dart +++ /dev/null @@ -1,25 +0,0 @@ -// 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. - -import 'dart:async'; -import 'package:test/test.dart'; - -import 'browser.dart'; -import 'safari.dart'; -import 'test_platform.dart'; - -void main() async { - test('Start Safari Browser', () async { - Browser browser = Safari(Uri.parse('https://www.google.com'), debug: false); - - var completer = Completer(); - - browser.onExit.then((_) { - throw Exception('Safari exited before connecting.'); - }).catchError((error, StackTrace stackTrace) { - if (completer.isCompleted) return; - completer.completeError(error, stackTrace); - }); - }); -} diff --git a/lib/web_ui/dev/supported_browsers.dart b/lib/web_ui/dev/supported_browsers.dart index 3d16f1a62013a..5581e1ba82eb2 100644 --- a/lib/web_ui/dev/supported_browsers.dart +++ b/lib/web_ui/dev/supported_browsers.dart @@ -10,7 +10,9 @@ import 'chrome_installer.dart'; import 'common.dart'; import 'environment.dart'; import 'firefox.dart'; -import 'firefox_installer.dart'; // ignore: implementation_imports +import 'firefox_installer.dart'; +import 'safari.dart'; +import 'safari_installation.dart'; /// Utilities for browsers, that tests are supported. /// @@ -22,18 +24,20 @@ import 'firefox_installer.dart'; // ignore: implementation_imports /// One should also implement [BrowserArgParser] and add it to the [argParsers]. class SupportedBrowsers { final List argParsers = - List.of([ChromeArgParser.instance, FirefoxArgParser.instance]); + List.of([ChromeArgParser.instance, FirefoxArgParser.instance, SafariArgParser.instance]); - final List supportedBrowserNames = ['chrome', 'firefox']; + final List supportedBrowserNames = ['chrome', 'firefox', 'safari']; final Map supportedBrowsersToRuntimes = { 'chrome': Runtime.chrome, - 'firefox': Runtime.firefox + 'firefox': Runtime.firefox, + 'safari': Runtime.safari, }; final Map browserToConfiguration = { 'chrome': '--configuration=${environment.webUiRootDir.path}/dart_test_chrome.yaml', 'firefox': '--configuration=${environment.webUiRootDir.path}/dart_test_firefox.yaml', + 'safari': '--configuration=${environment.webUiRootDir.path}/dart_test_safari.yaml', }; static final SupportedBrowsers _singletonInstance = SupportedBrowsers._(); @@ -48,6 +52,8 @@ class SupportedBrowsers { return Chrome(url, debug: debug); } else if (runtime == Runtime.firefox) { return Firefox(url, debug: debug); + } else if (runtime == Runtime.safari) { + return Safari(url, debug: debug); } else { throw new UnsupportedError('The browser type not supported in tests'); } From bac12468eace4257296509620e09bc45c9e1eb6d Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 18 Dec 2019 15:44:35 -0800 Subject: [PATCH 3/4] changing the directrory for redirect file. This works without opening a permision pop-up. Note: there is no headless mode in safari --- lib/web_ui/dev/safari.dart | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/dev/safari.dart b/lib/web_ui/dev/safari.dart index de80242e55187..f96bd7cbf7772 100644 --- a/lib/web_ui/dev/safari.dart +++ b/lib/web_ui/dev/safari.dart @@ -6,6 +6,8 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'environment.dart'; + import 'package:path/path.dart' as path; import 'package:pedantic/pedantic.dart'; @@ -41,20 +43,21 @@ class Safari extends Browser { infoLog: DevNull(), ); - var dir = createTempDir(); - // Safari will only open files (not general URLs) via the command-line // API, so we create a dummy file to redirect it to the page we actually // want it to load. - var redirect = path.join(dir, 'redirect.html'); + final Directory redirectDir = Directory( + path.join(environment.webUiDartToolDir.path), + ); + final redirect = path.join(redirectDir.path, 'redirect.html'); File(redirect).writeAsStringSync( ''); - var process = await Process.start( - installation.executable, [redirect] /* args */); + var process = + await Process.start(installation.executable, [redirect] /* args */); unawaited(process.exitCode - .then((_) => Directory(dir).deleteSync(recursive: true))); + .then((_) => File(redirect).deleteSync(recursive: true))); return process; }); From 0f58499a93aecfe7f774057ba5128ca68e716b1e Mon Sep 17 00:00:00 2001 From: nturgut Date: Thu, 19 Dec 2019 07:32:11 -0800 Subject: [PATCH 4/4] addressing reviewer comments --- lib/web_ui/dev/safari_installation.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/dev/safari_installation.dart b/lib/web_ui/dev/safari_installation.dart index 3df7ec6798dc2..2f651d72d2202 100644 --- a/lib/web_ui/dev/safari_installation.dart +++ b/lib/web_ui/dev/safari_installation.dart @@ -26,7 +26,7 @@ class SafariArgParser extends BrowserArgParser { 'safari-version', defaultsTo: 'system', help: 'The Safari version to use while running tests. The Safari ' - 'browswer installed on the system is used as the only option now.' + 'browser installed on the system is used as the only option now.' 'Soon we will add support for using different versions using the ' 'tech previews.', ); @@ -58,7 +58,8 @@ Future getOrInstallSafari( // These tests are aimed to run only on MacOs machines local or on LUCI. if (!io.Platform.isMacOS) { - throw UnimplementedError(); + throw UnimplementedError('Safari on ${io.Platform.operatingSystem} is' + ' not supported. Safari is only supported on MacOS.'); } infoLog ??= io.stdout; @@ -66,7 +67,7 @@ Future getOrInstallSafari( if (requestedVersion == 'system') { // Since Safari is included in MacOS, always assume there will be one on the // system. - infoLog.writeln('Using the system version is already installed.'); + infoLog.writeln('Using the system version that is already installed.'); return BrowserInstallation( version: 'system', executable: PlatformBinding.instance.getSafariSystemExecutablePath(),