Run integration tests on luci, use cipd package for Chrome (#18180)
* try to run integration tests on luci
* adds extra logs to check if chrome is on lUCI
* use the cipd chrome package for integration tests in LUCI
* add driver version for chrome 83.(cipd package has 83 now)
* no headless mode didn't worked on the bots. try the other option
* On LUCI also use chrome driver as a cipd package
* change the directory name to fit the cpid package
* adding blacklist functionality
* remove logs added for troubleshooting. remove the check that blocks int-test runs on LUCI
* add more comments to blacklists
* also use CIPD package chrome for unit tests
* addressing reviewer comments
* run integration tests first, upon reviewer request
* fix bug. keep running integration tests after unit tests
* update the logs for LUCI
* fix todo comments
diff --git a/lib/web_ui/dev/browser_lock.yaml b/lib/web_ui/dev/browser_lock.yaml
index 535847d..f83847d 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 48e10de..6f46c2a 100644
--- a/lib/web_ui/dev/chrome_installer.dart
+++ b/lib/web_ui/dev/chrome_installer.dart
@@ -22,19 +22,20 @@
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 @@
@override
String get version => _version;
+
+ String get pinnedChromeBuildNumber => _pinnedChromeBuildNumber.toString();
}
/// Returns the installation of Chrome, installing it if necessary.
@@ -177,11 +180,22 @@
}
Future<void> 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 @@
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<int> _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 ec3595d..04e7da3 100644
--- a/lib/web_ui/dev/integration_tests_manager.dart
+++ b/lib/web_ui/dev/integration_tests_manager.dart
@@ -9,6 +9,7 @@
import 'chrome_installer.dart';
import 'environment.dart';
import 'exceptions.dart';
+import 'common.dart';
import 'utils.dart';
class IntegrationTestsManager {
@@ -20,7 +21,8 @@
/// 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 @@
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 @@
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 @@
Future<void> _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 @@
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 @@
ChromeDriverInstaller chromeDriverInstaller =
ChromeDriverInstaller.withVersion(chromeDriverVersion);
await chromeDriverInstaller.install(alwaysInstall: true);
- await _runDriver();
io.Directory.current = temp;
}
@@ -153,6 +173,8 @@
.toList();
final List<String> e2eTestsToRun = List<String>();
+ final List<String> blackList =
+ blackListsMap[getBlackListMapKey(_browser)] ?? <String>[];
// 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 @@
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 @@
'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 @@
}
}
}
+
+/// 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<String, List<String>> blackListsMap = <String, List<String>>{
+ '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 30ffcfe..41f23d7 100644
--- a/lib/web_ui/dev/test_runner.dart
+++ b/lib/web_ui/dev/test_runner.dart
@@ -131,8 +131,8 @@
// 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 @@
}
Future<bool> 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();
}
