Run integration tests on ci (#17598)

* changing felt script to fetch flutter

* changing the clone_flutter.sh code

* running integration tests with felt on cirrus. fetch framework in CI (not in local).

* only run cirrus tests on chrome. fix a comma in the flutter command path

* adding comments to public flags

* use local engine parameter for flutter pub get

* change flutter executable used for flutter drive command

* fix a cleanup issue. address comments. add toolException. enable web in flutter

* address reviwer comments. fix issue with local-engine

* address reviwer comments. fix issue with local-engine

* using engine/flutter/.dart_tools as clone directory. enabling clone script for local usage

* clean flutter repo with felt clean. add a flag to skip cloning the repo. always clone the repo even for local development, unless this flag is set

* fixing typos. updating readme for the new flag.

* fix directory error

* addressing reviewer comments

Author: Nurhan Turgut

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):
 
 ```

lib/web_ui/dev/clean.dart

@@ -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,
@@ -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.';
 
@@ -48,6 +55,8 @@ class CleanCommand extends Command<bool> with ArgUtils {
       ...fontFiles,
       if (_alsoCleanNinja)
         environment.outDir,
+      if(_alsoCleanFlutterRepo)
+        environment.engineDartToolDir,
     ];
 
     await Future.wait(

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;

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',
+  ));
+
 }

lib/web_ui/dev/felt.dart

@@ -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) {

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<bool> _runPubGet(String workingDirectory) async {
-    final String executable = isCirrus ? environment.pubExecutable : 'flutter';
-    final List<String> arguments = isCirrus
-        ? <String>[
-            'get',
-          ]
-        : <String>[
-            'pub',
-            'get',
-          ];
+  Future<void> _runPubGet(String workingDirectory) async {
+    if (!_useSystemFlutter) {
+      await _cloneFlutterRepo();
+      await _enableWeb(workingDirectory);
+    }
+    await runFlutter(workingDirectory, <String>['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<void> _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,
+      <String>[
+        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<void> _enableWeb(String workingDirectory) async {
+    await runFlutter(workingDirectory, <String>['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<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}',

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<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 '
+        '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<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) {
           bool integrationTestResult = await runIntegrationTests();
           bool unitTestResult = await runUnitTests();
           if (integrationTestResult != unitTestResult) {
@@ -134,11 +147,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 {
@@ -205,6 +218,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.
+  ///
+  /// 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');

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<String> evalProcess(
   return result.stdout as String;
 }
 
+Future<void> runFlutter(
+  String workingDirectory,
+  List<String> 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({