diff --git a/lib/web_ui/dev/README.md b/lib/web_ui/dev/README.md index c79dc1f3d6c46..dba654e49c54e 100644 --- a/lib/web_ui/dev/README.md +++ b/lib/web_ui/dev/README.md @@ -43,12 +43,18 @@ To run unit tests only: felt test --unit-tests-only ``` -To run integration tests only. For now these tests are only available on Chrome Desktop browsers. +To run integration tests only. For now these tests are only available on Chrome Desktop browsers. These tests will fetch the flutter repository for using `flutter drive` and `flutter pub get` commands. The repository will be synced to the youngest commit older than the engine commit. ``` felt test --integration-tests-only ``` +To skip cloning the flutter repository use the following flag. This flag can save internet bandwidth. However use with caution. Note the tests results will not be consistent with CIs when this flag is set. flutter command should be set in the PATH for this flag to be useful. This flag can also be used to test local Flutter changes. + +``` +felt test --integration-tests-only --use-system-flutter +``` + To run tests on Firefox (this will work only on a Linux device): ``` diff --git a/lib/web_ui/dev/clean.dart b/lib/web_ui/dev/clean.dart index ba2da00c857f6..b3d065fc66534 100644 --- a/lib/web_ui/dev/clean.dart +++ b/lib/web_ui/dev/clean.dart @@ -15,6 +15,11 @@ import 'utils.dart'; class CleanCommand extends Command with ArgUtils { CleanCommand() { argParser + ..addFlag( + 'flutter', + defaultsTo: true, + help: 'Cleans up the .dart_tool directory under engine/src/flutter.', + ) ..addFlag( 'ninja', defaultsTo: false, @@ -27,6 +32,8 @@ class CleanCommand extends Command with ArgUtils { bool get _alsoCleanNinja => boolArg('ninja'); + bool get _alsoCleanFlutterRepo => boolArg('flutter'); + @override String get description => 'Deletes build caches and artifacts.'; @@ -48,6 +55,8 @@ class CleanCommand extends Command with ArgUtils { ...fontFiles, if (_alsoCleanNinja) environment.outDir, + if(_alsoCleanFlutterRepo) + environment.engineDartToolDir, ]; await Future.wait( diff --git a/lib/web_ui/dev/common.dart b/lib/web_ui/dev/common.dart index 344316be76511..588fe67eedf6f 100644 --- a/lib/web_ui/dev/common.dart +++ b/lib/web_ui/dev/common.dart @@ -231,4 +231,12 @@ class DevNull implements StringSink { void writeln([Object obj = ""]) {} } +/// Whether the felt command is running on Cirrus CI. bool get isCirrus => io.Platform.environment['CIRRUS_CI'] == 'true'; + +/// Whether the felt command is running on LUCI. +bool get isLuci => io.Platform.environment['LUCI_CONTEXT'] != null; + +/// Whether the felt command is running on one of the Continuous Integration +/// environements. +bool get isCi => isCirrus || isLuci; diff --git a/lib/web_ui/dev/environment.dart b/lib/web_ui/dev/environment.dart index 3214c8a083b3d..824bded595267 100644 --- a/lib/web_ui/dev/environment.dart +++ b/lib/web_ui/dev/environment.dart @@ -20,6 +20,7 @@ class Environment { factory Environment() { final io.File self = io.File.fromUri(io.Platform.script); final io.Directory engineSrcDir = self.parent.parent.parent.parent.parent; + final io.Directory engineToolsDir = io.Directory(pathlib.join(engineSrcDir.path, 'flutter', 'tools')); final io.Directory outDir = io.Directory(pathlib.join(engineSrcDir.path, 'out')); final io.Directory hostDebugUnoptDir = io.Directory(pathlib.join(outDir.path, 'host_debug_unopt')); final io.Directory dartSdkDir = io.Directory(pathlib.join(hostDebugUnoptDir.path, 'dart-sdk')); @@ -36,6 +37,7 @@ class Environment { self: self, webUiRootDir: webUiRootDir, engineSrcDir: engineSrcDir, + engineToolsDir: engineToolsDir, integrationTestsDir: integrationTestsDir, outDir: outDir, hostDebugUnoptDir: hostDebugUnoptDir, @@ -47,6 +49,7 @@ class Environment { this.self, this.webUiRootDir, this.engineSrcDir, + this.engineToolsDir, this.integrationTestsDir, this.outDir, this.hostDebugUnoptDir, @@ -62,6 +65,9 @@ class Environment { /// Path to the engine's "src" directory. final io.Directory engineSrcDir; + /// Path to the engine's "tools" directory. + final io.Directory engineToolsDir; + /// Path to the web integration tests. final io.Directory integrationTestsDir; @@ -112,6 +118,16 @@ class Environment { '.dart_tool', )); + /// Path to the ".dart_tool" directory living under `engine/src/flutter`. + /// + /// This is a designated area for tool downloads which can be used by + /// multiple platforms. For exampe: Flutter repo for e2e tests. + io.Directory get engineDartToolDir => io.Directory(pathlib.join( + engineSrcDir.path, + 'flutter', + '.dart_tool', + )); + /// Path to the "dev" directory containing engine developer tools and /// configuration files. io.Directory get webUiDevDir => io.Directory(pathlib.join( @@ -124,4 +140,23 @@ class Environment { webUiDartToolDir.path, 'goldens', )); + + /// Path to the script that clones the Flutter repo. + io.File get cloneFlutterScript => io.File(pathlib.join( + engineToolsDir.path, + 'clone_flutter.sh', + )); + + /// Path to flutter. + /// + /// For example, this can be used to run `flutter pub get`. + /// + /// Only use [cloneFlutterScript] to clone flutter to the engine build. + io.File get flutterCommand => io.File(pathlib.join( + engineDartToolDir.path, + 'flutter', + 'bin', + 'flutter', + )); + } diff --git a/lib/web_ui/dev/felt.dart b/lib/web_ui/dev/felt.dart index 32f3619dc7014..6366b6152991a 100644 --- a/lib/web_ui/dev/felt.dart +++ b/lib/web_ui/dev/felt.dart @@ -37,7 +37,6 @@ void main(List args) async { final bool result = (await runner.run(args)) as bool; if (result == false) { print('Sub-command returned false: `${args.join(' ')}`'); - await cleanup(); exitCode = 1; } } on UsageException catch (e) { diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 811e876664541..ec3595dedf268 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -7,8 +7,8 @@ import 'package:path/path.dart' as pathlib; import 'package:web_driver_installer/chrome_driver_installer.dart'; import 'chrome_installer.dart'; -import 'common.dart'; import 'environment.dart'; +import 'exceptions.dart'; import 'utils.dart'; class IntegrationTestsManager { @@ -29,7 +29,9 @@ class IntegrationTestsManager { /// tests shutdown. final io.Directory _drivers; - IntegrationTestsManager(this._browser) + final bool _useSystemFlutter; + + IntegrationTestsManager(this._browser, this._useSystemFlutter) : this._browserDriverDir = io.Directory(pathlib.join( environment.webUiDartToolDir.path, 'drivers', _browser)), this._drivers = io.Directory( @@ -46,37 +48,49 @@ class IntegrationTestsManager { } } - Future _runPubGet(String workingDirectory) async { - final String executable = isCirrus ? environment.pubExecutable : 'flutter'; - final List arguments = isCirrus - ? [ - 'get', - ] - : [ - 'pub', - 'get', - ]; + Future _runPubGet(String workingDirectory) async { + if (!_useSystemFlutter) { + await _cloneFlutterRepo(); + await _enableWeb(workingDirectory); + } + await runFlutter(workingDirectory, ['pub', 'get'], + useSystemFlutter: _useSystemFlutter); + } + + /// Clone flutter repository, use the youngest commit older than the engine + /// commit. + /// + /// Use engine/src/flutter/.dart_tools to clone the Flutter repo. + /// TODO(nurhan): Use git pull instead if repo exists. + Future _cloneFlutterRepo() async { + // Delete directory if exists. + if (environment.engineDartToolDir.existsSync()) { + environment.engineDartToolDir.deleteSync(); + } + environment.engineDartToolDir.createSync(); + final int exitCode = await runProcess( - executable, - arguments, - workingDirectory: workingDirectory, + environment.cloneFlutterScript.path, + [ + environment.engineDartToolDir.path, + ], + workingDirectory: environment.webUiRootDir.path, ); if (exitCode != 0) { - io.stderr.writeln( - 'ERROR: Failed to run pub get. Exited with exit code $exitCode'); - return false; - } else { - return true; + throw ToolException('ERROR: Failed to clone flutter repo. Exited with ' + 'exit code $exitCode'); } } + Future _enableWeb(String workingDirectory) async { + await runFlutter(workingDirectory, ['config', '--enable-web'], + useSystemFlutter: _useSystemFlutter); + } + void _runDriver() async { - startProcess( - './chromedriver/chromedriver', - ['--port=4444'], - workingDirectory: io.Directory.current.path - ); + startProcess('./chromedriver/chromedriver', ['--port=4444'], + workingDirectory: io.Directory.current.path); print('INFO: Driver started'); } @@ -175,8 +189,10 @@ class IntegrationTestsManager { Future _runTestsInProfileMode( io.Directory directory, String testName) async { + final String executable = + _useSystemFlutter ? 'flutter' : environment.flutterCommand.path; final int exitCode = await runProcess( - 'flutter', + executable, [ 'drive', '--target=test_driver/${testName}', diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 5b62a064d7b8d..598f85d9fb717 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -15,11 +15,11 @@ import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_im import 'package:test_core/src/executable.dart' as test; // ignore: implementation_imports +import 'environment.dart'; import 'exceptions.dart'; import 'integration_tests_manager.dart'; import 'supported_browsers.dart'; import 'test_platform.dart'; -import 'environment.dart'; import 'utils.dart'; /// The type of tests requested by the tool user. @@ -56,6 +56,18 @@ class TestCommand extends Command with ArgUtils { 'at the same time. If this flag is set, only run the integration ' 'tests.', ) + ..addFlag('use-system-flutter', + defaultsTo: false, + help: 'integration tests are using flutter repository for various tasks' + ', such as flutter drive, flutter pub get. If this flag is set, felt ' + 'will use flutter command without cloning the repository. This flag ' + 'can save internet bandwidth. However use with caution. Note that ' + 'since flutter repo is always synced to youngest commit older than ' + 'the engine commit for the tests running in CI, the tests results ' + 'won\'t be consistent with CIs when this flag is set. flutter ' + 'command should be set in the PATH for this flag to be useful.' + 'This flag can also be used to test local Flutter changes.' + ) ..addFlag( 'update-screenshot-goldens', defaultsTo: false, @@ -117,7 +129,8 @@ class TestCommand extends Command with ArgUtils { return runIntegrationTests(); case TestTypesRequested.all: // TODO(nurhan): https://github.com/flutter/flutter/issues/53322 - if (runAllTests) { + // TODO(nurhan): Expand browser matrix for felt integration tests. + if (runAllTests && isChrome) { bool integrationTestResult = await runIntegrationTests(); bool unitTestResult = await runUnitTests(); if (integrationTestResult != unitTestResult) { @@ -134,11 +147,11 @@ 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 || isCirrus) { + if (io.Platform.environment['LUCI_CONTEXT'] != null) { return true; } - return IntegrationTestsManager(browser).runTests(); + return IntegrationTestsManager(browser, useSystemFlutter).runTests(); } Future runUnitTests() async { @@ -205,6 +218,11 @@ class TestCommand extends Command with ArgUtils { /// Whether [browser] is set to "chrome". bool get isChrome => browser == 'chrome'; + /// Use system flutter instead of cloning the repository. + /// + /// Read the flag help for more details. Uses PATH to locate flutter. + bool get useSystemFlutter => boolArg('use-system-flutter'); + /// When running screenshot tests writes them to the file system into /// ".dart_tool/goldens". bool get doUpdateScreenshotGoldens => boolArg('update-screenshot-goldens'); diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index bf3475e8874fc..2ccdb04c5f049 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -11,6 +11,7 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'environment.dart'; +import 'exceptions.dart'; class FilePath { FilePath.fromCwd(String relativePath) @@ -107,6 +108,26 @@ Future evalProcess( return result.stdout as String; } +Future runFlutter( + String workingDirectory, + List arguments, { + bool useSystemFlutter = false, +}) async { + final String executable = + useSystemFlutter ? 'flutter' : environment.flutterCommand.path; + arguments.add('--local-engine=host_debug_unopt'); + final int exitCode = await runProcess( + executable, + arguments, + workingDirectory: workingDirectory, + ); + + if (exitCode != 0) { + throw ToolException('ERROR: Failed to run $executable with ' + 'arguments ${arguments.toString()}. Exited with exit code $exitCode'); + } +} + @immutable class ProcessException implements Exception { ProcessException({ diff --git a/tools/clone_flutter.sh b/tools/clone_flutter.sh index cb9f4916bc965..48debdf44475c 100755 --- a/tools/clone_flutter.sh +++ b/tools/clone_flutter.sh @@ -4,13 +4,12 @@ set -x if [[ "$CIRRUS_CI" = false || -z $CIRRUS_CI ]] then - echo "This script is aimed to be run on CI environments. Do not run locally." - exit 1 + echo "Cloning Flutter repo to local machine." fi if [[ -z $ENGINE_PATH ]] then - echo "Engine path should be set to run the script." + echo "Please set ENGINE_PATH environment variable." exit 1 fi @@ -33,10 +32,25 @@ fi LATEST_COMMIT_TIME_ENGINE=`git log -1 --date=local --format="%cd"` echo "Latest commit time on engine found as $LATEST_COMMIT_TIME_ENGINE" -# Do rest of the task in the root directory -cd ~ -mkdir -p $FRAMEWORK_PATH -cd $FRAMEWORK_PATH +# Check if there is an argument added for repo location. +# If not use the location that should be set by Cirrus/LUCI. +FLUTTER_CLONE_REPO_PATH=$1 + +if [[ -z $FLUTTER_CLONE_REPO_PATH ]] +then + if [[ -z $FRAMEWORK_PATH ]] + then + echo "Framework path should be set to run the script." + exit 1 + fi + # Do rest of the task in the root directory + cd ~ + mkdir -p $FRAMEWORK_PATH + cd $FRAMEWORK_PATH +else + cd $FLUTTER_CLONE_REPO_PATH +fi + # Clone the Flutter Framework. git clone https://github.com/flutter/flutter.git cd flutter