[CP-beta][web] Text editing test accepts both behaviors in Firefox#173053
Conversation
|
@eyebrowsoffire please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request addresses a flaky test in Firefox by making the test accept two different behaviors for focus handling. This is a pragmatic solution for dealing with environment-dependent test failures. My review focuses on improving the clarity and maintainability of the code comment that explains this workaround.
| // | ||
| // We've seen cases in LUCI where Firefox behaves like Chrome (sets focus on the input | ||
| // element). But when running locally, we are seeing the wrong behavior explained in the | ||
| // comment above. To work around this, the test will accept both behaviors for now. | ||
| expect( | ||
| domDocument.activeElement, | ||
| anyOf(textEditing.strategy.domElement, flutterView.dom.rootElement), | ||
| ); |
There was a problem hiding this comment.
The comment explaining this workaround can be made more concise and self-contained for better readability and future maintenance. Adding a TODO to track the underlying flaky behavior would also be beneficial.
// We've seen cases in LUCI where Firefox behaves like Chrome (sets focus on the input
// element), but when running locally it does not. To work around this
// test flakiness, we accept both behaviors for now.
// TODO(flutter/flutter#<issue_number_if_any>): Track Firefox focus issue.
expect(
domDocument.activeElement,
anyOf(textEditing.strategy.domElement, flutterView.dom.rootElement),
);
52cc75c
into
flutter:flutter-3.35-candidate.0
This pull request is created by automatic cherry pick workflow
Issue Link:
#172713
Changelog Description:
Fix web engine unit tests to work on multiple versions of Firefox.
Impact Description:
Without this fix, some unit tests fail fairly regularly on Firefox.
Workaround:
This does not affect end-users, so no workaround necessary.
Risk:
Low
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Yes
Validation Steps:
Run CI.