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 all 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
1 change: 1 addition & 0 deletions lib/web_ui/dev/felt.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ void main(List<String> args) async {
final bool result = await runner.run(args);
if (result == false) {
print('Sub-command returned false: `${args.join(' ')}`');
_cleanup();
io.exit(1);
}
} on UsageException catch (e) {
Expand Down
15 changes: 9 additions & 6 deletions lib/web_ui/dev/integration_tests_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import 'dart:io' as io;
import 'package:path/path.dart' as pathlib;
import 'package:web_driver_installer/chrome_driver_installer.dart';
import 'package:yaml/yaml.dart';

import 'chrome_installer.dart';
import 'common.dart';
Expand All @@ -21,8 +20,7 @@ class IntegrationTestsManager {
/// 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): change the web installers to install driver and the browser
// at the same time.
// TODO(nurhan): https://github.com/flutter/flutter/issues/53179.
final io.Directory _browserDriverDir;

/// This is the parent directory for all drivers.
Expand All @@ -32,10 +30,10 @@ class IntegrationTestsManager {
final io.Directory _drivers;

IntegrationTestsManager(this._browser)
: this._browserDriverDir = io.Directory(
pathlib.join(environment.webUiRootDir.path, 'drivers', _browser)),
: this._browserDriverDir = io.Directory(pathlib.join(
environment.webUiDartToolDir.path, 'drivers', _browser)),
this._drivers = io.Directory(
pathlib.join(environment.webUiRootDir.path, 'drivers'));
pathlib.join(environment.webUiDartToolDir.path, 'drivers'));

Future<bool> runTests() async {
if (_browser != 'chrome') {
Expand Down Expand Up @@ -77,6 +75,7 @@ class IntegrationTestsManager {
startProcess(
'./chromedriver/chromedriver',
['--port=4444'],
workingDirectory: io.Directory.current.path
);
print('INFO: Driver started');
}
Expand All @@ -89,12 +88,16 @@ class IntegrationTestsManager {
_browserDriverDir.createSync(recursive: true);
temporaryDirectories.add(_drivers);

io.Directory temp = io.Directory.current;
io.Directory.current = _browserDriverDir;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this done only to resolve ./chromedriver/chromedriver from the current directory? If so, I'd recommend calling startProcess(pathlib.join(_browserDriverDir.path, 'chromedriver', 'chromedriver')) with the same effect but without changing global program state. Same for workingDirectory.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately we also need it since the library we are importing from web_drivers also need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in other words this line is necessary for both line 98 and 99. Suggestion solution only considers line 99.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.


// TODO(nurhan): https://github.com/flutter/flutter/issues/53179
final String chromeDriverVersion = await queryChromeDriverVersion();
ChromeDriverInstaller chromeDriverInstaller =
ChromeDriverInstaller.withVersion(chromeDriverVersion);
await chromeDriverInstaller.install(alwaysInstall: true);
await _runDriver();
io.Directory.current = temp;
}

/// Runs all the web tests under e2e_tests/web.
Expand Down
35 changes: 24 additions & 11 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,18 @@ class TestCommand extends Command<bool> {
case TestTypesRequested.integration:
return runIntegrationTests();
case TestTypesRequested.all:
bool integrationTestResult = await runIntegrationTests();
bool unitTestResult = await runUnitTests();
if (integrationTestResult != unitTestResult) {
print('Tests run. Integration tests passed: $integrationTestResult '
'unit tests passed: $unitTestResult');
// TODO(nurhan): https://github.com/flutter/flutter/issues/53322
if (runAllTests) {
bool integrationTestResult = await runIntegrationTests();
bool unitTestResult = await runUnitTests();
if (integrationTestResult != unitTestResult) {
print('Tests run. Integration tests passed: $integrationTestResult '
'unit tests passed: $unitTestResult');
}
return integrationTestResult && unitTestResult;
} else {
return await runUnitTests();
}
return integrationTestResult && unitTestResult;
}
return false;
}
Expand All @@ -145,13 +150,11 @@ class TestCommand extends Command<bool> {
await _runPubGet();
}

final List<FilePath> targets =
this.targets.map((t) => FilePath.fromCwd(t)).toList();
await _buildTests(targets: targets);
if (targets.isEmpty) {
await _buildTests(targets: targetFiles);
if (runAllTests) {
await _runAllTests();
} else {
await _runTargetTests(targets);
await _runTargetTests(targetFiles);
}
return true;
}
Expand All @@ -165,6 +168,16 @@ class TestCommand extends Command<bool> {
/// Paths to targets to run, e.g. a single test.
List<String> get targets => argResults.rest;

/// The target test files to run.
///
/// The value can be null if the developer prefers to run all the tests.
List<FilePath> get targetFiles => (targets.isEmpty)
? null
: targets.map((t) => FilePath.fromCwd(t)).toList();

/// Whether all tests should run.
bool get runAllTests => targets.isEmpty;

String get browser => argResults['browser'];

bool get isChrome => argResults['browser'] == 'chrome';
Expand Down