Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Upgrades to felt (running on multiple modes, multiple backends, single test target option) #22260

Merged
nturgut merged 16 commits into from
Nov 12, 2020
Merged

Upgrades to felt (running on multiple modes, multiple backends, single test target option) #22260

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
40848e0
testing running the tests on all build modes
Nov 3, 2020
f2213f3
don't run debug mode on other browsers
Nov 3, 2020
75465a3
fix platform message test failures
Nov 3, 2020
eae835f
some cleanup. change dispose platform channel message
Nov 3, 2020
5889d0c
adding flags to control the integration tests better with felt
Nov 3, 2020
8117c92
running tests by target name, selecting web rendering backend
Nov 4, 2020
3d7e145
fix conditions
Nov 4, 2020
0f13714
carrying some conditions to helper methods. Adding comments
Nov 4, 2020
dfb5353
create a blocked list for failing canvaskit test
Nov 4, 2020
78fad8e
parse parameters before all integration tests
Nov 4, 2020
9b5f830
Give better warning to developers for tests that are blocked for CI
Nov 9, 2020
2598808
address some reviwer comments (more remains)
Nov 9, 2020
1812790
remove named parameters
Nov 9, 2020
73fab26
also run with auto mode
Nov 10, 2020
c997661
add verbose option
Nov 11, 2020
f373ba6
reduce the number of tests running. skip url_test for now
Nov 11, 2020
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
Prev Previous commit
Next Next commit
address some reviwer comments (more remains)
  • Loading branch information
nturgut committed Nov 10, 2020
commit 259880832a31a164c23229090082659d6168ed7c
36 changes: 19 additions & 17 deletions lib/web_ui/dev/integration_tests_manager.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -566,29 +566,31 @@ 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.
void parseOptions(ArgResults argResults) {
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');
}
}
}
Expand Down Expand Up @@ -638,7 +640,7 @@ const Map<String, List<String>> blockedTestsListsMap = <String, List<String>>{

/// Tests blocked for one of the build modes.
Copy link
Contributor

@yjbanov yjbanov Nov 5, 2020

Choose a reason for hiding this comment

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

It is more common to call such tests "skipped" rather than "blocked".

Copy link
Contributor Author

@nturgut nturgut Nov 9, 2020

Choose a reason for hiding this comment

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

It is outside of the scope of this work. It has been a while since we added 'blocked' test concept. Blocked word is introduced as a replacement to 'blacklist' a while ago.

///
/// 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<String, List<String>> blockedTestsListsMapForModes =
Copy link
Contributor

@yjbanov yjbanov Nov 5, 2020

Choose a reason for hiding this comment

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

It's still weird that one needs to recompile felt in order to change the list of tests they want to run. I'd prefer a YAML file that lives closer to the test code itself.

Copy link
Contributor Author

@nturgut nturgut Nov 9, 2020

Choose a reason for hiding this comment

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

Since it's been a while we added blocked tests logic, it seems like the request is outside of this PR. I create another issue and address that change there.

Expand Down