From e5bd86132e75139ea48137fd2c220f3d808dc063 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 30 Jun 2021 16:17:35 -0600 Subject: [PATCH 1/2] [web] replace browser-related conditional logic with BrowserEnvironment --- lib/web_ui/dev/browser.dart | 54 ++++++ lib/web_ui/dev/chrome.dart | 90 ++++++++++ lib/web_ui/dev/edge.dart | 24 +++ lib/web_ui/dev/firefox.dart | 23 +++ lib/web_ui/dev/safari.dart | 74 -------- ...creenshot_manager.dart => safari_ios.dart} | 162 +++++++----------- lib/web_ui/dev/safari_macos.dart | 82 +++++++++ lib/web_ui/dev/supported_browsers.dart | 91 ---------- lib/web_ui/dev/test_platform.dart | 58 +++---- lib/web_ui/dev/test_runner.dart | 80 ++++++--- 10 files changed, 416 insertions(+), 322 deletions(-) delete mode 100644 lib/web_ui/dev/safari.dart rename lib/web_ui/dev/{screenshot_manager.dart => safari_ios.dart} (58%) create mode 100644 lib/web_ui/dev/safari_macos.dart delete mode 100644 lib/web_ui/dev/supported_browsers.dart diff --git a/lib/web_ui/dev/browser.dart b/lib/web_ui/dev/browser.dart index 5f1d31774c50f..ed7a94d63e97d 100644 --- a/lib/web_ui/dev/browser.dart +++ b/lib/web_ui/dev/browser.dart @@ -5,11 +5,50 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:math' as math; +import 'package:image/image.dart'; import 'package:pedantic/pedantic.dart'; import 'package:stack_trace/stack_trace.dart'; +import 'package:test_api/src/backend/runtime.dart'; import 'package:typed_data/typed_buffers.dart'; +/// Provides the environment for a specific web browser. +abstract class BrowserEnvironment { + /// The [Runtime] used by `package:test` to identify this browser type. + Runtime get packageTestRuntime; + + /// The name of the configuration YAML file used to configure `package:test`. + /// + /// The configuration file is expected to be a direct child of the `web_ui` + /// directory. + String get packageTestConfigurationYamlFile; + + /// Prepares the OS environment to run tests for this browser. + /// + /// This may include things like staring web drivers, iOS Simulators, and/or + /// Android emulators. + /// + /// Typically the browser environment is prepared once and supports multiple + /// browser instances. + Future prepareEnvironment(); + + /// Launches a browser instance. + /// + /// The browser will be directed to open the provided [url]. + /// + /// If [debug] is true and the browser supports debugging, launches the + /// browser in debug mode by pausing test execution after the code is loaded + /// but before calling the `main()` function of the test, giving the + /// developer a chance to set breakpoints. + Browser launchBrowserInstance(Uri url, {bool debug = false}); + + /// Returns the screenshot manager used by this browser. + /// + /// If the browser does not support screenshots, returns null. + ScreenshotManager? getScreenshotManager(); +} + /// An interface for running browser instances. /// /// This is intentionally coarse-grained: browsers are controlled primary from @@ -147,3 +186,18 @@ abstract class Browser { return onExit.catchError((dynamic _) {}); } } + +/// Interface for capturing screenshots from a browser. +abstract class ScreenshotManager { + /// Capture a screenshot. + /// + /// Please read the details for the implementing classes. + Future capture(math.Rectangle region); + + /// Suffix to be added to the end of the filename. + /// + /// Example file names: + /// - Chrome, no-suffix: backdrop_filter_clip_moved.actual.png + /// - iOS Safari: backdrop_filter_clip_moved.actual.iOS_Safari.png + String get filenameSuffix; +} diff --git a/lib/web_ui/dev/chrome.dart b/lib/web_ui/dev/chrome.dart index cc8a8452bd097..79012d2239e75 100644 --- a/lib/web_ui/dev/chrome.dart +++ b/lib/web_ui/dev/chrome.dart @@ -5,14 +5,43 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:math' as math; +import 'package:image/image.dart'; import 'package:pedantic/pedantic.dart'; +import 'package:test_api/src/backend/runtime.dart'; +import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' + as wip; import 'browser.dart'; import 'chrome_installer.dart'; import 'common.dart'; import 'environment.dart'; +/// Provides an environment for desktop Chrome. +class ChromeEnvironment implements BrowserEnvironment { + @override + Browser launchBrowserInstance(Uri url, {bool debug = false}) { + return Chrome(url, debug: debug); + } + + @override + Runtime get packageTestRuntime => Runtime.chrome; + + @override + Future prepareEnvironment() async { + // Chrome doesn't need any special prep. + } + + @override + ScreenshotManager? getScreenshotManager() { + return ChromeScreenshotManager(); + } + + @override + String get packageTestConfigurationYamlFile => 'dart_test_chrome.yaml'; +} + /// A class for running an instance of Chrome. /// /// Most of the communication with the browser is expected to happen via HTTP, @@ -174,3 +203,64 @@ Future getRemoteDebuggerUrl(Uri base) async { return base; } } + +/// [ScreenshotManager] implementation for Chrome. +/// +/// This manager can be used for both macOS and Linux. +// TODO: https://github.com/flutter/flutter/issues/65673 +class ChromeScreenshotManager extends ScreenshotManager { + String get filenameSuffix => ''; + + /// Capture a screenshot of the web content. + /// + /// Uses Webkit Inspection Protocol server's `captureScreenshot` API. + /// + /// [region] is used to decide which part of the web content will be used in + /// test image. It includes starting coordinate x,y as well as height and + /// width of the area to capture. + Future capture(math.Rectangle? region) async { + final wip.ChromeConnection chromeConnection = + wip.ChromeConnection('localhost', kDevtoolsPort); + final wip.ChromeTab? chromeTab = await chromeConnection.getTab( + (wip.ChromeTab chromeTab) => chromeTab.url.contains('localhost')); + if (chromeTab == null) { + throw StateError( + 'Failed locate Chrome tab with the test page', + ); + } + final wip.WipConnection wipConnection = await chromeTab.connect(); + + Map? captureScreenshotParameters = null; + if (region != null) { + captureScreenshotParameters = { + 'format': 'png', + 'clip': { + 'x': region.left, + 'y': region.top, + 'width': region.width, + 'height': region.height, + 'scale': + // This is NOT the DPI of the page, instead it's the "zoom level". + 1, + }, + }; + } + + // Setting hardware-independent screen parameters: + // https://chromedevtools.github.io/devtools-protocol/tot/Emulation + await wipConnection + .sendCommand('Emulation.setDeviceMetricsOverride', { + 'width': kMaxScreenshotWidth, + 'height': kMaxScreenshotHeight, + 'deviceScaleFactor': 1, + 'mobile': false, + }); + final wip.WipResponse response = await wipConnection.sendCommand( + 'Page.captureScreenshot', captureScreenshotParameters); + + final Image screenshot = + decodePng(base64.decode(response.result!['data'] as String))!; + + return screenshot; + } +} diff --git a/lib/web_ui/dev/edge.dart b/lib/web_ui/dev/edge.dart index 3059fd21f4aff..1acb270cdc03c 100644 --- a/lib/web_ui/dev/edge.dart +++ b/lib/web_ui/dev/edge.dart @@ -5,10 +5,34 @@ import 'dart:async'; import 'dart:io'; +import 'package:test_api/src/backend/runtime.dart'; + import 'browser.dart'; import 'common.dart'; import 'edge_installation.dart'; +/// Provides an environment for the desktop Microsoft Edge (Chromium-based). +class EdgeEnvironment implements BrowserEnvironment { + @override + Browser launchBrowserInstance(Uri url, {bool debug = false}) { + return Edge(url, debug: debug); + } + + @override + Runtime get packageTestRuntime => Runtime.internetExplorer; + + @override + Future prepareEnvironment() async { + // Edge doesn't need any special prep. + } + + @override + ScreenshotManager? getScreenshotManager() => null; + + @override + String get packageTestConfigurationYamlFile => 'dart_test_edge.yaml'; +} + /// A class for running an instance of Edge. /// /// Most of the communication with the browser is expected to happen via HTTP, diff --git a/lib/web_ui/dev/firefox.dart b/lib/web_ui/dev/firefox.dart index c479133587762..156c89fecef76 100644 --- a/lib/web_ui/dev/firefox.dart +++ b/lib/web_ui/dev/firefox.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'package:pedantic/pedantic.dart'; import 'package:path/path.dart' as path; +import 'package:test_api/src/backend/runtime.dart'; import 'package:test_core/src/util/io.dart'; import 'browser.dart'; @@ -15,6 +16,28 @@ import 'common.dart'; import 'environment.dart'; import 'firefox_installer.dart'; +/// Provides an environment for the desktop Firefox. +class FirefoxEnvironment implements BrowserEnvironment { + @override + Browser launchBrowserInstance(Uri url, {bool debug = false}) { + return Firefox(url, debug: debug); + } + + @override + Runtime get packageTestRuntime => Runtime.firefox; + + @override + Future prepareEnvironment() async { + // Firefox doesn't need any special prep. + } + + @override + String get packageTestConfigurationYamlFile => 'dart_test_firefox.yaml'; + + @override + ScreenshotManager? getScreenshotManager() => null; +} + /// A class for running an instance of Firefox. /// /// Most of the communication with the browser is expected to happen via HTTP, diff --git a/lib/web_ui/dev/safari.dart b/lib/web_ui/dev/safari.dart deleted file mode 100644 index cbcd42ca2f8e0..0000000000000 --- a/lib/web_ui/dev/safari.dart +++ /dev/null @@ -1,74 +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 'dart:io'; - -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'; - - /// 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}) { - final String version = SafariArgParser.instance.version; - final bool isMobileBrowser = SafariArgParser.instance.isMobileBrowser; - return Safari._(() async { - if (isMobileBrowser) { - // iOS-Safari - // Uses `xcrun simctl`. It is a command line utility to control the - // Simulator. For more details on interacting with the simulator: - // https://developer.apple.com/library/archive/documentation/IDEs/Conceptual/iOS_Simulator_Guide/InteractingwiththeiOSSimulator/InteractingwiththeiOSSimulator.html - var process = await Process.start('xcrun', [ - 'simctl', - 'openurl', // Opens the url on Safari installed on the simulator. - 'booted', // The simulator is already booted. - '${url.toString()}' - ]); - - return process; - } else { - // Desktop-Safari - // TODO(nurhan): Configure info log for LUCI. - final BrowserInstallation installation = await getOrInstallSafari( - version, - infoLog: DevNull(), - ); - - // In the macOS Catalina opening Safari browser with a file brings - // a popup which halts the test. - // The following list of arguments needs to be provided to the `open` - // command to open Safari for a given URL. In summary, `open` tool opens - // a new Safari browser (even if one is already open), opens it with no - // persistent state and wait until it opens. - // The details copied from `man open` on macOS. - // TODO(nurhan): https://github.com/flutter/flutter/issues/50809 - var process = await Process.start(installation.executable, [ - // These are flags for `open` command line tool. - '-F', // Open a fresh Safari with no persistent state. - '-W', // Wait until the Safari opens. - '-n', // Open a new instance of the Safari even another one is open. - '-b', // Specifies the bundle identifier for the application to use. - 'com.apple.Safari', // Bundle identifier for Safari. - '${url.toString()}' - ]); - - return process; - } - }); - } - - Safari._(Future startBrowser()) : super(startBrowser); -} diff --git a/lib/web_ui/dev/screenshot_manager.dart b/lib/web_ui/dev/safari_ios.dart similarity index 58% rename from lib/web_ui/dev/screenshot_manager.dart rename to lib/web_ui/dev/safari_ios.dart index 6a75ebc252ab5..c1906ab3d621d 100644 --- a/lib/web_ui/dev/screenshot_manager.dart +++ b/lib/web_ui/dev/safari_ios.dart @@ -2,89 +2,86 @@ // 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 'dart:convert'; -import 'dart:math'; +import 'dart:math' as math; import 'package:image/image.dart'; import 'package:path/path.dart' as path; -import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart' - as wip; +import 'package:test_api/src/backend/runtime.dart'; import 'package:yaml/yaml.dart'; +import 'browser.dart'; import 'common.dart'; import 'environment.dart'; import 'safari_installation.dart'; import 'utils.dart'; -/// [ScreenshotManager] implementation for Chrome. -/// -/// This manager can be used for both macOS and Linux. -// TODO: https://github.com/flutter/flutter/issues/65673 -class ChromeScreenshotManager extends ScreenshotManager { - String get filenameSuffix => ''; +/// Provides an environment for the mobile variant of Safari running in an iOS +/// simulator. +class SafariIosEnvironment implements BrowserEnvironment { + @override + Browser launchBrowserInstance(Uri url, {bool debug = false}) { + return SafariIos(url); + } - /// Capture a screenshot of the web content. - /// - /// Uses Webkit Inspection Protocol server's `captureScreenshot` API. - /// - /// [region] is used to decide which part of the web content will be used in - /// test image. It includes starting coordinate x,y as well as height and - /// width of the area to capture. - Future capture(Rectangle? region) async { - final wip.ChromeConnection chromeConnection = - wip.ChromeConnection('localhost', kDevtoolsPort); - final wip.ChromeTab? chromeTab = await chromeConnection.getTab( - (wip.ChromeTab chromeTab) => chromeTab.url.contains('localhost')); - if (chromeTab == null) { - throw StateError( - 'Failed locate Chrome tab with the test page', - ); - } - final wip.WipConnection wipConnection = await chromeTab.connect(); - - Map? captureScreenshotParameters = null; - if (region != null) { - captureScreenshotParameters = { - 'format': 'png', - 'clip': { - 'x': region.left, - 'y': region.top, - 'width': region.width, - 'height': region.height, - 'scale': - // This is NOT the DPI of the page, instead it's the "zoom level". - 1, - }, - }; - } + @override + Runtime get packageTestRuntime => Runtime.safari; - // Setting hardware-independent screen parameters: - // https://chromedevtools.github.io/devtools-protocol/tot/Emulation - await wipConnection - .sendCommand('Emulation.setDeviceMetricsOverride', { - 'width': kMaxScreenshotWidth, - 'height': kMaxScreenshotHeight, - 'deviceScaleFactor': 1, - 'mobile': false, - }); - final wip.WipResponse response = await wipConnection.sendCommand( - 'Page.captureScreenshot', captureScreenshotParameters); + @override + Future prepareEnvironment() async { + await IosSafariArgParser.instance.initIosSimulator(); + } - final Image screenshot = - decodePng(base64.decode(response.result!['data'] as String))!; + @override + ScreenshotManager? getScreenshotManager() { + return SafariIosScreenshotManager(); + } + + @override + String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml'; +} - return screenshot; +/// A class for running an instance of Safari for iOS (i.e. mobile 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 SafariIos extends Browser { + @override + final name = 'Safari iOS'; + + /// Starts a new instance of Safari open to the given [url], which may be a + /// [Uri]. + factory SafariIos(Uri url) { + return SafariIos._(() async { + // iOS-Safari + // Uses `xcrun simctl`. It is a command line utility to control the + // Simulator. For more details on interacting with the simulator: + // https://developer.apple.com/library/archive/documentation/IDEs/Conceptual/iOS_Simulator_Guide/InteractingwiththeiOSSimulator/InteractingwiththeiOSSimulator.html + var process = await io.Process.start('xcrun', [ + 'simctl', + 'openurl', // Opens the url on Safari installed on the simulator. + 'booted', // The simulator is already booted. + '${url.toString()}' + ]); + + return process; + }); } + + SafariIos._(Future startBrowser()) : super(startBrowser); } /// [ScreenshotManager] implementation for Safari. /// /// This manager will only be created/used for macOS. -class IosSafariScreenshotManager extends ScreenshotManager { +class SafariIosScreenshotManager extends ScreenshotManager { String get filenameSuffix => '.iOS_Safari'; - IosSafariScreenshotManager() { + SafariIosScreenshotManager() { final YamlMap browserLock = BrowserLock.instance.configuration; _heightOfHeader = browserLock['ios-safari']['heightOfHeader'] as int; _heightOfFooter = browserLock['ios-safari']['heightOfFooter'] as int; @@ -157,7 +154,7 @@ class IosSafariScreenshotManager extends ScreenshotManager { /// width of the area to capture. /// /// Uses simulator tool `xcrun simctl`'s 'screenshot' command. - Future capture(Rectangle? region) async { + Future capture(math.Rectangle? region) async { final String filename = 'screenshot${_fileNameCounter}.png'; _fileNameCounter++; @@ -189,7 +186,7 @@ class IosSafariScreenshotManager extends ScreenshotManager { if (region == null) { return content; } else { - final Rectangle scaledRegion = _scaleScreenshotRegion(region); + final math.Rectangle scaledRegion = _scaleScreenshotRegion(region); return copyCrop( content, scaledRegion.left.toInt(), @@ -203,8 +200,8 @@ class IosSafariScreenshotManager extends ScreenshotManager { /// Perform a linear transform on the screenshot region to convert its /// dimensions from linear coordinated to coordinated on the phone screen. /// This uniform/isotropic scaling is done using [_scaleFactor]. - Rectangle _scaleScreenshotRegion(Rectangle region) { - return Rectangle( + math.Rectangle _scaleScreenshotRegion(math.Rectangle region) { + return math.Rectangle( region.left * _scaleFactor, region.top * _scaleFactor, region.width * _scaleFactor, @@ -212,40 +209,3 @@ class IosSafariScreenshotManager extends ScreenshotManager { ); } } - -const String _kBrowserChrome = 'chrome'; -const String _kBrowserIOSSafari = 'ios-safari'; - -typedef ScreenshotManagerFactory = ScreenshotManager Function(); - -/// Abstract class for taking screenshots in one of the browsers. -abstract class ScreenshotManager { - static final Map _browserFactories = - { - _kBrowserChrome: () => ChromeScreenshotManager(), - _kBrowserIOSSafari: () => IosSafariScreenshotManager(), - }; - - static bool isBrowserSupported(String browser) => - _browserFactories.containsKey(browser); - - static ScreenshotManager choose(String browser) { - if (isBrowserSupported(browser)) { - return _browserFactories[browser]!(); - } - throw StateError('Screenshot tests are only supported on Chrome and on ' - 'iOS Safari'); - } - - /// Capture a screenshot. - /// - /// Please read the details for the implementing classes. - Future capture(Rectangle region); - - /// Suffix to be added to the end of the filename. - /// - /// Example file names: - /// - Chrome, no-suffix: backdrop_filter_clip_moved.actual.png - /// - iOS Safari: backdrop_filter_clip_moved.actual.iOS_Safari.png - String get filenameSuffix; -} diff --git a/lib/web_ui/dev/safari_macos.dart b/lib/web_ui/dev/safari_macos.dart new file mode 100644 index 0000000000000..b8acf2526e30c --- /dev/null +++ b/lib/web_ui/dev/safari_macos.dart @@ -0,0 +1,82 @@ +// 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'; + +import 'package:test_api/src/backend/runtime.dart'; + +import 'browser.dart'; +import 'common.dart'; +import 'safari_installation.dart'; + +/// Provides an environment for the desktop variant of Safari running on macOS. +class SafariMacOsEnvironment implements BrowserEnvironment { + @override + Browser launchBrowserInstance(Uri url, {bool debug = false}) { + return SafariMacOs(url); + } + + @override + Runtime get packageTestRuntime => Runtime.safari; + + @override + Future prepareEnvironment() async { + // Nothing extra to prepare for desktop Safari. + } + + // We do not yet support screenshots on desktop Safari. + @override + ScreenshotManager? getScreenshotManager() => null; + + @override + String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml'; +} + +/// A class for running an instance of Safari for macOS (i.e. desktop 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 SafariMacOs extends Browser { + @override + final name = 'Safari macOS'; + + /// Starts a new instance of Safari open to the given [url], which may be a + /// [Uri]. + factory SafariMacOs(Uri url) { + final String version = SafariArgParser.instance.version; + return SafariMacOs._(() async { + // TODO(nurhan): Configure info log for LUCI. + final BrowserInstallation installation = await getOrInstallSafari( + version, + infoLog: DevNull(), + ); + + // In the macOS Catalina opening Safari browser with a file brings + // a popup which halts the test. + // The following list of arguments needs to be provided to the `open` + // command to open Safari for a given URL. In summary, `open` tool opens + // a new Safari browser (even if one is already open), opens it with no + // persistent state and wait until it opens. + // The details copied from `man open` on macOS. + // TODO(nurhan): https://github.com/flutter/flutter/issues/50809 + var process = await Process.start(installation.executable, [ + // These are flags for `open` command line tool. + '-F', // Open a fresh Safari with no persistent state. + '-W', // Wait until the Safari opens. + '-n', // Open a new instance of the Safari even another one is open. + '-b', // Specifies the bundle identifier for the application to use. + 'com.apple.Safari', // Bundle identifier for Safari. + '${url.toString()}' + ]); + + return process; + }); + } + + SafariMacOs._(Future startBrowser()) : super(startBrowser); +} diff --git a/lib/web_ui/dev/supported_browsers.dart b/lib/web_ui/dev/supported_browsers.dart deleted file mode 100644 index dff687a209ac0..0000000000000 --- a/lib/web_ui/dev/supported_browsers.dart +++ /dev/null @@ -1,91 +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 'package:test_api/src/backend/runtime.dart'; - -import 'browser.dart'; -import 'chrome.dart'; -import 'chrome_installer.dart'; -import 'common.dart'; -import 'edge.dart'; -import 'environment.dart'; -import 'edge_installation.dart'; -import 'firefox.dart'; -import 'firefox_installer.dart'; -import 'safari.dart'; -import 'safari_installation.dart'; - -/// Utilities for browsers, that tests are supported. -/// -/// Extending [Browser] is not enough for supporting test. -/// -/// Each new browser should be added to the [Runtime] map, to the [getBrowser] -/// method. -/// -/// One should also implement [BrowserArgParser] and add it to the [argParsers]. -class SupportedBrowsers { - final List argParsers = List.of([ - ChromeArgParser.instance, - EdgeArgParser.instance, - FirefoxArgParser.instance, - SafariArgParser.instance - ]); - - final List supportedBrowserNames = [ - 'chrome', - 'edge', - 'firefox', - 'safari' - ]; - - final Map supportedBrowsersToRuntimes = { - 'chrome': Runtime.chrome, - 'edge': Runtime.internetExplorer, - 'firefox': Runtime.firefox, - 'safari': Runtime.safari, - 'ios-safari': Runtime.safari, - }; - - final Map supportedBrowserToPlatform = { - 'chrome': 'chrome', - 'edge': 'ie', - 'firefox': 'firefox', - 'safari': 'safari', - 'ios-safari': 'safari', - }; - - final Map browserToConfiguration = { - 'chrome': - '--configuration=${environment.webUiRootDir.path}/dart_test_chrome.yaml', - 'edge': - '--configuration=${environment.webUiRootDir.path}/dart_test_edge.yaml', - 'firefox': - '--configuration=${environment.webUiRootDir.path}/dart_test_firefox.yaml', - 'safari': - '--configuration=${environment.webUiRootDir.path}/dart_test_safari.yaml', - 'ios-safari': - '--configuration=${environment.webUiRootDir.path}/dart_test_safari.yaml', - }; - - static final SupportedBrowsers _singletonInstance = SupportedBrowsers._(); - - /// The [SupportedBrowsers] singleton. - static SupportedBrowsers get instance => _singletonInstance; - - SupportedBrowsers._(); - - Browser getBrowser(Runtime runtime, Uri url, {bool debug = false}) { - if (runtime == Runtime.chrome) { - return Chrome(url, debug: debug); - } else if (runtime == Runtime.internetExplorer) { - return Edge(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'); - } - } -} diff --git a/lib/web_ui/dev/test_platform.dart b/lib/web_ui/dev/test_platform.dart index a04764e2e83af..946fb29026de3 100644 --- a/lib/web_ui/dev/test_platform.dart +++ b/lib/web_ui/dev/test_platform.dart @@ -39,26 +39,22 @@ import 'package:test_core/src/runner/configuration.dart'; import 'browser.dart'; import 'common.dart'; import 'environment.dart' as env; -import 'screenshot_manager.dart'; -import 'supported_browsers.dart'; /// Custom test platform that serves web engine unit tests. class BrowserPlatform extends PlatformPlugin { /// Starts the server. /// - /// [browserName] is the name of the browser that's used to run the test. It - /// must be supported by [SupportedBrowsers]. + /// [browserEnvironment] provides the browser environment to run the test. /// /// If [doUpdateScreenshotGoldens] is true updates screenshot golden files /// instead of failing the test on screenshot mismatches. static Future start({ - required String browserName, + required BrowserEnvironment browserEnvironment, required bool doUpdateScreenshotGoldens, }) async { - assert(SupportedBrowsers.instance.supportedBrowserNames.contains(browserName)); final shelf_io.IOServer server = shelf_io.IOServer(await HttpMultiServer.loopback(0)); return BrowserPlatform._( - browserName: browserName, + browserEnvironment: browserEnvironment, server: server, isDebug: Configuration.current.pauseAfterLoad, faviconPath: p.fromUri(await Isolate.resolvePackageUri( @@ -76,8 +72,8 @@ class BrowserPlatform extends PlatformPlugin { /// The underlying server. final shelf.Server server; - /// Name for the running browser. Not final on purpose can be mutated later. - String browserName; + /// Provides the environment for the browser running tests. + final BrowserEnvironment browserEnvironment; /// The URL for this server. Uri get url => server.url.resolve('/'); @@ -105,7 +101,7 @@ class BrowserPlatform extends PlatformPlugin { final PackageConfig packageConfig; BrowserPlatform._({ - required this.browserName, + required this.browserEnvironment, required this.server, required this.isDebug, required String faviconPath, @@ -154,10 +150,9 @@ class BrowserPlatform extends PlatformPlugin { // This handler goes last, after all more specific handlers failed to handle the request. .add(_createAbsolutePackageUrlHandler()); - // Screenshot tests are only enabled in Chrome and Safari iOS for now. - if (browserName == 'chrome' || browserName == 'ios-safari') { + _screenshotManager = browserEnvironment.getScreenshotManager(); + if (_screenshotManager != null) { cascade = cascade.add(_screeshotHandler); - _screenshotManager = ScreenshotManager.choose(browserName); } server.mount(cascade.handler); @@ -238,11 +233,6 @@ class BrowserPlatform extends PlatformPlugin { } Future _screeshotHandler(shelf.Request request) async { - if (browserName != 'chrome' && browserName != 'ios-safari') { - throw Exception('Screenshots tests are only available in Chrome ' - 'and in Safari-iOS.'); - } - if (!request.requestedUri.path.endsWith('/screenshot')) { return shelf.Response.notFound( 'This request is not handled by the screenshot handler'); @@ -370,7 +360,7 @@ class BrowserPlatform extends PlatformPlugin { p.toUri(p.withoutExtension(p.relative(path, from: env.environment.webUiBuildDir.path)) + '.html')); _checkNotClosed(); - final BrowserManager? browserManager = await _browserManagerFor(browser); + final BrowserManager? browserManager = await _startBrowserManager(); if (browserManager == null) { throw StateError('Failed to initialize browser manager for ${browser.name}'); } @@ -386,10 +376,10 @@ class BrowserPlatform extends PlatformPlugin { Future? _browserManager; - /// Returns the [BrowserManager] for [runtime], which should be a browser. + /// Starts a browser manager for the browser provided by [browserEnvironment]; /// /// If no browser manager is running yet, starts one. - Future _browserManagerFor(Runtime browser) { + Future _startBrowserManager() { if (_browserManager != null) { return _browserManager!; } @@ -405,7 +395,7 @@ class BrowserPlatform extends PlatformPlugin { }); final Future future = BrowserManager.start( - runtime: browser, + browserEnvironment: browserEnvironment, url: hostUrl, future: completer.future, packageConfig: packageConfig, @@ -505,8 +495,8 @@ class BrowserManager { /// The browser instance that this is connected to via [_channel]. final Browser _browser; - /// The [Runtime] for [_browser]. - final Runtime _runtime; + /// The browser environment for this test. + final BrowserEnvironment _browserEnvironment; /// The channel used to communicate with the browser. /// @@ -567,13 +557,13 @@ class BrowserManager { /// Returns the browser manager, or throws an [Exception] if a /// connection fails to be established. static Future start({ - required Runtime runtime, + required BrowserEnvironment browserEnvironment, required Uri url, required Future future, required PackageConfig packageConfig, bool debug = false, }) { - var browser = _newBrowser(url, runtime, debug: debug); + var browser = _newBrowser(url, browserEnvironment, debug: debug); var completer = Completer(); @@ -593,7 +583,7 @@ class BrowserManager { if (completer.isCompleted) { return; } - completer.complete(BrowserManager._(packageConfig, browser, runtime, webSocket)); + completer.complete(BrowserManager._(packageConfig, browser, browserEnvironment, webSocket)); }).catchError((Object error, StackTrace stackTrace) { browser.close(); if (completer.isCompleted) { @@ -605,16 +595,16 @@ class BrowserManager { return completer.future; } - /// Starts the browser identified by [browser] using [settings] and has it load [url]. + /// Starts the browser and requests that it load the test page at [url]. /// /// If [debug] is true, starts the browser in debug mode. - static Browser _newBrowser(Uri url, Runtime browser, {bool debug = false}) { - return SupportedBrowsers.instance.getBrowser(browser, url, debug: debug); + static Browser _newBrowser(Uri url, BrowserEnvironment browserEnvironment, {bool debug = false}) { + return browserEnvironment.launchBrowserInstance(url, debug: debug); } - /// Creates a new BrowserManager that communicates with [browser] over + /// Creates a new BrowserManager that communicates with the browser over /// [webSocket]. - BrowserManager._(this.packageConfig, this._browser, this._runtime, WebSocketChannel webSocket) { + BrowserManager._(this.packageConfig, this._browser, this._browserEnvironment, WebSocketChannel webSocket) { // The duration should be short enough that the debugging console is open as // soon as the user is done setting breakpoints, but long enough that a test // doing a lot of synchronous work doesn't trigger a false positive. @@ -665,7 +655,7 @@ class BrowserManager { url = url.replace( fragment: Uri.encodeFull(jsonEncode({ 'metadata': suiteConfig.metadata.serialize(), - 'browser': _runtime.identifier + 'browser': _browserEnvironment.packageTestRuntime.identifier }))); var suiteID = _suiteID++; @@ -697,7 +687,7 @@ class BrowserManager { }); try { - controller = deserializeSuite(path, currentPlatform(_runtime), + controller = deserializeSuite(path, currentPlatform(_browserEnvironment.packageTestRuntime), suiteConfig, await _environment, suiteChannel, message); final String sourceMapFileName = diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index f1edb7ee50da2..f0322de57dcbb 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -28,13 +28,21 @@ import 'package:test_core/src/executable.dart' as test; import 'package:web_test_utils/goldens.dart'; +import 'browser.dart'; +import 'chrome.dart'; +import 'chrome_installer.dart'; import 'common.dart'; +import 'edge.dart'; +import 'edge_installation.dart'; import 'environment.dart'; import 'exceptions.dart'; +import 'firefox.dart'; +import 'firefox_installer.dart'; import 'integration_tests_manager.dart'; import 'macos_info.dart'; import 'safari_installation.dart'; -import 'supported_browsers.dart'; +import 'safari_ios.dart'; +import 'safari_macos.dart'; import 'test_platform.dart'; import 'utils.dart'; import 'watcher.dart'; @@ -62,6 +70,29 @@ enum TestTypesRequested { all, } +/// Command-line argument parsers that parse browser-specific options. +final List _browserArgParsers = [ + ChromeArgParser.instance, + EdgeArgParser.instance, + FirefoxArgParser.instance, + SafariArgParser.instance, +]; + +/// Creates an environment for a browser. +/// +/// The [browserName] matches the browser name passed as the `--browser` option. +BrowserEnvironment _createBrowserEnvironment(String browserName) { + switch (browserName) { + case 'chrome': return ChromeEnvironment(); + case 'edge': return EdgeEnvironment(); + case 'firefox': return FirefoxEnvironment(); + case 'safari': return SafariMacOsEnvironment(); + case 'ios-safari': return SafariIosEnvironment(); + } + throw UnsupportedError('Browser $browserName is not supported.'); +} + +/// Runs tests. class TestCommand extends Command with ArgUtils { TestCommand() { argParser @@ -113,12 +144,11 @@ class TestCommand extends Command with ArgUtils { 'for example, when a new browser version affects pixels.', ) ..addFlag( - 'fetch-goldens-repo', - defaultsTo: true, - negatable: true, - help: 'Whether to fetch the goldens repo. Set this to false to iterate ' - 'on golden tests without fearing that the fetcher will overwrite ' - 'your local changes.', + 'skip-goldens-repo-fetch', + defaultsTo: false, + help: 'If set reuses the existig flutter/goldens repo clone. Use this ' + 'to avoid overwriting local changes when iterating on golden ' + 'tests. This is off by default.', ) ..addOption( 'browser', @@ -136,8 +166,9 @@ class TestCommand extends Command with ArgUtils { 'finish.', ); - SupportedBrowsers.instance.argParsers - .forEach((t) => t.populateOptions(argParser)); + for (BrowserArgParser browserArgParser in _browserArgParsers) { + browserArgParser.populateOptions(argParser); + } GeneralTestsArgumentParser.instance.populateOptions(argParser); IntegrationTestsArgumentParser.instance.populateOptions(argParser); } @@ -183,8 +214,9 @@ class TestCommand extends Command with ArgUtils { @override Future run() async { - SupportedBrowsers.instance - ..argParsers.forEach((t) => t.parseOptions(argResults!)); + for (BrowserArgParser browserArgParser in _browserArgParsers) { + browserArgParser.parseOptions(argResults!); + } GeneralTestsArgumentParser.instance.parseOptions(argResults!); /// Collect information on the bot. @@ -310,7 +342,7 @@ class TestCommand extends Command with ArgUtils { environment.webUiTestResultsDirectory.createSync(recursive: true); // If screenshot tests are available, fetch the screenshot goldens. - if (isScreenshotTestsAvailable && doFetchGoldensRepo) { + if (isScreenshotTestsAvailable && !skipGoldensRepoFetch) { if (isVerboseLoggingEnabled) { print('INFO: Fetching goldens repo'); } @@ -320,11 +352,7 @@ class TestCommand extends Command with ArgUtils { await goldensRepoFetcher.fetch(); } - // In order to run iOS Safari unit tests we need to make sure iOS Simulator - // is booted. - if (isSafariIOS) { - await IosSafariArgParser.instance.initIosSimulator(); - } + await browserEnvironment.prepareEnvironment(); _testPreparationReady = true; } @@ -391,6 +419,10 @@ class TestCommand extends Command with ArgUtils { /// The name of the browser to run tests in. String get browser => stringArg('browser')!; + /// The browser environment for the [browser]. + BrowserEnvironment get browserEnvironment => (_browserEnvironment ??= _createBrowserEnvironment(browser)); + BrowserEnvironment? _browserEnvironment; + /// Whether [browser] is set to "chrome". bool get isChrome => browser == 'chrome'; @@ -455,7 +487,7 @@ class TestCommand extends Command with ArgUtils { bool get doUpdateScreenshotGoldens => boolArg('update-screenshot-goldens')!; /// Whether to fetch the goldens repo prior to running tests. - bool get doFetchGoldensRepo => boolArg('fetch-goldens-repo')!; + bool get skipGoldensRepoFetch => boolArg('skip-goldens-repo-fetch')!; /// Runs all tests specified in [targets]. /// @@ -692,6 +724,10 @@ class TestCommand extends Command with ArgUtils { required int concurrency, required bool expectFailure, }) async { + final String configurationFilePath = path.join( + environment.webUiRootDir.path, + browserEnvironment.packageTestConfigurationYamlFile, + ); final List testArgs = [ ...['-r', 'compact'], '--concurrency=$concurrency', @@ -699,9 +735,9 @@ class TestCommand extends Command with ArgUtils { // Don't pollute logs with output from tests that are expected to fail. if (expectFailure) '--reporter=name-only', - '--platform=${SupportedBrowsers.instance.supportedBrowserToPlatform[browser]}', + '--platform=${browserEnvironment.packageTestRuntime.identifier}', '--precompiled=${environment.webUiBuildDir.path}', - SupportedBrowsers.instance.browserToConfiguration[browser]!, + '--configuration=$configurationFilePath', '--', ...testFiles.map((f) => f.relativeToWebUi).toList(), ]; @@ -716,10 +752,10 @@ class TestCommand extends Command with ArgUtils { } hack.registerPlatformPlugin([ - SupportedBrowsers.instance.supportedBrowsersToRuntimes[browser]! + browserEnvironment.packageTestRuntime, ], () { return BrowserPlatform.start( - browserName: browser, + browserEnvironment: browserEnvironment, // It doesn't make sense to update a screenshot for a test that is // expected to fail. doUpdateScreenshotGoldens: !expectFailure && doUpdateScreenshotGoldens, From 32739bb1d710a5b026b0d7397e41b6a493715282 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Fri, 2 Jul 2021 10:53:41 -0600 Subject: [PATCH 2/2] address comments --- lib/web_ui/dev/browser.dart | 2 +- lib/web_ui/dev/chrome.dart | 2 +- lib/web_ui/dev/edge.dart | 2 +- lib/web_ui/dev/firefox.dart | 2 +- lib/web_ui/dev/safari_ios.dart | 4 ++-- lib/web_ui/dev/safari_macos.dart | 2 +- lib/web_ui/dev/test_platform.dart | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/dev/browser.dart b/lib/web_ui/dev/browser.dart index ed7a94d63e97d..a3d222be9be3a 100644 --- a/lib/web_ui/dev/browser.dart +++ b/lib/web_ui/dev/browser.dart @@ -198,6 +198,6 @@ abstract class ScreenshotManager { /// /// Example file names: /// - Chrome, no-suffix: backdrop_filter_clip_moved.actual.png - /// - iOS Safari: backdrop_filter_clip_moved.actual.iOS_Safari.png + /// - iOS Safari: backdrop_filter_clip_moved.iOS_Safari.actual.png String get filenameSuffix; } diff --git a/lib/web_ui/dev/chrome.dart b/lib/web_ui/dev/chrome.dart index 79012d2239e75..af0c671bb245e 100644 --- a/lib/web_ui/dev/chrome.dart +++ b/lib/web_ui/dev/chrome.dart @@ -42,7 +42,7 @@ class ChromeEnvironment implements BrowserEnvironment { String get packageTestConfigurationYamlFile => 'dart_test_chrome.yaml'; } -/// A class for running an instance of Chrome. +/// Runs desktop Chrome. /// /// 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 diff --git a/lib/web_ui/dev/edge.dart b/lib/web_ui/dev/edge.dart index 1acb270cdc03c..203b2736bf19f 100644 --- a/lib/web_ui/dev/edge.dart +++ b/lib/web_ui/dev/edge.dart @@ -33,7 +33,7 @@ class EdgeEnvironment implements BrowserEnvironment { String get packageTestConfigurationYamlFile => 'dart_test_edge.yaml'; } -/// A class for running an instance of Edge. +/// Runs desktop Edge. /// /// 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 diff --git a/lib/web_ui/dev/firefox.dart b/lib/web_ui/dev/firefox.dart index 156c89fecef76..2ad987bf2e2b3 100644 --- a/lib/web_ui/dev/firefox.dart +++ b/lib/web_ui/dev/firefox.dart @@ -38,7 +38,7 @@ class FirefoxEnvironment implements BrowserEnvironment { ScreenshotManager? getScreenshotManager() => null; } -/// A class for running an instance of Firefox. +/// Runs desktop Firefox. /// /// 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 diff --git a/lib/web_ui/dev/safari_ios.dart b/lib/web_ui/dev/safari_ios.dart index c1906ab3d621d..f12936c1c654a 100644 --- a/lib/web_ui/dev/safari_ios.dart +++ b/lib/web_ui/dev/safari_ios.dart @@ -42,7 +42,7 @@ class SafariIosEnvironment implements BrowserEnvironment { String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml'; } -/// A class for running an instance of Safari for iOS (i.e. mobile Safari). +/// Runs an instance of Safari for iOS (i.e. mobile 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 @@ -65,7 +65,7 @@ class SafariIos extends Browser { 'simctl', 'openurl', // Opens the url on Safari installed on the simulator. 'booted', // The simulator is already booted. - '${url.toString()}' + '${url.toString()}', ]); return process; diff --git a/lib/web_ui/dev/safari_macos.dart b/lib/web_ui/dev/safari_macos.dart index b8acf2526e30c..7579155789b3c 100644 --- a/lib/web_ui/dev/safari_macos.dart +++ b/lib/web_ui/dev/safari_macos.dart @@ -34,7 +34,7 @@ class SafariMacOsEnvironment implements BrowserEnvironment { String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml'; } -/// A class for running an instance of Safari for macOS (i.e. desktop Safari). +/// Runs an instance of Safari for macOS (i.e. desktop 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 diff --git a/lib/web_ui/dev/test_platform.dart b/lib/web_ui/dev/test_platform.dart index 946fb29026de3..c1f3a871deda5 100644 --- a/lib/web_ui/dev/test_platform.dart +++ b/lib/web_ui/dev/test_platform.dart @@ -485,7 +485,7 @@ class OneOffHandler { } } -/// A class that manages the connection to a single running browser. +/// Manages the connection to a single running browser. /// /// This is in charge of telling the browser which test suites to load and /// converting its responses into [Suite] objects.