From 40848e0f894cf991b7332b8c2d055fbafd170c82 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 2 Nov 2020 17:30:26 -0800 Subject: [PATCH 01/16] testing running the tests on all build modes --- .../regular_integration_tests/pubspec.yaml | 2 +- .../platform_messages_integration.dart | 2 +- .../test_driver/text_editing_integration.dart | 28 +++++++------ lib/web_ui/dev/integration_tests_manager.dart | 39 +++++++++++++------ web_sdk/web_test_utils/pubspec.yaml | 2 +- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/e2etests/web/regular_integration_tests/pubspec.yaml b/e2etests/web/regular_integration_tests/pubspec.yaml index 9c32394c945b5..edff2da9d7896 100644 --- a/e2etests/web/regular_integration_tests/pubspec.yaml +++ b/e2etests/web/regular_integration_tests/pubspec.yaml @@ -2,7 +2,7 @@ name: regular_integration_tests publish_to: none environment: - sdk: ">=2.2.2 <3.0.0" + sdk: ">=2.11.0-0 <3.0.0" dependencies: flutter: diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart index af77ea1a1437b..58842b067177d 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart @@ -28,7 +28,7 @@ void main() async { await tester.tap(find.byKey(const Key('input'))); // Focus in input, otherwise clipboard will fail with // 'document is not focused' platform exception. - html.document.querySelector('input').focus(); + html.document.querySelector('input')?.focus(); await Clipboard.setData(const ClipboardData(text: 'sample text')); }, skip: true); // https://github.com/flutter/flutter/issues/54296 diff --git a/e2etests/web/regular_integration_tests/test_driver/text_editing_integration.dart b/e2etests/web/regular_integration_tests/test_driver/text_editing_integration.dart index 49247d7d79cfb..1fe774d855785 100644 --- a/e2etests/web/regular_integration_tests/test_driver/text_editing_integration.dart +++ b/e2etests/web/regular_integration_tests/test_driver/text_editing_integration.dart @@ -13,7 +13,9 @@ import 'package:flutter/material.dart'; import 'package:integration_test/integration_test.dart'; void main() { - final IntegrationTestWidgetsFlutterBinding binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized() as IntegrationTestWidgetsFlutterBinding; + final IntegrationTestWidgetsFlutterBinding binding = + IntegrationTestWidgetsFlutterBinding.ensureInitialized() + as IntegrationTestWidgetsFlutterBinding; testWidgets('Focused text field creates a native input element', (WidgetTester tester) async { @@ -38,7 +40,7 @@ void main() { // Change the value of the TextFormField. final TextFormField textFormField = tester.widget(finder); - textFormField.controller.text = 'New Value'; + textFormField.controller?.text = 'New Value'; // DOM element's value also changes. expect(input.value, 'New Value'); @@ -68,7 +70,7 @@ void main() { // Change the value of the TextFormField. final TextFormField textFormField = tester.widget(finder); - textFormField.controller.text = 'New Value'; + textFormField.controller?.text = 'New Value'; // DOM element's value also changes. expect(input.value, 'New Value'); }); @@ -145,9 +147,9 @@ void main() { expect(input2.value, 'Text2'); }); - testWidgets('Jump between TextFormFields with tab key after CapsLock is' - 'activated', - (WidgetTester tester) async { + testWidgets( + 'Jump between TextFormFields with tab key after CapsLock is' + 'activated', (WidgetTester tester) async { app.main(); await tester.pumpAndSettle(); @@ -163,7 +165,7 @@ void main() { final List nodeList = document.getElementsByTagName('input'); expect(nodeList.length, equals(1)); final InputElement input = - document.getElementsByTagName('input')[0] as InputElement; + document.getElementsByTagName('input')[0] as InputElement; // Press and release CapsLock. dispatchKeyboardEvent(input, 'keydown', { @@ -207,7 +209,7 @@ void main() { // A native input element for the next TextField should be attached to the // DOM. final InputElement input2 = - document.getElementsByTagName('input')[0] as InputElement; + document.getElementsByTagName('input')[0] as InputElement; expect(input2.value, 'Text2'); }); @@ -243,8 +245,8 @@ void main() { expect(input.hasAttribute('readonly'), isTrue); // Make sure the entire text is selected. - TextRange range = - TextRange(start: input.selectionStart, end: input.selectionEnd); + TextRange range = TextRange( + start: input.selectionStart ?? 0, end: input.selectionEnd ?? 0); expect(range.textInside(text), text); // Double tap to select the first word. @@ -257,7 +259,8 @@ void main() { await gesture.up(); await gesture.down(firstWordOffset); await gesture.up(); - range = TextRange(start: input.selectionStart, end: input.selectionEnd); + range = TextRange( + start: input.selectionStart ?? 0, end: input.selectionEnd ?? 0); expect(range.textInside(text), 'Lorem'); // Double tap to select the last word. @@ -270,7 +273,8 @@ void main() { await gesture.up(); await gesture.down(lastWordOffset); await gesture.up(); - range = TextRange(start: input.selectionStart, end: input.selectionEnd); + range = TextRange( + start: input.selectionStart ?? 0, end: input.selectionEnd ?? 0); expect(range.textInside(text), 'amet'); }); } diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index ac6c2b50a6183..e8fbc3715b946 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -144,13 +144,18 @@ class IntegrationTestsManager { int numberOfPassedTests = 0; int numberOfFailedTests = 0; + final Set buildModes = {'debug', 'profile', 'release'}; for (String fileName in e2eTestsToRun) { - final bool testResults = - await _runTestsInProfileMode(directory, fileName); - if (testResults) { - numberOfPassedTests++; - } else { - numberOfFailedTests++; + for (String mode in buildModes) { + if (!blockedTestsListsMapForModes[mode].contains(fileName)) { + final bool testResults = + await _runTestsInMode(directory, fileName, mode: mode); + if (testResults) { + numberOfPassedTests++; + } else { + numberOfFailedTests++; + } + } } } final int numberOfTestsRun = numberOfPassedTests + numberOfFailedTests; @@ -160,8 +165,8 @@ class IntegrationTestsManager { return numberOfFailedTests == 0; } - Future _runTestsInProfileMode( - io.Directory directory, String testName) async { + Future _runTestsInMode(io.Directory directory, String testName, + {String mode = 'profile'}) async { String executable = _useSystemFlutter ? 'flutter' : environment.flutterCommand.path; Map enviroment = Map(); @@ -172,16 +177,17 @@ class IntegrationTestsManager { IntegrationArguments.fromBrowser(_browser); final int exitCode = await runProcess( executable, - arguments.getTestArguments(testName, 'profile'), + arguments.getTestArguments(testName, mode), workingDirectory: directory.path, environment: enviroment, ); if (exitCode != 0) { + final String command = arguments.getCommandToRun(testName, mode); io.stderr .writeln('ERROR: Failed to run test. Exited with exit code $exitCode' '. To run $testName locally use the following command:' - '\n\n${arguments.getCommandToRun(testName, 'profile')}'); + '\n\n$command'); return false; } else { return true; @@ -349,7 +355,7 @@ class ChromeIntegrationArguments extends IntegrationArguments { String getCommandToRun(String testName, String mode) { String statementToRun = 'flutter drive ' - '--target=test_driver/${testName} -d web-server --profile ' + '--target=test_driver/${testName} -d web-server --$mode ' '--browser-name=chrome --local-engine=host_debug_unopt'; if (isLuci) { statementToRun = '$statementToRun --chrome-binary=' @@ -440,3 +446,14 @@ const Map> blockedTestsListsMap = >{ 'target_platform_ios_integration.dart', ], }; + +/// Tests blocked for one of the build modes. +const Map> blockedTestsListsMapForModes = + >{ + 'debug': [ + 'treeshaking_integration.dart', + 'text_editing_integration.dart', + ], + 'profile': [], + 'release': [], +}; diff --git a/web_sdk/web_test_utils/pubspec.yaml b/web_sdk/web_test_utils/pubspec.yaml index 7f92e2348463e..15b2cacc1b604 100644 --- a/web_sdk/web_test_utils/pubspec.yaml +++ b/web_sdk/web_test_utils/pubspec.yaml @@ -1,7 +1,7 @@ name: web_test_utils environment: - sdk: ">=2.2.0 <3.0.0" + sdk: ">=2.11.0-0 <3.0.0" dependencies: path: 1.8.0-nullsafety.3 From f2213f39b8b403732144ee711df71e388d92c399 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 2 Nov 2020 17:39:20 -0800 Subject: [PATCH 02/16] don't run debug mode on other browsers --- lib/web_ui/dev/integration_tests_manager.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index e8fbc3715b946..4596900fe4fa2 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -144,7 +144,9 @@ class IntegrationTestsManager { int numberOfPassedTests = 0; int numberOfFailedTests = 0; - final Set buildModes = {'debug', 'profile', 'release'}; + final Set buildModes = _browser == 'chrome' + ? {'debug', 'profile', 'release'} + : {'profile', 'release'}; for (String fileName in e2eTestsToRun) { for (String mode in buildModes) { if (!blockedTestsListsMapForModes[mode].contains(fileName)) { From 75465a33bf5b42aec7377bca2ec9140babba7667 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 2 Nov 2020 20:43:49 -0800 Subject: [PATCH 03/16] fix platform message test failures --- .../test_driver/platform_messages_integration.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart index 58842b067177d..2da071aa11ad7 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart @@ -46,12 +46,12 @@ void main() async { app.main(); await tester.pumpAndSettle(); final Map createArgs = { - 'id': '567', + 'id': 567, 'viewType': 'MyView', }; await SystemChannels.platform_views.invokeMethod('create', createArgs); final Map disposeArgs = { - 'id': '567', + 'id': 567, }; await SystemChannels.platform_views.invokeMethod('dispose', disposeArgs); expect(viewInstanceCount, 1); From eae835f401a9236625accb2951eda8c4f891dc0d Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 2 Nov 2020 21:10:06 -0800 Subject: [PATCH 04/16] some cleanup. change dispose platform channel message --- .../lib/profile_diagnostics_main.dart | 1 - .../test_driver/platform_messages_integration.dart | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/e2etests/web/regular_integration_tests/lib/profile_diagnostics_main.dart b/e2etests/web/regular_integration_tests/lib/profile_diagnostics_main.dart index 26dba545b9261..fe9c4ddf58b12 100644 --- a/e2etests/web/regular_integration_tests/lib/profile_diagnostics_main.dart +++ b/e2etests/web/regular_integration_tests/lib/profile_diagnostics_main.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; void main() { runApp(MyApp()); diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart index 2da071aa11ad7..c20e91163d82c 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_integration.dart @@ -36,7 +36,7 @@ void main() async { (WidgetTester tester) async { int viewInstanceCount = 0; - final int currentViewId = platformViewsRegistry.getNextPlatformViewId(); + platformViewsRegistry.getNextPlatformViewId(); // ignore: undefined_prefixed_name ui.platformViewRegistry.registerViewFactory('MyView', (int viewId) { ++viewInstanceCount; @@ -50,10 +50,7 @@ void main() async { 'viewType': 'MyView', }; await SystemChannels.platform_views.invokeMethod('create', createArgs); - final Map disposeArgs = { - 'id': 567, - }; - await SystemChannels.platform_views.invokeMethod('dispose', disposeArgs); + await SystemChannels.platform_views.invokeMethod('dispose', 567); expect(viewInstanceCount, 1); }); } From 5889d0cceac8992da42966f4da0ea1c8d001e968 Mon Sep 17 00:00:00 2001 From: nturgut Date: Tue, 3 Nov 2020 15:53:03 -0800 Subject: [PATCH 05/16] adding flags to control the integration tests better with felt --- lib/web_ui/dev/integration_tests_manager.dart | 93 ++++++++++++++++++- lib/web_ui/dev/test_runner.dart | 11 ++- 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 4596900fe4fa2..99b35a4c0a6c9 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -5,6 +5,7 @@ // @dart = 2.6 import 'dart:io' as io; +import 'package:args/args.dart'; import 'package:path/path.dart' as pathlib; import 'chrome_installer.dart'; @@ -144,9 +145,20 @@ class IntegrationTestsManager { int numberOfPassedTests = 0; int numberOfFailedTests = 0; - final Set buildModes = _browser == 'chrome' - ? {'debug', 'profile', 'release'} - : {'profile', 'release'}; + + Set buildModes; + if (_buildModeSelected) { + final String mode = IntegrationTestsArgumentParser.instance.buildMode; + if (mode == 'debug' && _browser != 'chrome') { + throw ToolException('Debug mode is only supported for Chrome.'); + } else { + buildModes = {mode}; + } + } else { + buildModes = _browser == 'chrome' + ? {'debug', 'profile', 'release'} + : {'profile', 'release'}; + } for (String fileName in e2eTestsToRun) { for (String mode in buildModes) { if (!blockedTestsListsMapForModes[mode].contains(fileName)) { @@ -298,9 +310,19 @@ class IntegrationTestsManager { } } + bool get _buildModeSelected => + !IntegrationTestsArgumentParser.instance.buildMode.isEmpty; + /// Validate the given `browser`, `platform` combination is suitable for /// integration tests to run. bool validateIfTestsShouldRun() { + if (_buildModeSelected) { + final String mode = IntegrationTestsArgumentParser.instance.buildMode; + if (mode == 'debug' && _browser != 'chrome') { + throw ToolException('Debug mode is only supported for Chrome.'); + } + } + // Chrome tests should run at all Platforms (Linux, macOS, Windows). // They can also run successfully on CI and local. if (_browser == 'chrome') { @@ -406,6 +428,71 @@ class SafariIntegrationArguments extends IntegrationArguments { 'flutter ${getTestArguments(testName, mode).join(' ')}'; } +/// Parses additional options that can be used when running integration tests. +class IntegrationTestsArgumentParser { + static final IntegrationTestsArgumentParser _singletonInstance = + IntegrationTestsArgumentParser._(); + + /// The [IntegrationTestsArgumentParser] singleton. + static IntegrationTestsArgumentParser get instance => _singletonInstance; + + IntegrationTestsArgumentParser._(); + + /// If target name is provided integration tests can run that one test + /// instead of running all the tests. + String testTarget; + + /// The build mode to run the integration tests. + /// + /// If not specified, these tests will run using 'debug, profile, release' + /// modes on Chrome and will run using 'profile, release' on other browsers. + /// + /// + String buildMode; + + /// Whether the rendering backend to be used is html. + /// + /// If set to false canvaskit rendering backend will be used. + bool useHtmlBackend; + + void populateOptions(ArgParser argParser) { + argParser + ..addOption( + 'target', + help: 'By default integration tests are run for all the tests under' + 'flutter/e2etests/web directory. If a test name is specified, that ' + 'only that test will run. The test name will be the name of the ' + 'integration test (e2e test) file. For example: ' + 'text_editing_integration.dart or ' + 'profile_diagnostics_integration.dart', + ) + ..addOption('build-mode', + defaultsTo: '', + help: 'Flutter supports three modes when building your app. The By ' + 'default debug, profile, release modes will be used one by one ' + 'on Chrome and profile, release modes will be used for other ' + 'browsers. If a build mode is selected tests will only be run ' + 'using that mode. ') + ..addFlag('html-backend', + defaultsTo: true, + help: 'Default rendering backend is HTML. If this flag is set to ' + 'false canvaskit backend will be used for rendering when running ' + 'integration tests.'); + } + + /// Populate browser with results of the arguments passed. + void parseOptions(ArgResults argResults) { + testTarget = argResults['target'] as String; + buildMode = argResults['build-mode'] as String; + if (buildMode.toLowerCase() != 'debug' && + buildMode.toLowerCase() != 'profile' && + buildMode.toLowerCase() != 'release') { + throw ArgumentError('Unexpected build mode: $buildMode'); + } + useHtmlBackend = argResults['html-backend'] as bool; + } +} + /// Prepares a key for the [blackList] map. /// /// Uses the browser name and the operating system name. diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index c9dfacdb33c6a..52acb05f6b088 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -90,6 +90,7 @@ class TestCommand extends Command with ArgUtils { SupportedBrowsers.instance.argParsers .forEach((t) => t.populateOptions(argParser)); + IntegrationTestsArgumentParser.instance.populateOptions(argParser); } @override @@ -147,6 +148,8 @@ class TestCommand extends Command with ArgUtils { case TestTypesRequested.unit: return runUnitTests(); case TestTypesRequested.integration: + // Parse additional arguments specific for integration testing. + IntegrationTestsArgumentParser.instance.parseOptions(argResults); return runIntegrationTests(); case TestTypesRequested.all: if (runAllTests && isIntegrationTestsAvailable) { @@ -165,9 +168,9 @@ class TestCommand extends Command with ArgUtils { } Future runIntegrationTests() async { - if(!_testPreparationReady) { - await _prepare(); - } + // if(!_testPreparationReady) { + // await _prepare(); + // } return IntegrationTestsManager( browser, useSystemFlutter, doUpdateScreenshotGoldens) .runTests(); @@ -204,7 +207,7 @@ class TestCommand extends Command with ArgUtils { // If screenshot tests are available, fetch the screenshot goldens. if (isScreenshotTestsAvailable) { - print('screenshot tests available'); + print('INFO: Screenshot tests available'); final GoldensRepoFetcher goldensRepoFetcher = GoldensRepoFetcher( environment.webUiGoldensRepositoryDirectory, path.join(environment.webUiDevDir.path, 'goldens_lock.yaml')); From 8117c928b77ce23fcfa239933d00c28e09bde533 Mon Sep 17 00:00:00 2001 From: nturgut Date: Tue, 3 Nov 2020 17:49:46 -0800 Subject: [PATCH 06/16] running tests by target name, selecting web rendering backend --- lib/web_ui/dev/integration_tests_manager.dart | 157 +++++++++++++----- lib/web_ui/dev/test_runner.dart | 6 +- 2 files changed, 114 insertions(+), 49 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 99b35a4c0a6c9..c96ba298a6077 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -122,27 +122,41 @@ class IntegrationTestsManager { final List blockedTests = blockedTestsListsMap[getBlockedTestsListMapKey(_browser)] ?? []; - // The following loops over the contents of the directory and saves an - // expected driver file name for each e2e test assuming any dart file - // not ending with `_test.dart` is an e2e test. - // Other files are not considered since developers can add files such as - // README. - for (io.File f in entities) { - final String basename = pathlib.basename(f.path); - if (!basename.contains('_test.dart') && basename.endsWith('.dart')) { - // Do not add the basename if it is in the `blockedTests`. - if (!blockedTests.contains(basename)) { - e2eTestsToRun.add(basename); - } else { - print('INFO: Test $basename is skipped since it is blocked for ' - '${getBlockedTestsListMapKey(_browser)}'); + // If no target is specified run all the tests. + if (IntegrationTestsArgumentParser.instance.testTarget.isEmpty) { + // The following loops over the contents of the directory and saves an + // expected driver file name for each e2e test assuming any dart file + // not ending with `_test.dart` is an e2e test. + // Other files are not considered since developers can add files such as + // README. + for (io.File f in entities) { + final String basename = pathlib.basename(f.path); + if (!basename.contains('_test.dart') && basename.endsWith('.dart')) { + // Do not add the basename if it is in the `blockedTests`. + if (!blockedTests.contains(basename)) { + e2eTestsToRun.add(basename); + } else { + print('INFO: Test $basename is skipped since it is blocked for ' + '${getBlockedTestsListMapKey(_browser)}'); + } } } + print( + 'INFO: In project ${directory} ${e2eTestsToRun.length} tests to run.'); + } else { + // If a target is specified it will run regardless of if it's blocked or + // not. There will be an info note to warn the developer. + final String targetTest = + IntegrationTestsArgumentParser.instance.testTarget; + final io.File file = + entities.singleWhere((f) => pathlib.basename(f.path) == targetTest); + final String basename = pathlib.basename(file.path); + if (blockedTests.contains(basename)) { + print('INFO: Test $basename do not run on CI environments.'); + } + e2eTestsToRun.add(basename); } - print( - 'INFO: In project ${directory} ${e2eTestsToRun.length} tests to run.'); - int numberOfPassedTests = 0; int numberOfFailedTests = 0; @@ -160,27 +174,52 @@ class IntegrationTestsManager { : {'profile', 'release'}; } for (String fileName in e2eTestsToRun) { - for (String mode in buildModes) { - if (!blockedTestsListsMapForModes[mode].contains(fileName)) { - final bool testResults = - await _runTestsInMode(directory, fileName, mode: mode); - if (testResults) { - numberOfPassedTests++; - } else { - numberOfFailedTests++; - } - } - } + await _runTestsTarget(directory, fileName, buildModes); } - final int numberOfTestsRun = numberOfPassedTests + numberOfFailedTests; + + final int numberOfTestsRun = _numberOfPassedTests + _numberOfFailedTests; print('INFO: ${numberOfTestsRun} tests run. ${numberOfPassedTests} passed ' 'and ${numberOfFailedTests} failed.'); return numberOfFailedTests == 0; } + int _numberOfPassedTests = 0; + int _numberOfFailedTests = 0; + + Future _runTestsTarget( + io.Directory directory, String target, Set buildModes) async { + for (String mode in buildModes) { + if (!blockedTestsListsMapForModes[mode].contains(target)) { + // Run tests on html backend if no backend is selected or only + // `html` backend is selected. + if (!_renderingBackendSelected || _renderingBackendHtml) { + final bool htmlTestResults = + await _runTestsInMode(directory, target, mode: mode); + if (htmlTestResults) { + _numberOfPassedTests++; + } else { + _numberOfFailedTests++; + } + } + // Run the same test with canvaskit rendering backend if `html` + // backend is not specifically selected. + if (!_renderingBackendSelected || !_renderingBackendHtml) { + final bool canvaskitTestResults = await _runTestsInMode( + directory, target, + mode: mode, canvaskitBackend: true); + if (canvaskitTestResults) { + _numberOfPassedTests++; + } else { + _numberOfFailedTests++; + } + } + } + } + } + Future _runTestsInMode(io.Directory directory, String testName, - {String mode = 'profile'}) async { + {String mode = 'profile', bool canvaskitBackend = false}) async { String executable = _useSystemFlutter ? 'flutter' : environment.flutterCommand.path; Map enviroment = Map(); @@ -189,6 +228,10 @@ class IntegrationTestsManager { } final IntegrationArguments arguments = IntegrationArguments.fromBrowser(_browser); + final List testArgs = arguments.getTestArguments(testName, mode); + if (canvaskitBackend) { + testArgs.add('--web-renderer=canvaskit'); + } final int exitCode = await runProcess( executable, arguments.getTestArguments(testName, mode), @@ -313,6 +356,12 @@ class IntegrationTestsManager { bool get _buildModeSelected => !IntegrationTestsArgumentParser.instance.buildMode.isEmpty; + bool get _renderingBackendSelected => + !IntegrationTestsArgumentParser.instance.renderingBackend.isEmpty; + + bool get _renderingBackendHtml => + IntegrationTestsArgumentParser.instance.renderingBackend == 'html'; + /// Validate the given `browser`, `platform` combination is suitable for /// integration tests to run. bool validateIfTestsShouldRun() { @@ -447,13 +496,14 @@ class IntegrationTestsArgumentParser { /// If not specified, these tests will run using 'debug, profile, release' /// modes on Chrome and will run using 'profile, release' on other browsers. /// - /// + /// In order to skip a test for one of the modes, add the test to the + /// `blockedTestsListsMapForModes` list for the relevant compile mode. String buildMode; - /// Whether the rendering backend to be used is html. + /// Whether to use html or canvaskit backend. /// - /// If set to false canvaskit rendering backend will be used. - bool useHtmlBackend; + /// If not set html rendering backend will be used. + String renderingBackend; void populateOptions(ArgParser argParser) { argParser @@ -468,28 +518,39 @@ class IntegrationTestsArgumentParser { ) ..addOption('build-mode', defaultsTo: '', - help: 'Flutter supports three modes when building your app. The By ' - 'default debug, profile, release modes will be used one by one ' - 'on Chrome and profile, release modes will be used for other ' - 'browsers. If a build mode is selected tests will only be run ' - 'using that mode. ') - ..addFlag('html-backend', - defaultsTo: true, - help: 'Default rendering backend is HTML. If this flag is set to ' - 'false canvaskit backend will be used for rendering when running ' - 'integration tests.'); + help: 'Flutter supports three modes when building your app. This ' + 'option sets the build mode for the integration tests. ' + 'By default an integration test will sequentially run on ' + 'multiple modes. All three modes (debug, release, profile) are ' + 'used for Chrome. Only profile, release modes will be used for ' + 'other browsers. In other words, if a build mode is selected ' + 'tests will only be run using that mode. ' + 'See https://flutter.dev/docs/testing/build-modes for more ' + 'details on the build modes.') + ..addOption('rendering-backend', + defaultsTo: 'html', + help: 'By default both `html` and `canvaskit` rendering backends are ' + ' tested when running integration tests. If this option is set ' + ' only one of these backends will be used. `canvaskit` and `html`' + ' are the only available options. '); } /// Populate browser with results of the arguments passed. void parseOptions(ArgResults argResults) { testTarget = argResults['target'] as String; buildMode = argResults['build-mode'] as String; - if (buildMode.toLowerCase() != 'debug' && + if (!buildMode.isEmpty && + buildMode.toLowerCase() != 'debug' && buildMode.toLowerCase() != 'profile' && buildMode.toLowerCase() != 'release') { throw ArgumentError('Unexpected build mode: $buildMode'); } - useHtmlBackend = argResults['html-backend'] as bool; + renderingBackend = argResults['rendering-backend'] as String; + if (!renderingBackend.isEmpty && + renderingBackend.toLowerCase() != 'html' && + renderingBackend.toLowerCase() != 'canvaskit') { + throw ArgumentError('Unexpected rendering backend: $renderingBackend'); + } } } @@ -537,6 +598,10 @@ const Map> blockedTestsListsMap = >{ }; /// Tests blocked for one of the build modes. +/// +/// If a test is not suppose to run for one of the modes also add that test +/// to the corresponding list. +// TODO(nurhan): Remove the failing test after fixing. const Map> blockedTestsListsMapForModes = >{ 'debug': [ diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 52acb05f6b088..4936781908d96 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -168,9 +168,9 @@ class TestCommand extends Command with ArgUtils { } Future runIntegrationTests() async { - // if(!_testPreparationReady) { - // await _prepare(); - // } + if(!_testPreparationReady) { + await _prepare(); + } return IntegrationTestsManager( browser, useSystemFlutter, doUpdateScreenshotGoldens) .runTests(); From 3d7e14589b8966221649277827409f28126c634c Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 4 Nov 2020 10:16:21 -0800 Subject: [PATCH 07/16] fix conditions --- lib/web_ui/dev/integration_tests_manager.dart | 65 +++++++++++-------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index c96ba298a6077..3d771952f184c 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -160,19 +160,8 @@ class IntegrationTestsManager { int numberOfPassedTests = 0; int numberOfFailedTests = 0; - Set buildModes; - if (_buildModeSelected) { - final String mode = IntegrationTestsArgumentParser.instance.buildMode; - if (mode == 'debug' && _browser != 'chrome') { - throw ToolException('Debug mode is only supported for Chrome.'); - } else { - buildModes = {mode}; - } - } else { - buildModes = _browser == 'chrome' - ? {'debug', 'profile', 'release'} - : {'profile', 'release'}; - } + final Set buildModes = _getBuildModes(); + for (String fileName in e2eTestsToRun) { await _runTestsTarget(directory, fileName, buildModes); } @@ -228,13 +217,10 @@ class IntegrationTestsManager { } final IntegrationArguments arguments = IntegrationArguments.fromBrowser(_browser); - final List testArgs = arguments.getTestArguments(testName, mode); - if (canvaskitBackend) { - testArgs.add('--web-renderer=canvaskit'); - } final int exitCode = await runProcess( executable, - arguments.getTestArguments(testName, mode), + arguments.getTestArguments(testName, mode, + isCanvaskitBackend: canvaskitBackend), workingDirectory: directory.path, environment: enviroment, ); @@ -251,6 +237,23 @@ class IntegrationTestsManager { } } + Set _getBuildModes() { + Set buildModes; + if (_buildModeSelected) { + final String mode = IntegrationTestsArgumentParser.instance.buildMode; + if (mode == 'debug' && _browser != 'chrome') { + throw ToolException('Debug mode is only supported for Chrome.'); + } else { + buildModes = {mode}; + } + } else { + buildModes = _browser == 'chrome' + ? {'debug', 'profile', 'release'} + : {'profile', 'release'}; + } + return buildModes; + } + /// Validate the directory has a `pubspec.yaml` file and a `test_driver` /// directory. /// @@ -405,14 +408,16 @@ abstract class IntegrationArguments { } } - List getTestArguments(String testName, String mode); + List getTestArguments(String testName, String mode, + {bool isCanvaskitBackend = false}); String getCommandToRun(String testName, String mode); } /// Arguments to give `flutter drive` to run the integration tests on Chrome. class ChromeIntegrationArguments extends IntegrationArguments { - List getTestArguments(String testName, String mode) { + List getTestArguments(String testName, String mode, + {bool isCanvaskitBackend = false}) { return [ 'drive', '--target=test_driver/${testName}', @@ -423,10 +428,12 @@ class ChromeIntegrationArguments extends IntegrationArguments { if (isLuci) '--chrome-binary=${preinstalledChromeExecutable()}', '--headless', '--local-engine=host_debug_unopt', + if (isCanvaskitBackend) '--web-renderer=canvaskit', ]; } - String getCommandToRun(String testName, String mode) { + String getCommandToRun(String testName, String mode, + {bool isCanvaskitBackend = false}) { String statementToRun = 'flutter drive ' '--target=test_driver/${testName} -d web-server --$mode ' '--browser-name=chrome --local-engine=host_debug_unopt'; @@ -440,7 +447,8 @@ class ChromeIntegrationArguments extends IntegrationArguments { /// Arguments to give `flutter drive` to run the integration tests on Firefox. class FirefoxIntegrationArguments extends IntegrationArguments { - List getTestArguments(String testName, String mode) { + List getTestArguments(String testName, String mode, + {bool isCanvaskitBackend = false}) { return [ 'drive', '--target=test_driver/${testName}', @@ -450,10 +458,12 @@ class FirefoxIntegrationArguments extends IntegrationArguments { '--browser-name=firefox', '--headless', '--local-engine=host_debug_unopt', + if (isCanvaskitBackend) '--web-renderer=canvaskit', ]; } - String getCommandToRun(String testName, String mode) => + String getCommandToRun(String testName, String mode, + {bool isCanvaskitBackend = false}) => 'flutter ${getTestArguments(testName, mode).join(' ')}'; } @@ -461,7 +471,8 @@ class FirefoxIntegrationArguments extends IntegrationArguments { class SafariIntegrationArguments extends IntegrationArguments { SafariIntegrationArguments(); - List getTestArguments(String testName, String mode) { + List getTestArguments(String testName, String mode, + {bool isCanvaskitBackend = false}) { return [ 'drive', '--target=test_driver/${testName}', @@ -470,10 +481,12 @@ class SafariIntegrationArguments extends IntegrationArguments { '--$mode', '--browser-name=safari', '--local-engine=host_debug_unopt', + if (isCanvaskitBackend) '--web-renderer=canvaskit', ]; } - String getCommandToRun(String testName, String mode) => + String getCommandToRun(String testName, String mode, + {bool isCanvaskitBackend = false}) => 'flutter ${getTestArguments(testName, mode).join(' ')}'; } @@ -528,7 +541,7 @@ class IntegrationTestsArgumentParser { 'See https://flutter.dev/docs/testing/build-modes for more ' 'details on the build modes.') ..addOption('rendering-backend', - defaultsTo: 'html', + defaultsTo: '', help: 'By default both `html` and `canvaskit` rendering backends are ' ' tested when running integration tests. If this option is set ' ' only one of these backends will be used. `canvaskit` and `html`' From 0f13714108f9f9865b40704b171fc0626b87ffe0 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 4 Nov 2020 10:43:29 -0800 Subject: [PATCH 08/16] carrying some conditions to helper methods. Adding comments --- lib/web_ui/dev/integration_tests_manager.dart | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 3d771952f184c..10e81899f3f5d 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -110,6 +110,9 @@ class IntegrationTestsManager { return testResults; } + int _numberOfPassedTests = 0; + int _numberOfFailedTests = 0; + Future _runTestsInDirectory(io.Directory directory) async { final io.Directory testDirectory = io.Directory(pathlib.join(directory.path, 'test_driver')); @@ -123,7 +126,7 @@ class IntegrationTestsManager { blockedTestsListsMap[getBlockedTestsListMapKey(_browser)] ?? []; // If no target is specified run all the tests. - if (IntegrationTestsArgumentParser.instance.testTarget.isEmpty) { + if (_runAllTestTargets) { // The following loops over the contents of the directory and saves an // expected driver file name for each e2e test assuming any dart file // not ending with `_test.dart` is an e2e test. @@ -173,16 +176,11 @@ class IntegrationTestsManager { return numberOfFailedTests == 0; } - int _numberOfPassedTests = 0; - int _numberOfFailedTests = 0; - Future _runTestsTarget( io.Directory directory, String target, Set buildModes) async { for (String mode in buildModes) { if (!blockedTestsListsMapForModes[mode].contains(target)) { - // Run tests on html backend if no backend is selected or only - // `html` backend is selected. - if (!_renderingBackendSelected || _renderingBackendHtml) { + if (!_skipHtmlBackendTesting(target)) { final bool htmlTestResults = await _runTestsInMode(directory, target, mode: mode); if (htmlTestResults) { @@ -191,9 +189,8 @@ class IntegrationTestsManager { _numberOfFailedTests++; } } - // Run the same test with canvaskit rendering backend if `html` - // backend is not specifically selected. - if (!_renderingBackendSelected || !_renderingBackendHtml) { + // Run the same test with canvaskit rendering backend. + if (!_skipCanvaskitBackendTesting(target)) { final bool canvaskitTestResults = await _runTestsInMode( directory, target, mode: mode, canvaskitBackend: true); @@ -365,6 +362,21 @@ class IntegrationTestsManager { bool get _renderingBackendHtml => IntegrationTestsArgumentParser.instance.renderingBackend == 'html'; + bool get _runAllTestTargets => + IntegrationTestsArgumentParser.instance.testTarget.isEmpty; + + // Skip tests on html backend if backend is selected and selection is not + // `html` backend or if the test target is in the blocked list. + bool _skipHtmlBackendTesting(String testName) => + (_renderingBackendSelected && !_renderingBackendHtml) || + blockedTestsListsMapForRenderBackends['html'].contains(testName); + + // Skip tests on canvaskit backend if backend is selected and selection is + // `html` backend or if the test target is in the blocked list. + bool _skipCanvaskitBackendTesting(String testName) => + (_renderingBackendSelected && _renderingBackendHtml) || + blockedTestsListsMapForRenderBackends['canvaskit'].contains(testName); + /// Validate the given `browser`, `platform` combination is suitable for /// integration tests to run. bool validateIfTestsShouldRun() { @@ -522,6 +534,7 @@ class IntegrationTestsArgumentParser { argParser ..addOption( 'target', + defaultsTo: '', help: 'By default integration tests are run for all the tests under' 'flutter/e2etests/web directory. If a test name is specified, that ' 'only that test will run. The test name will be the name of the ' @@ -624,3 +637,14 @@ const Map> blockedTestsListsMapForModes = 'profile': [], 'release': [], }; + +/// Tests blocked for one of the build modes. +/// +/// If a test is not suppose to run for one of the modes also add that test +/// to the corresponding list. +// TODO(nurhan): Remove the failing test after fixing. +const Map> blockedTestsListsMapForRenderBackends = + >{ + 'html': [], + 'canvaskit': [], +}; From dfb5353ecbf9fc84fc76067dfd93ed2df06c9402 Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 4 Nov 2020 11:31:20 -0800 Subject: [PATCH 09/16] create a blocked list for failing canvaskit test --- lib/web_ui/dev/integration_tests_manager.dart | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 10e81899f3f5d..0d1c59b35ab95 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -449,6 +449,9 @@ class ChromeIntegrationArguments extends IntegrationArguments { String statementToRun = 'flutter drive ' '--target=test_driver/${testName} -d web-server --$mode ' '--browser-name=chrome --local-engine=host_debug_unopt'; + if (isCanvaskitBackend) { + statementToRun = '$statementToRun --web-renderer=canvaskit'; + } if (isLuci) { statementToRun = '$statementToRun --chrome-binary=' '${preinstalledChromeExecutable()}'; @@ -475,8 +478,12 @@ class FirefoxIntegrationArguments extends IntegrationArguments { } String getCommandToRun(String testName, String mode, - {bool isCanvaskitBackend = false}) => - 'flutter ${getTestArguments(testName, mode).join(' ')}'; + {bool isCanvaskitBackend = false}) { + final String arguments = + getTestArguments(testName, mode, isCanvaskitBackend: isCanvaskitBackend) + .join(' '); + return 'flutter $arguments'; + } } /// Arguments to give `flutter drive` to run the integration tests on Safari. @@ -498,8 +505,12 @@ class SafariIntegrationArguments extends IntegrationArguments { } String getCommandToRun(String testName, String mode, - {bool isCanvaskitBackend = false}) => - 'flutter ${getTestArguments(testName, mode).join(' ')}'; + {bool isCanvaskitBackend = false}) { + final String arguments = + getTestArguments(testName, mode, isCanvaskitBackend: isCanvaskitBackend) + .join(' '); + return 'flutter $arguments'; + } } /// Parses additional options that can be used when running integration tests. @@ -646,5 +657,6 @@ const Map> blockedTestsListsMapForModes = const Map> blockedTestsListsMapForRenderBackends = >{ 'html': [], - 'canvaskit': [], + // This test failed on canvaskit on all three build modes. + 'canvaskit': ['image_loading_integration.dart'], }; From 78fad8eeeeac1ddea08919b18b0dfceaf51323ba Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 4 Nov 2020 12:43:36 -0800 Subject: [PATCH 10/16] parse parameters before all integration tests --- lib/web_ui/dev/test_runner.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 4936781908d96..34cf16c9db2b9 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -148,8 +148,6 @@ class TestCommand extends Command with ArgUtils { case TestTypesRequested.unit: return runUnitTests(); case TestTypesRequested.integration: - // Parse additional arguments specific for integration testing. - IntegrationTestsArgumentParser.instance.parseOptions(argResults); return runIntegrationTests(); case TestTypesRequested.all: if (runAllTests && isIntegrationTestsAvailable) { @@ -168,6 +166,8 @@ class TestCommand extends Command with ArgUtils { } Future runIntegrationTests() async { + // Parse additional arguments specific for integration testing. + IntegrationTestsArgumentParser.instance.parseOptions(argResults); if(!_testPreparationReady) { await _prepare(); } From 9b5f83021872072a37ae1c1435b5bf8c7e80e8a0 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 9 Nov 2020 14:17:06 -0800 Subject: [PATCH 11/16] Give better warning to developers for tests that are blocked for CI --- lib/web_ui/dev/integration_tests_manager.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 0d1c59b35ab95..b270b4d50e56a 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -155,7 +155,9 @@ class IntegrationTestsManager { entities.singleWhere((f) => pathlib.basename(f.path) == targetTest); final String basename = pathlib.basename(file.path); if (blockedTests.contains(basename)) { - print('INFO: Test $basename do not run on CI environments.'); + print('INFO: Test $basename do not run on CI environments. Please ' + 'remove it from the blocked tests list if you want to enable this ' + 'test on CI.'); } e2eTestsToRun.add(basename); } From 259880832a31a164c23229090082659d6168ed7c Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 9 Nov 2020 14:39:31 -0800 Subject: [PATCH 12/16] address some reviwer comments (more remains) --- lib/web_ui/dev/integration_tests_manager.dart | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index b270b4d50e56a..287243fb838e0 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -359,10 +359,10 @@ class IntegrationTestsManager { !IntegrationTestsArgumentParser.instance.buildMode.isEmpty; bool get _renderingBackendSelected => - !IntegrationTestsArgumentParser.instance.renderingBackend.isEmpty; + !IntegrationTestsArgumentParser.instance.webRenderer.isEmpty; bool get _renderingBackendHtml => - IntegrationTestsArgumentParser.instance.renderingBackend == 'html'; + IntegrationTestsArgumentParser.instance.webRenderer == 'html'; bool get _runAllTestTargets => IntegrationTestsArgumentParser.instance.testTarget.isEmpty; @@ -541,7 +541,7 @@ class IntegrationTestsArgumentParser { /// Whether to use html or canvaskit backend. /// /// If not set html rendering backend will be used. - String renderingBackend; + String webRenderer; void populateOptions(ArgParser argParser) { argParser @@ -566,12 +566,13 @@ class IntegrationTestsArgumentParser { 'tests will only be run using that mode. ' 'See https://flutter.dev/docs/testing/build-modes for more ' 'details on the build modes.') - ..addOption('rendering-backend', + ..addOption('web-renderer', defaultsTo: '', - help: 'By default both `html` and `canvaskit` rendering backends are ' - ' tested when running integration tests. If this option is set ' - ' only one of these backends will be used. `canvaskit` and `html`' - ' are the only available options. '); + help: 'By default all three options (`html`, `canvaskit`, `auto`) ' + ' for rendering backends are tested when running integration ' + ' tests. If this option is set only the backend provided by this ' + ' option will be used. `auto`, `canvaskit` and `html`' + ' are the available options. '); } /// Populate browser with results of the arguments passed. @@ -579,16 +580,17 @@ class IntegrationTestsArgumentParser { testTarget = argResults['target'] as String; buildMode = argResults['build-mode'] as String; if (!buildMode.isEmpty && - buildMode.toLowerCase() != 'debug' && - buildMode.toLowerCase() != 'profile' && - buildMode.toLowerCase() != 'release') { + buildMode != 'debug' && + buildMode != 'profile' && + buildMode != 'release') { throw ArgumentError('Unexpected build mode: $buildMode'); } - renderingBackend = argResults['rendering-backend'] as String; - if (!renderingBackend.isEmpty && - renderingBackend.toLowerCase() != 'html' && - renderingBackend.toLowerCase() != 'canvaskit') { - throw ArgumentError('Unexpected rendering backend: $renderingBackend'); + webRenderer = argResults['web-renderer'] as String; + if (!webRenderer.isEmpty && + webRenderer != 'html' && + webRenderer != 'canvaskit' && + webRenderer != 'auto') { + throw ArgumentError('Unexpected rendering backend: $webRenderer'); } } } @@ -638,7 +640,7 @@ const Map> blockedTestsListsMap = >{ /// Tests blocked for one of the build modes. /// -/// If a test is not suppose to run for one of the modes also add that test +/// If a test is not supposed to run for one of the modes also add that test /// to the corresponding list. // TODO(nurhan): Remove the failing test after fixing. const Map> blockedTestsListsMapForModes = From 18127900828a56da52ac6e8645ff01f53d231a88 Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 9 Nov 2020 14:57:52 -0800 Subject: [PATCH 13/16] remove named parameters --- lib/web_ui/dev/integration_tests_manager.dart | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index 287243fb838e0..afee0d9e97a42 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -218,14 +218,14 @@ class IntegrationTestsManager { IntegrationArguments.fromBrowser(_browser); final int exitCode = await runProcess( executable, - arguments.getTestArguments(testName, mode, - isCanvaskitBackend: canvaskitBackend), + arguments.getTestArguments(testName, mode, canvaskitBackend), workingDirectory: directory.path, environment: enviroment, ); if (exitCode != 0) { - final String command = arguments.getCommandToRun(testName, mode); + final String command = + arguments.getCommandToRun(testName, mode, canvaskitBackend); io.stderr .writeln('ERROR: Failed to run test. Exited with exit code $exitCode' '. To run $testName locally use the following command:' @@ -422,16 +422,16 @@ abstract class IntegrationArguments { } } - List getTestArguments(String testName, String mode, - {bool isCanvaskitBackend = false}); + List getTestArguments( + String testName, String mode, bool isCanvaskitBackend); - String getCommandToRun(String testName, String mode); + String getCommandToRun(String testName, String mode, bool isCanvaskitBackend); } /// Arguments to give `flutter drive` to run the integration tests on Chrome. class ChromeIntegrationArguments extends IntegrationArguments { - List getTestArguments(String testName, String mode, - {bool isCanvaskitBackend = false}) { + List getTestArguments( + String testName, String mode, bool isCanvaskitBackend) { return [ 'drive', '--target=test_driver/${testName}', @@ -446,8 +446,8 @@ class ChromeIntegrationArguments extends IntegrationArguments { ]; } - String getCommandToRun(String testName, String mode, - {bool isCanvaskitBackend = false}) { + String getCommandToRun( + String testName, String mode, bool isCanvaskitBackend) { String statementToRun = 'flutter drive ' '--target=test_driver/${testName} -d web-server --$mode ' '--browser-name=chrome --local-engine=host_debug_unopt'; @@ -464,8 +464,8 @@ class ChromeIntegrationArguments extends IntegrationArguments { /// Arguments to give `flutter drive` to run the integration tests on Firefox. class FirefoxIntegrationArguments extends IntegrationArguments { - List getTestArguments(String testName, String mode, - {bool isCanvaskitBackend = false}) { + List getTestArguments( + String testName, String mode, bool isCanvaskitBackend) { return [ 'drive', '--target=test_driver/${testName}', @@ -479,11 +479,10 @@ class FirefoxIntegrationArguments extends IntegrationArguments { ]; } - String getCommandToRun(String testName, String mode, - {bool isCanvaskitBackend = false}) { + String getCommandToRun( + String testName, String mode, bool isCanvaskitBackend) { final String arguments = - getTestArguments(testName, mode, isCanvaskitBackend: isCanvaskitBackend) - .join(' '); + getTestArguments(testName, mode, isCanvaskitBackend).join(' '); return 'flutter $arguments'; } } @@ -492,8 +491,8 @@ class FirefoxIntegrationArguments extends IntegrationArguments { class SafariIntegrationArguments extends IntegrationArguments { SafariIntegrationArguments(); - List getTestArguments(String testName, String mode, - {bool isCanvaskitBackend = false}) { + List getTestArguments( + String testName, String mode, bool isCanvaskitBackend) { return [ 'drive', '--target=test_driver/${testName}', @@ -506,11 +505,10 @@ class SafariIntegrationArguments extends IntegrationArguments { ]; } - String getCommandToRun(String testName, String mode, - {bool isCanvaskitBackend = false}) { + String getCommandToRun( + String testName, String mode, bool isCanvaskitBackend) { final String arguments = - getTestArguments(testName, mode, isCanvaskitBackend: isCanvaskitBackend) - .join(' '); + getTestArguments(testName, mode, isCanvaskitBackend).join(' '); return 'flutter $arguments'; } } @@ -538,9 +536,10 @@ class IntegrationTestsArgumentParser { /// `blockedTestsListsMapForModes` list for the relevant compile mode. String buildMode; - /// Whether to use html or canvaskit backend. + /// Whether to use html, canvaskit or auto for web renderer. /// - /// If not set html rendering backend will be used. + /// If not set all backends will be used one after another for integration + /// tests. If set only the provided option will be used. String webRenderer; void populateOptions(ArgParser argParser) { From 73fab26557b6c9055e352d31328b14759ec58f3a Mon Sep 17 00:00:00 2001 From: nturgut Date: Mon, 9 Nov 2020 16:23:39 -0800 Subject: [PATCH 14/16] also run with auto mode --- lib/web_ui/dev/integration_tests_manager.dart | 98 ++++++++----------- 1 file changed, 41 insertions(+), 57 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index afee0d9e97a42..ff38683fa235d 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -180,23 +180,15 @@ class IntegrationTestsManager { Future _runTestsTarget( io.Directory directory, String target, Set buildModes) async { - for (String mode in buildModes) { - if (!blockedTestsListsMapForModes[mode].contains(target)) { - if (!_skipHtmlBackendTesting(target)) { - final bool htmlTestResults = - await _runTestsInMode(directory, target, mode: mode); - if (htmlTestResults) { - _numberOfPassedTests++; - } else { - _numberOfFailedTests++; - } - } - // Run the same test with canvaskit rendering backend. - if (!_skipCanvaskitBackendTesting(target)) { - final bool canvaskitTestResults = await _runTestsInMode( - directory, target, - mode: mode, canvaskitBackend: true); - if (canvaskitTestResults) { + final Set renderingBackends = _getRenderingBackends(); + for (String renderingBackend in renderingBackends) { + for (String mode in buildModes) { + if (!blockedTestsListsMapForModes[mode].contains(target) && + !blockedTestsListsMapForRenderBackends[renderingBackend] + .contains(target)) { + final bool result = await _runTestsInMode(directory, target, + mode: mode, webRenderer: renderingBackend); + if (result) { _numberOfPassedTests++; } else { _numberOfFailedTests++; @@ -207,7 +199,7 @@ class IntegrationTestsManager { } Future _runTestsInMode(io.Directory directory, String testName, - {String mode = 'profile', bool canvaskitBackend = false}) async { + {String mode = 'profile', String webRenderer = 'html'}) async { String executable = _useSystemFlutter ? 'flutter' : environment.flutterCommand.path; Map enviroment = Map(); @@ -218,14 +210,14 @@ class IntegrationTestsManager { IntegrationArguments.fromBrowser(_browser); final int exitCode = await runProcess( executable, - arguments.getTestArguments(testName, mode, canvaskitBackend), + arguments.getTestArguments(testName, mode, webRenderer), workingDirectory: directory.path, environment: enviroment, ); if (exitCode != 0) { final String command = - arguments.getCommandToRun(testName, mode, canvaskitBackend); + arguments.getCommandToRun(testName, mode, webRenderer); io.stderr .writeln('ERROR: Failed to run test. Exited with exit code $exitCode' '. To run $testName locally use the following command:' @@ -236,6 +228,17 @@ class IntegrationTestsManager { } } + Set _getRenderingBackends() { + Set renderingBackends; + if (_renderingBackendSelected) { + final String mode = IntegrationTestsArgumentParser.instance.webRenderer; + renderingBackends = {mode}; + } else { + renderingBackends = {'auto', 'html', 'canvaskit'}; + } + return renderingBackends; + } + Set _getBuildModes() { Set buildModes; if (_buildModeSelected) { @@ -361,24 +364,9 @@ class IntegrationTestsManager { bool get _renderingBackendSelected => !IntegrationTestsArgumentParser.instance.webRenderer.isEmpty; - bool get _renderingBackendHtml => - IntegrationTestsArgumentParser.instance.webRenderer == 'html'; - bool get _runAllTestTargets => IntegrationTestsArgumentParser.instance.testTarget.isEmpty; - // Skip tests on html backend if backend is selected and selection is not - // `html` backend or if the test target is in the blocked list. - bool _skipHtmlBackendTesting(String testName) => - (_renderingBackendSelected && !_renderingBackendHtml) || - blockedTestsListsMapForRenderBackends['html'].contains(testName); - - // Skip tests on canvaskit backend if backend is selected and selection is - // `html` backend or if the test target is in the blocked list. - bool _skipCanvaskitBackendTesting(String testName) => - (_renderingBackendSelected && _renderingBackendHtml) || - blockedTestsListsMapForRenderBackends['canvaskit'].contains(testName); - /// Validate the given `browser`, `platform` combination is suitable for /// integration tests to run. bool validateIfTestsShouldRun() { @@ -423,15 +411,15 @@ abstract class IntegrationArguments { } List getTestArguments( - String testName, String mode, bool isCanvaskitBackend); + String testName, String mode, String webRenderer); - String getCommandToRun(String testName, String mode, bool isCanvaskitBackend); + String getCommandToRun(String testName, String mode, String webRenderer); } /// Arguments to give `flutter drive` to run the integration tests on Chrome. class ChromeIntegrationArguments extends IntegrationArguments { List getTestArguments( - String testName, String mode, bool isCanvaskitBackend) { + String testName, String mode, String webRenderer) { return [ 'drive', '--target=test_driver/${testName}', @@ -442,18 +430,15 @@ class ChromeIntegrationArguments extends IntegrationArguments { if (isLuci) '--chrome-binary=${preinstalledChromeExecutable()}', '--headless', '--local-engine=host_debug_unopt', - if (isCanvaskitBackend) '--web-renderer=canvaskit', + '--web-renderer=$webRenderer', ]; } - String getCommandToRun( - String testName, String mode, bool isCanvaskitBackend) { + String getCommandToRun(String testName, String mode, String webRenderer) { String statementToRun = 'flutter drive ' '--target=test_driver/${testName} -d web-server --$mode ' - '--browser-name=chrome --local-engine=host_debug_unopt'; - if (isCanvaskitBackend) { - statementToRun = '$statementToRun --web-renderer=canvaskit'; - } + '--browser-name=chrome --local-engine=host_debug_unopt ' + '--web-renderer=$webRenderer'; if (isLuci) { statementToRun = '$statementToRun --chrome-binary=' '${preinstalledChromeExecutable()}'; @@ -465,7 +450,7 @@ class ChromeIntegrationArguments extends IntegrationArguments { /// Arguments to give `flutter drive` to run the integration tests on Firefox. class FirefoxIntegrationArguments extends IntegrationArguments { List getTestArguments( - String testName, String mode, bool isCanvaskitBackend) { + String testName, String mode, String webRenderer) { return [ 'drive', '--target=test_driver/${testName}', @@ -475,14 +460,13 @@ class FirefoxIntegrationArguments extends IntegrationArguments { '--browser-name=firefox', '--headless', '--local-engine=host_debug_unopt', - if (isCanvaskitBackend) '--web-renderer=canvaskit', + '--web-renderer=$webRenderer', ]; } - String getCommandToRun( - String testName, String mode, bool isCanvaskitBackend) { + String getCommandToRun(String testName, String mode, String webRenderer) { final String arguments = - getTestArguments(testName, mode, isCanvaskitBackend).join(' '); + getTestArguments(testName, mode, webRenderer).join(' '); return 'flutter $arguments'; } } @@ -492,7 +476,7 @@ class SafariIntegrationArguments extends IntegrationArguments { SafariIntegrationArguments(); List getTestArguments( - String testName, String mode, bool isCanvaskitBackend) { + String testName, String mode, String webRenderer) { return [ 'drive', '--target=test_driver/${testName}', @@ -501,14 +485,13 @@ class SafariIntegrationArguments extends IntegrationArguments { '--$mode', '--browser-name=safari', '--local-engine=host_debug_unopt', - if (isCanvaskitBackend) '--web-renderer=canvaskit', + '--web-renderer=$webRenderer', ]; } - String getCommandToRun( - String testName, String mode, bool isCanvaskitBackend) { + String getCommandToRun(String testName, String mode, String webRenderer) { final String arguments = - getTestArguments(testName, mode, isCanvaskitBackend).join(' '); + getTestArguments(testName, mode, webRenderer).join(' '); return 'flutter $arguments'; } } @@ -652,13 +635,14 @@ const Map> blockedTestsListsMapForModes = 'release': [], }; -/// Tests blocked for one of the build modes. +/// Tests blocked for one of the rendering backends. /// -/// If a test is not suppose to run for one of the modes also add that test +/// If a test is not suppose to run for one of the backends also add that test /// to the corresponding list. // TODO(nurhan): Remove the failing test after fixing. const Map> blockedTestsListsMapForRenderBackends = >{ + 'auto': [], 'html': [], // This test failed on canvaskit on all three build modes. 'canvaskit': ['image_loading_integration.dart'], From c997661ef1e02bd00c7f639923d503c235a9a165 Mon Sep 17 00:00:00 2001 From: nturgut Date: Tue, 10 Nov 2020 17:14:36 -0800 Subject: [PATCH 15/16] add verbose option --- lib/web_ui/dev/integration_tests_manager.dart | 10 +++--- lib/web_ui/dev/test_runner.dart | 6 +++- lib/web_ui/dev/utils.dart | 32 +++++++++++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index ff38683fa235d..da2177c879fac 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -144,8 +144,10 @@ class IntegrationTestsManager { } } } - print( - 'INFO: In project ${directory} ${e2eTestsToRun.length} tests to run.'); + if (isVerboseLoggingEnabled) { + print( + 'INFO: In project ${directory} ${e2eTestsToRun.length} tests to run.'); + } } else { // If a target is specified it will run regardless of if it's blocked or // not. There will be an info note to warn the developer. @@ -154,7 +156,7 @@ class IntegrationTestsManager { final io.File file = entities.singleWhere((f) => pathlib.basename(f.path) == targetTest); final String basename = pathlib.basename(file.path); - if (blockedTests.contains(basename)) { + if (blockedTests.contains(basename) && isVerboseLoggingEnabled) { print('INFO: Test $basename do not run on CI environments. Please ' 'remove it from the blocked tests list if you want to enable this ' 'test on CI.'); @@ -557,7 +559,7 @@ class IntegrationTestsArgumentParser { ' are the available options. '); } - /// Populate browser with results of the arguments passed. + /// Populate results of the arguments passed. void parseOptions(ArgResults argResults) { testTarget = argResults['target'] as String; buildMode = argResults['build-mode'] as String; diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index 34cf16c9db2b9..872698d20c848 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -90,6 +90,7 @@ class TestCommand extends Command with ArgUtils { SupportedBrowsers.instance.argParsers .forEach((t) => t.populateOptions(argParser)); + GeneralTestsArgumentParser.instance.populateOptions(argParser); IntegrationTestsArgumentParser.instance.populateOptions(argParser); } @@ -134,6 +135,7 @@ class TestCommand extends Command with ArgUtils { Future run() async { SupportedBrowsers.instance ..argParsers.forEach((t) => t.parseOptions(argResults)); + GeneralTestsArgumentParser.instance.parseOptions(argResults); // Check the flags to see what type of integration tests are requested. testTypesRequested = findTestType(); @@ -207,7 +209,9 @@ class TestCommand extends Command with ArgUtils { // If screenshot tests are available, fetch the screenshot goldens. if (isScreenshotTestsAvailable) { - print('INFO: Screenshot tests available'); + if (isVerboseLoggingEnabled) { + print('INFO: Screenshot tests available'); + } final GoldensRepoFetcher goldensRepoFetcher = GoldensRepoFetcher( environment.webUiGoldensRepositoryDirectory, path.join(environment.webUiDevDir.path, 'goldens_lock.yaml')); diff --git a/lib/web_ui/dev/utils.dart b/lib/web_ui/dev/utils.dart index 72a1b295a9e38..662550ad03253 100644 --- a/lib/web_ui/dev/utils.dart +++ b/lib/web_ui/dev/utils.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:io' as io; +import 'package:args/args.dart'; import 'package:args/command_runner.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; @@ -185,6 +186,37 @@ mixin ArgUtils on Command { } } +/// Parses additional options that can be used for all tests. +class GeneralTestsArgumentParser { + static final GeneralTestsArgumentParser _singletonInstance = + GeneralTestsArgumentParser._(); + + /// The [GeneralTestsArgumentParser] singleton. + static GeneralTestsArgumentParser get instance => _singletonInstance; + + GeneralTestsArgumentParser._(); + + /// If target name is provided integration tests can run that one test + /// instead of running all the tests. + bool verbose = false; + + void populateOptions(ArgParser argParser) { + argParser + ..addFlag( + 'verbose', + defaultsTo: false, + help: 'Flag to indicate extra logs should also be printed.', + ); + } + + /// Populate results of the arguments passed. + void parseOptions(ArgResults argResults) { + verbose = argResults['verbose'] as bool; + } +} + +bool get isVerboseLoggingEnabled => GeneralTestsArgumentParser.instance.verbose; + /// There might be proccesses started during the tests. /// /// Use this list to store those Processes, for cleaning up before shutdown. From f373ba626d93cbe02a35d4ae5073d19b688c23ea Mon Sep 17 00:00:00 2001 From: nturgut Date: Wed, 11 Nov 2020 15:35:21 -0800 Subject: [PATCH 16/16] reduce the number of tests running. skip url_test for now --- lib/web_ui/dev/integration_tests_manager.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/dev/integration_tests_manager.dart b/lib/web_ui/dev/integration_tests_manager.dart index da2177c879fac..5ce70b0f4132d 100644 --- a/lib/web_ui/dev/integration_tests_manager.dart +++ b/lib/web_ui/dev/integration_tests_manager.dart @@ -236,7 +236,8 @@ class IntegrationTestsManager { final String mode = IntegrationTestsArgumentParser.instance.webRenderer; renderingBackends = {mode}; } else { - renderingBackends = {'auto', 'html', 'canvaskit'}; + // TODO(nurhan): Enable `auto` when recipe is sharded. + renderingBackends = {'html', 'canvaskit'}; } return renderingBackends; } @@ -251,9 +252,10 @@ class IntegrationTestsManager { buildModes = {mode}; } } else { + // TODO(nurhan): Enable `release` when recipe is sharded. buildModes = _browser == 'chrome' - ? {'debug', 'profile', 'release'} - : {'profile', 'release'}; + ? {'debug', 'profile'} + : {'profile'}; } return buildModes; } @@ -632,6 +634,7 @@ const Map> blockedTestsListsMapForModes = 'debug': [ 'treeshaking_integration.dart', 'text_editing_integration.dart', + 'url_strategy_integration.dart', ], 'profile': [], 'release': [],