Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
8 changes: 7 additions & 1 deletion lib/web_ui/dev/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

```
felt test --integration-tests-only --use-system-flutter
```

To run tests on Firefox (this will work only on a Linux device):

```
Expand Down
9 changes: 9 additions & 0 deletions lib/web_ui/dev/clean.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import 'utils.dart';
class CleanCommand extends Command<bool> with ArgUtils {
CleanCommand() {
argParser
..addFlag(
'flutter',
defaultsTo: true,
help: 'Cleans up the .dart_tool directory under engine/src/flutter.',
)
..addFlag(
'ninja',
defaultsTo: false,
Expand All @@ -27,6 +32,8 @@ class CleanCommand extends Command<bool> with ArgUtils {

bool get _alsoCleanNinja => boolArg('ninja');

bool get _alsoCleanFlutterRepo => boolArg('flutter');

@override
String get description => 'Deletes build caches and artifacts.';

Expand All @@ -48,6 +55,8 @@ class CleanCommand extends Command<bool> with ArgUtils {
...fontFiles,
if (_alsoCleanNinja)
environment.outDir,
if(_alsoCleanFlutterRepo)
environment.engineDartToolDir,
];

await Future.wait(
Expand Down
8 changes: 8 additions & 0 deletions lib/web_ui/dev/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
35 changes: 35 additions & 0 deletions lib/web_ui/dev/environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand All @@ -36,6 +37,7 @@ class Environment {
self: self,
webUiRootDir: webUiRootDir,
engineSrcDir: engineSrcDir,
engineToolsDir: engineToolsDir,
integrationTestsDir: integrationTestsDir,
outDir: outDir,
hostDebugUnoptDir: hostDebugUnoptDir,
Expand All @@ -47,6 +49,7 @@ class Environment {
this.self,
this.webUiRootDir,
this.engineSrcDir,
this.engineToolsDir,
this.integrationTestsDir,
this.outDir,
this.hostDebugUnoptDir,
Expand All @@ -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;

Expand Down Expand Up @@ -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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: example

io.Directory get engineDartToolDir => io.Directory(pathlib.join(
engineSrcDir.path,
'flutter',
'.dart_tool',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems useful for android and iOS e2e too. We currently have duplicated scripts to build AOT/JIT apps, but it would be a win if we can just reuse the tool logic.

));

/// Path to the "dev" directory containing engine developer tools and
/// configuration files.
io.Directory get webUiDevDir => io.Directory(pathlib.join(
Expand All @@ -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',
));

}
1 change: 0 additions & 1 deletion lib/web_ui/dev/felt.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ void main(List<String> 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) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

__

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

😊

Expand Down
81 changes: 63 additions & 18 deletions lib/web_ui/dev/integration_tests_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes

import 'environment.dart';
import 'exceptions.dart';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • [ ]
Suggested change
import 'exceptions.dart';
import 'exceptions.dart';```suggestion
import 'exceptions.dart';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes

import 'utils.dart';

class IntegrationTestsManager {
Expand All @@ -29,7 +29,9 @@ class IntegrationTestsManager {
/// tests shutdown.
final io.Directory _drivers;

IntegrationTestsManager(this._browser)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

final bool _useSystemFlutter;

IntegrationTestsManager(this._browser, this._useSystemFlutter)
: this._browserDriverDir = io.Directory(pathlib.join(
environment.webUiDartToolDir.path, 'drivers', _browser)),
this._drivers = io.Directory(
Expand All @@ -47,18 +49,19 @@ class IntegrationTestsManager {
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I


Future<bool> _runPubGet(String workingDirectory) async {
final String executable = isCirrus ? environment.pubExecutable : 'flutter';
final List<String> arguments = isCirrus
? <String>[
'get',
]
: <String>[
'pub',
'get',
];
if (!_useSystemFlutter) {
await _cloneFlutterRepo();
await _enableWeb(workingDirectory);
}
final String executable =
_useSystemFlutter ? 'flutter' : environment.flutterCommand.path;
final int exitCode = await runProcess(
executable,
arguments,
<String>[
'pub',
Comment thread
nturgut marked this conversation as resolved.
Outdated
'get',
'--local-engine=host_debug_unopt',
],
workingDirectory: workingDirectory,
);

Expand All @@ -71,12 +74,52 @@ class IntegrationTestsManager {
}
}

void _runDriver() async {
startProcess(
'./chromedriver/chromedriver',
['--port=4444'],
workingDirectory: io.Directory.current.path
/// 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Future<void> _cloneFlutterRepo() async {
// Delete directory if exists.
if(environment.engineDartToolDir.existsSync()) {
environment.engineDartToolDir.deleteSync();
}
environment.engineDartToolDir.createSync();

final int exitCode = await runProcess(
environment.cloneFlutterScript.path,
<String>[
environment.engineDartToolDir.path,
],
workingDirectory: environment.webUiRootDir.path,
);

if (exitCode != 0) {
throw ToolException('ERROR: Failed to clone flutter repo. Exited with '
'exit code $exitCode');
}
}

Future<void> _enableWeb(String workingDirectory) async {
final int exitCode = await runProcess(
environment.flutterCommand.path,
<String>[
'config',
'--local-engine=host_debug_unopt',
Comment thread
nturgut marked this conversation as resolved.
Outdated
'--enable-web',
],
workingDirectory: workingDirectory,
);

if (exitCode != 0) {
throw ToolException(
'ERROR: Failed to enable web. Exited with exit code $exitCode');
}
}

void _runDriver() async {
startProcess('./chromedriver/chromedriver', ['--port=4444'],
workingDirectory: io.Directory.current.path);
print('INFO: Driver started');
}

Expand Down Expand Up @@ -175,8 +218,10 @@ class IntegrationTestsManager {

Future<bool> _runTestsInProfileMode(
io.Directory directory, String testName) async {
final String executable =
_useSystemFlutter ? 'flutter' : environment.flutterCommand.path;
final int exitCode = await runProcess(
'flutter',
executable,
<String>[
'drive',
'--target=test_driver/${testName}',
Expand Down
25 changes: 21 additions & 4 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -56,6 +56,17 @@ class TestCommand extends Command<bool> 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 '
Comment thread
nturgut marked this conversation as resolved.
'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.'
)
..addFlag(
'update-screenshot-goldens',
defaultsTo: false,
Expand Down Expand Up @@ -117,7 +128,8 @@ class TestCommand extends Command<bool> 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) {
Comment thread
nturgut marked this conversation as resolved.
bool integrationTestResult = await runIntegrationTests();
bool unitTestResult = await runUnitTests();
if (integrationTestResult != unitTestResult) {
Expand All @@ -134,11 +146,11 @@ class TestCommand extends Command<bool> with ArgUtils {

Future<bool> 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<bool> runUnitTests() async {
Expand Down Expand Up @@ -205,6 +217,11 @@ class TestCommand extends Command<bool> with ArgUtils {
/// Whether [browser] is set to "chrome".
bool get isChrome => browser == 'chrome';

/// Use system flutter instead of cloning the repository.
Comment thread
nturgut marked this conversation as resolved.
///
/// Read the flag help for more details.
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');
Expand Down
28 changes: 21 additions & 7 deletions tools/clone_flutter.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Comment thread
nturgut marked this conversation as resolved.
exit 1
fi

Expand All @@ -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.
Comment thread
nturgut marked this conversation as resolved.
# 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
Expand Down