Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Enable firefox integration tests #19164

Merged
nturgut merged 2 commits into from
Jun 22, 2020
Merged

Enable firefox integration tests #19164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d75cc7
firefox integration tests enable
Jun 19, 2020
7c9ad2d
addressing reviewer comments
Jun 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/web_ui/dev/browser_lock.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ ios-safari:
majorVersion: 13
minorVersion: 5
device: 'iPhone 11'
## geckodriver is used for testing Firefox Browser. It works with multiple
## Firefox Browser versions.
## See: https://github.com/mozilla/geckodriver/releases
geckodriver: 'v0.26.0'
76 changes: 72 additions & 4 deletions lib/web_ui/dev/driver_manager.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import 'dart:io' as io;
import 'package:meta/meta.dart';
import 'package:path/path.dart' as pathlib;
import 'package:web_driver_installer/chrome_driver_installer.dart';
import 'package:web_driver_installer/firefox_driver_installer.dart';
import 'package:web_driver_installer/safari_driver_runner.dart';
import 'package:yaml/yaml.dart';

import 'chrome_installer.dart';
import 'common.dart';
Expand All @@ -16,10 +18,11 @@ import 'utils.dart';

/// [DriverManager] implementation for Chrome.
///
/// This manager can be used for both MacOS and Linux.
/// This manager can be used for both macOS and Linux.
class ChromeDriverManager extends DriverManager {
ChromeDriverManager(String browser) : super(browser);

@override
Future<void> _installDriver() async {
if (_browserDriverDir.existsSync()) {
_browserDriverDir.deleteSync(recursive: true);
Expand All @@ -45,6 +48,7 @@ class ChromeDriverManager extends DriverManager {
/// Throw an error if driver directory does not exists.
///
/// Driver should already exist on LUCI as a CIPD package.
@override
Future<void> _verifyDriverForLUCI() {
if (!_browserDriverDir.existsSync()) {
throw StateError('Failed to locate Chrome driver on LUCI on path:'
Expand All @@ -53,31 +57,93 @@ class ChromeDriverManager extends DriverManager {
return Future<void>.value();
}

@override
Future<void> _startDriver(String driverPath) async {
await startProcess('./chromedriver/chromedriver', ['--port=4444'],
workingDirectory: driverPath);
print('INFO: Driver started');
}
}

/// [DriverManager] implementation for Firefox.
///
/// This manager can be used for both macOS and Linux.
class FirefoxDriverManager extends DriverManager {
FirefoxDriverManager(String browser) : super(browser);

FirefoxDriverInstaller firefoxDriverInstaller =
FirefoxDriverInstaller(geckoDriverVersion: getLockedGeckoDriverVersion());

@override
Future<void> _installDriver() async {
if (_browserDriverDir.existsSync()) {
_browserDriverDir.deleteSync(recursive: true);
}

_browserDriverDir.createSync(recursive: true);
temporaryDirectories.add(_drivers);

final io.Directory temp = io.Directory.current;
io.Directory.current = _browserDriverDir;

try {
await firefoxDriverInstaller.install(alwaysInstall: false);
} finally {
io.Directory.current = temp;
}
}

/// Throw an error if driver directory does not exist.
///
/// Driver should already exist on LUCI as a CIPD package.
@override
Future<void> _verifyDriverForLUCI() {
if (!_browserDriverDir.existsSync()) {
throw StateError('Failed to locate Firefox driver on LUCI on path:'
'${_browserDriverDir.path}');
}
return Future<void>.value();
Copy link
Contributor

@yjbanov yjbanov Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why return asynchronously?

Copy link
Contributor Author

@nturgut nturgut Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is overriding the abstract class' method. I added @override everywhere to make it more clear.

In this class we always return return Future<void>.value(); value. For example line 143.

}

@override
Future<void> _startDriver(String driverPath) async {
await startProcess('./firefoxdriver/geckodriver', ['--port=4444'],
workingDirectory: driverPath);
print('INFO: Driver started');
}

/// Get the geckodriver version to be used with [FirefoxDriverInstaller].
///
/// For different versions of geckodriver. See:
/// https://github.com/mozilla/geckodriver/releases
static String getLockedGeckoDriverVersion() {
final YamlMap browserLock = BrowserLock.instance.configuration;
String geckoDriverReleaseVersion = browserLock['geckodriver'] as String;
return geckoDriverReleaseVersion;
}
}

/// [DriverManager] implementation for Safari.
///
/// This manager is will only be created/used for MacOS.
/// This manager is will only be created/used for macOS.
class SafariDriverManager extends DriverManager {
SafariDriverManager(String browser) : super(browser);

@override
Future<void> _installDriver() {
// No-op.
// macOS comes with Safari Driver installed.
return new Future<void>.value();
}

@override
Future<void> _verifyDriverForLUCI() {
// No-op.
// macOS comes with Safari Driver installed.
return Future<void>.value();
}

@override
Future<void> _startDriver(String driverPath) async {
final SafariDriverRunner safariDriverRunner = SafariDriverRunner();

Expand Down Expand Up @@ -137,11 +203,13 @@ abstract class DriverManager {
static DriverManager chooseDriver(String browser) {
if (browser == 'chrome') {
return ChromeDriverManager(browser);
} else if (browser == 'firefox') {
return FirefoxDriverManager(browser);
} else if (browser == 'safari' && io.Platform.isMacOS) {
return SafariDriverManager(browser);
} else {
throw StateError('Integration tests are only supported on Chrome or '
'on Safari (running on MacOS)');
throw StateError('Integration tests are only supported on Firefox, Chrome'
' and on Safari (running on macOS)');
}
}
}
38 changes: 35 additions & 3 deletions lib/web_ui/dev/integration_tests_manager.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import 'exceptions.dart';
import 'common.dart';
import 'utils.dart';

const String _unsupportedConfigurationWarning = 'WARNING: integration '
'tests are only supported on Chrome or on Safari (running on MacOS)';
const String _unsupportedConfigurationWarning = 'WARNING: integration tests '
'are only supported on Chrome, Firefox and on Safari (running on macOS)';

class IntegrationTestsManager {
final String _browser;
Expand Down Expand Up @@ -285,10 +285,13 @@ class IntegrationTestsManager {
/// Validate the given `browser`, `platform` combination is suitable for
/// integration tests to run.
bool validateIfTestsShouldRun() {
// Chrome tests should run at all Platforms (Linux, MacOS, Windows).
// Chrome tests should run at all Platforms (Linux, macOS, Windows).
// They can also run successfully on CI and local.
if (_browser == 'chrome') {
return true;
} else if (_browser == 'firefox' &&
(io.Platform.isLinux || io.Platform.isMacOS)) {
return true;
} else if (_browser == 'safari' && io.Platform.isMacOS && !isLuci) {
return true;
} else {
Expand All @@ -306,6 +309,8 @@ abstract class IntegrationArguments {
factory IntegrationArguments.fromBrowser(String browser) {
if (browser == 'chrome') {
return ChromeIntegrationArguments();
} else if (browser == 'firefox') {
return FirefoxIntegrationArguments();
} else if (browser == 'safari' && io.Platform.isMacOS) {
return SafariIntegrationArguments();
} else {
Expand Down Expand Up @@ -346,6 +351,25 @@ class ChromeIntegrationArguments extends IntegrationArguments {
}
}

/// Arguments to give `flutter drive` to run the integration tests on Firefox.
class FirefoxIntegrationArguments extends IntegrationArguments {
List<String> getTestArguments(String testName, String mode) {
return <String>[
'drive',
'--target=test_driver/${testName}',
'-d',
'web-server',
'--$mode',
'--browser-name=firefox',
'--headless',
'--local-engine=host_debug_unopt',
];
}

String getCommandToRun(String testName, String mode) =>
'flutter ${getTestArguments(testName, mode).join(' ')}';
}

/// Arguments to give `flutter drive` to run the integration tests on Safari.
class SafariIntegrationArguments extends IntegrationArguments {
SafariIntegrationArguments();
Expand Down Expand Up @@ -397,4 +421,12 @@ const Map<String, List<String>> blockedTestsListsMap = <String, List<String>>{
'target_platform_android_e2e.dart',
'image_loading_e2e.dart',
],
'firefox-linux': [
'target_platform_ios_e2e.dart',
'target_platform_macos_e2e.dart',
],
'firefox-macos': [
'target_platform_android_e2e.dart',
'target_platform_ios_e2e.dart',
],
};
7 changes: 5 additions & 2 deletions lib/web_ui/dev/test_runner.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class TestCommand extends Command<bool> with ArgUtils {
print('Running the unit tests only');
return TestTypesRequested.unit;
} else if (boolArg('integration-tests-only')) {
if (!isChrome && !isSafariOnMacOS) {
if (!isChrome && !isSafariOnMacOS && !isFirefox) {
throw UnimplementedError(
'Integration tests are only available on Chrome Desktop for now');
}
Expand All @@ -132,7 +132,7 @@ class TestCommand extends Command<bool> with ArgUtils {
case TestTypesRequested.all:
// TODO(nurhan): https://github.com/flutter/flutter/issues/53322
// TODO(nurhan): Expand browser matrix for felt integration tests.
if (runAllTests && (isChrome || isSafariOnMacOS)) {
if (runAllTests && (isChrome || isSafariOnMacOS || isFirefox)) {
bool unitTestResult = await runUnitTests();
bool integrationTestResult = await runIntegrationTests();
if (integrationTestResult != unitTestResult) {
Expand Down Expand Up @@ -263,6 +263,9 @@ class TestCommand extends Command<bool> with ArgUtils {
/// Whether [browser] is set to "chrome".
bool get isChrome => browser == 'chrome';

/// Whether [browser] is set to "firefox".
bool get isFirefox => browser == 'firefox';

/// Whether [browser] is set to "safari".
bool get isSafariOnMacOS => browser == 'safari'
&& io.Platform.isMacOS;
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ dev_dependencies:
git:
url: git://github.com/flutter/web_installers.git
path: packages/web_drivers/
ref: 41f96bb55d2f064dac3c9fc727ebdf4b0cdf79c4
ref: 1cea0d79cad1ebc217c4bcbeba1be41470674a49