diff --git a/lib/web_ui/dev/browser_lock.yaml b/lib/web_ui/dev/browser_lock.yaml index 535847dde9c..f83847d484b 100644 --- a/lib/web_ui/dev/browser_lock.yaml +++ b/lib/web_ui/dev/browser_lock.yaml @@ -1,7 +1,9 @@ chrome: # It seems Chrome can't always release from the same build for all operating # systems, so we specify per-OS build number. - Linux: 753189 + # Note: 741412 binary is for Chrome Version 82. Driver for Chrome version 83 + # is not working with chrome.binary option. + Linux: 741412 Mac: 735194 Win: 735105 firefox: diff --git a/lib/web_ui/dev/chrome_installer.dart b/lib/web_ui/dev/chrome_installer.dart index 48e10def23f..6f46c2a57d9 100644 --- a/lib/web_ui/dev/chrome_installer.dart +++ b/lib/web_ui/dev/chrome_installer.dart @@ -22,19 +22,20 @@ class ChromeArgParser extends BrowserArgParser { static ChromeArgParser get instance => _singletonInstance; String _version; + int _pinnedChromeBuildNumber; ChromeArgParser._(); @override void populateOptions(ArgParser argParser) { final YamlMap browserLock = BrowserLock.instance.configuration; - final int pinnedChromeVersion = + _pinnedChromeBuildNumber = PlatformBinding.instance.getChromeBuild(browserLock); argParser ..addOption( 'chrome-version', - defaultsTo: '$pinnedChromeVersion', + defaultsTo: '$pinnedChromeBuildNumber', help: 'The Chrome version to use while running tests. If the requested ' 'version has not been installed, it will be downloaded and installed ' 'automatically. A specific Chrome build version number, such as 695653, ' @@ -51,6 +52,8 @@ class ChromeArgParser extends BrowserArgParser { @override String get version => _version; + + String get pinnedChromeBuildNumber => _pinnedChromeBuildNumber.toString(); } /// Returns the installation of Chrome, installing it if necessary. @@ -177,11 +180,22 @@ class ChromeInstaller { } Future install() async { - if (versionDir.existsSync()) { + if (versionDir.existsSync() && !isLuci) { versionDir.deleteSync(recursive: true); + versionDir.createSync(recursive: true); + } else if (versionDir.existsSync() && isLuci) { + print('INFO: Chrome version directory in LUCI: ' + '${versionDir.path}'); + } else if(!versionDir.existsSync() && isLuci) { + // Chrome should have been deployed as a CIPD package on LUCI. + // Throw if it does not exists. + throw StateError('Failed to locate Chrome on LUCI on path:' + '${versionDir.path}'); + } else { + // If the directory does not exists and felt is not running on LUCI. + versionDir.createSync(recursive: true); } - versionDir.createSync(recursive: true); final String url = PlatformBinding.instance.getChromeDownloadUrl(version); final StreamedResponse download = await client.send(Request( 'GET', @@ -241,9 +255,32 @@ Future queryChromeDriverVersion() async { return chromeDriverVersion; } +/// Make sure LUCI bot has the pinned Chrome version and return the executable. +/// +/// We are using CIPD packages in LUCI. The pinned chrome version from the +/// `browser_lock.yaml` file will already be installed in the LUCI bot. +/// Verify if Chrome is installed and use it for the integration tests. +String preinstalledChromeExecutable() { + // Note that build number and major version is different for Chrome. + // For example for a build number `753189`, major version is 83. + final String buildNumber = ChromeArgParser.instance.pinnedChromeBuildNumber; + final ChromeInstaller chromeInstaller = ChromeInstaller(version: buildNumber); + if (chromeInstaller.isInstalled) { + print('INFO: Found chrome executable for LUCI: ' + '${chromeInstaller.getInstallation().executable}'); + return chromeInstaller.getInstallation().executable; + } else { + throw StateError( + 'Failed to locate pinned Chrome build: $buildNumber on LUCI.'); + } +} + Future _querySystemChromeMajorVersion() async { String chromeExecutable = ''; - if (io.Platform.isLinux) { + // LUCI uses the Chrome from CIPD packages. + if (isLuci) { + chromeExecutable = preinstalledChromeExecutable(); + } else if (io.Platform.isLinux) { chromeExecutable = 'google-chrome'; } else if (io.Platform.isMacOS) { chromeExecutable = await _findChromeExecutableOnMac(); diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index ec3595dedf2..04e7da3e4be 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -9,6 +9,7 @@ import 'package:web_driver_installer/chrome_driver_installer.dart'; import 'chrome_installer.dart'; import 'environment.dart'; import 'exceptions.dart'; +import 'common.dart'; import 'utils.dart'; class IntegrationTestsManager { @@ -20,7 +21,8 @@ class IntegrationTestsManager { /// It usually changes with each the browser version changes. /// A better solution would be installing the browser and the driver at the /// same time. - // TODO(nurhan): https://github.com/flutter/flutter/issues/53179. + // TODO(nurhan): https://github.com/flutter/flutter/issues/53179. Partly + // solved. Remaining local integration tests using the locked Chrome version. final io.Directory _browserDriverDir; /// This is the parent directory for all drivers. @@ -33,7 +35,10 @@ class IntegrationTestsManager { IntegrationTestsManager(this._browser, this._useSystemFlutter) : this._browserDriverDir = io.Directory(pathlib.join( - environment.webUiDartToolDir.path, 'drivers', _browser)), + environment.webUiDartToolDir.path, + 'drivers', + _browser, + '${_browser}driver-${io.Platform.operatingSystem.toString()}')), this._drivers = io.Directory( pathlib.join(environment.webUiDartToolDir.path, 'drivers')); @@ -42,7 +47,13 @@ class IntegrationTestsManager { print('WARNING: integration tests are only supported on chrome for now'); return false; } else { - await prepareDriver(); + if (!isLuci) { + // LUCI installs driver from CIPD, so we skip installing it on LUCI. + await _prepareDriver(); + } else { + await _verifyDriverForLUCI(); + } + await _startDriver(_browserDriverDir.path); // TODO(nurhan): https://github.com/flutter/flutter/issues/52987 return await _runTests(); } @@ -65,7 +76,7 @@ class IntegrationTestsManager { Future _cloneFlutterRepo() async { // Delete directory if exists. if (environment.engineDartToolDir.existsSync()) { - environment.engineDartToolDir.deleteSync(); + environment.engineDartToolDir.deleteSync(recursive: true); } environment.engineDartToolDir.createSync(); @@ -88,13 +99,23 @@ class IntegrationTestsManager { useSystemFlutter: _useSystemFlutter); } - void _runDriver() async { - startProcess('./chromedriver/chromedriver', ['--port=4444'], - workingDirectory: io.Directory.current.path); + /// Driver should already exist on LUCI as a CIPD package. + /// + /// Throw an error if directory does not exists. + void _verifyDriverForLUCI() { + if (!_browserDriverDir.existsSync()) { + throw StateError('Failed to locate Chrome driver on LUCI on path:' + '${_browserDriverDir.path}'); + } + } + + void _startDriver(String workingDirectory) async { + await startProcess('./chromedriver/chromedriver', ['--port=4444'], + workingDirectory: workingDirectory); print('INFO: Driver started'); } - void prepareDriver() async { + void _prepareDriver() async { if (_browserDriverDir.existsSync()) { _browserDriverDir.deleteSync(recursive: true); } @@ -110,7 +131,6 @@ class IntegrationTestsManager { ChromeDriverInstaller chromeDriverInstaller = ChromeDriverInstaller.withVersion(chromeDriverVersion); await chromeDriverInstaller.install(alwaysInstall: true); - await _runDriver(); io.Directory.current = temp; } @@ -153,6 +173,8 @@ class IntegrationTestsManager { .toList(); final List e2eTestsToRun = List(); + final List blackList = + blackListsMap[getBlackListMapKey(_browser)] ?? []; // The following loops over the contents of the directory and saves an // expected driver file name for each e2e test assuming any dart file @@ -162,7 +184,13 @@ class IntegrationTestsManager { for (io.File f in entities) { final String basename = pathlib.basename(f.path); if (!basename.contains('_test.dart') && basename.endsWith('.dart')) { - e2eTestsToRun.add(basename); + // Do not add the basename if it is in the blacklist. + if (!blackList.contains(basename)) { + e2eTestsToRun.add(basename); + } else { + print('INFO: Test $basename is skipped since it is blacklisted for ' + '${getBlackListMapKey(_browser)}'); + } } } @@ -200,15 +228,21 @@ class IntegrationTestsManager { 'web-server', '--profile', '--browser-name=$_browser', + if (isLuci) '--chrome-binary=${preinstalledChromeExecutable()}', + if (isLuci) '--headless', '--local-engine=host_debug_unopt', ], workingDirectory: directory.path, ); if (exitCode != 0) { - final String statementToRun = 'flutter drive ' + String statementToRun = 'flutter drive ' '--target=test_driver/${testName} -d web-server --profile ' '--browser-name=$_browser --local-engine=host_debug_unopt'; + if (isLuci) { + statementToRun = '$statementToRun --chrome-binary=' + '${preinstalledChromeExecutable()}'; + } io.stderr .writeln('ERROR: Failed to run test. Exited with exit code $exitCode' '. Statement to run $testName locally use the following ' @@ -321,3 +355,32 @@ class IntegrationTestsManager { } } } + +/// Prepares a key for the [blackList] map. +/// +/// Uses the browser name and the operating system name. +String getBlackListMapKey(String browser) => + '${browser}-${io.Platform.operatingSystem}'; + +/// Tests that should be skipped run for a specific platform-browser +/// combination. +/// +/// These tests might be failing or might have been implemented for a specific +/// configuration. +/// +/// For example a mobile browser only tests will be added to blacklist for +/// `chrome-linux`, `safari-macos` and `chrome-macos`. +/// +/// Note that integration tests are only running on chrome for now. +const Map> blackListsMap = >{ + 'chrome-linux': [ + 'target_platform_ios_e2e.dart', + 'target_platform_macos_e2e.dart', + 'target_platform_android_e2e.dart', + ], + 'chrome-macos': [ + 'target_platform_ios_e2e.dart', + 'target_platform_macos_e2e.dart', + 'target_platform_android_e2e.dart', + ], +}; diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 30ffcfea693..41f23d75c92 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -131,8 +131,8 @@ class TestCommand extends Command with ArgUtils { // TODO(nurhan): https://github.com/flutter/flutter/issues/53322 // TODO(nurhan): Expand browser matrix for felt integration tests. if (runAllTests && isChrome) { - bool integrationTestResult = await runIntegrationTests(); bool unitTestResult = await runUnitTests(); + bool integrationTestResult = await runIntegrationTests(); if (integrationTestResult != unitTestResult) { print('Tests run. Integration tests passed: $integrationTestResult ' 'unit tests passed: $unitTestResult'); @@ -146,11 +146,6 @@ class TestCommand extends Command with ArgUtils { } Future runIntegrationTests() async { - // TODO(nurhan): https://github.com/flutter/flutter/issues/52983 - if (io.Platform.environment['LUCI_CONTEXT'] != null) { - return true; - } - return IntegrationTestsManager(browser, useSystemFlutter).runTests(); }