Add web e2e to the flutter/flutter repo #67693
Conversation
flutter-dashboard
bot
added
the
c: contributor-productivity
There was a problem hiding this comment.
Can you include the command for running chromedriver on port 4444?
There was a problem hiding this comment.
done. I also added links for more information.
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
LGTM, with a couple of comments!
There was a problem hiding this comment.
Should this be integration_test: ^0.9.2? e2e is discontinued.
There was a problem hiding this comment.
I'm changing the these pubspec due to update-packages. I think they didn't migrated those tests yet. I'm not sure which team is using them though :)
There was a problem hiding this comment.
Wow, this test is involved, and text edition is still changing quite a bit, do you think this can break because of legit changes in the way text editing is handled in the engine? Can this prevent engine rolls to flow into flutter?
There was a problem hiding this comment.
Thanks! yes this is a very good test.
I think the current situation is worse (which happened a few times)
- If there is a change on the framework, engine tests won't run and won't break.
- developer will merge the change to flutter/flutter
- engine developers will start seeing failure on random PRs for felt firefox step (since it's the first step that runs integration tests)
- Later (after a research probably) the flutter/flutter PR will be reverted
- PR author will try to test the PR running felt locally, this is also a little tricky since there are many flutter/flutter developers who does not normally do engine development
In the new process,
- flutter/flutter developers would be protected against the changes to e2e and also against platform channels and text editing changes
- flutter/engine developers, will break the the tests on the engine side first if they are making a change that would change the communication between flutter and the web engine (so the tree will be read)
- If the change is on purpose, then there should be a handshake (there should be 3 PRs in a row, first one would change the test or the code to a temporary state. Last one will fix it.)
I think this is good since this is also reminds both engine/flutter PRs about the other repo. If this was a package (rather than the engine itself) best could have been to use a version in the pubspec of course. But I think we have this problem for many issues between engine<->flutter repo.
Thanks for the reminder though I'm chat with @Hixie to see what can we do for this type of changes. May be we can add a guide for developers.
|
When I check the results Google tests are passing. It didn't synced the status somehow. |
3 participants
Description
There are no tests in the flutter/flutter repo right now that tests the
integration_testpackages usage with flutter drive on web. Current driver tests does not coverintegration_testpackage. This created a P0 yesterday. I'm carrying the text_editing test we have on the engine to flutter repo.The original test used: https://github.com/flutter/engine/blob/master/e2etests/web/regular_integration_tests/test_driver/text_editing_integration.dart
Why this test:
Related Issues
#67560
Tests
This is a test file, however it will only be tested after the following steps.
-> add a new validation to the recipes (recipe side change: https://flutter-review.googlesource.com/c/recipes/+/7060)
-> add this as a new builder to flutter/infra
-> add this change to try builds under flutter/flutter
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.