[flutter_test] Change KeyEventSimulator default transit mode #143847

bleroux · 2024-02-21T13:48:14Z

Description

This PRs changes the default value transit mode for key event simulation.

The default transit mode for key event simulation is currently KeyDataTransitMode.rawKeyData while on the framework side KeyDataTransitMode.keyDataThenRawKeyData is the preferred transit mode.

KeyDataTransitMode.keyDataThenRawKeyData is more accurate and can help detect issues.

For instance the following test will fail with KeyDataTransitMode.rawKeyData because raw keyboard logic for modifier keys is less accurate:

  testWidgets('Press control left once', (WidgetTester tester) async {
    debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.keyDataThenRawKeyData;

    final List<KeyEvent> events = <KeyEvent>[];
    final FocusNode focusNode = FocusNode();
    addTearDown(focusNode.dispose);

    await tester.pumpWidget(
      Focus(
        focusNode: focusNode,
        autofocus: true,
        onKeyEvent: (_, KeyEvent event) {
          events.add(event);
          return KeyEventResult.handled;
        },
        child: Container(),
      ),
    );

    await simulateKeyDownEvent(LogicalKeyboardKey.controlLeft);

    // This will fail when transit mode is KeyDataTransitMode.rawKeyData
    // because a down event for controlRight is synthesized.
    expect(events.length, 1);

    debugKeyEventSimulatorTransitModeOverride = null;
  });

And the following this test is ok with KeyDataTransitMode.rawKeyData but rightly fails with KeyDataTransitMode.keyDataThenRawKeyData:

  testWidgets('Simulates consecutive key down events', (WidgetTester tester) async {
    debugKeyEventSimulatorTransitModeOverride = KeyDataTransitMode.rawKeyData;

    // Simulating several key down events without key up in between is tolerated
    // when transit mode is KeyDataTransitMode.rawKeyData, it will trigger an
    // assert on KeyDataTransitMode.keyDataThenRawKeyData.
    await simulateKeyDownEvent(LogicalKeyboardKey.arrowDown);
    await simulateKeyDownEvent(LogicalKeyboardKey.arrowDown);

    debugKeyEventSimulatorTransitModeOverride = null;
  });

Related Issue

Fixes #143845

Tests

Adds two tests.

bleroux · 2024-02-21T14:51:25Z

@gspencergoog This will unblock #143579

This PR also updates some existing tests, because they triggered some asserts when the transit mode is KeyDataTransitMode.keyDataThenRawKeyData (they did not when the transit mode was KeyDataTransitMode.rawKeyData). Some were sending several key down events without key up events in between.

I also added KeySimulatorTransitModeVariant.rawKeyData() because some tests require this mode (especially the ones for RawKeyboard).

bleroux · 2024-02-21T14:53:51Z

This PR surfaced some wrong key sequences in customer test.
See Flutter-Bounty-Hunters/flutter_test_robots#25 and Flutter-Bounty-Hunters/super_editor#1853.

gspencergoog

Thanks, this is a good thing to do, I had forgotten to set that default when I deprecated the RawKeyEvent stuff.

packages/flutter_test/test/event_simulation_test.dart

gspencergoog · 2024-02-21T19:15:17Z

This PR surfaced some wrong key sequences in customer test. See Flutter-Bounty-Hunters/flutter_test_robots#25 and superlistapp/super_editor#1853.

These will have to be fixed before we can commit this PR, since they're in customer tests.

bleroux · 2024-02-22T18:06:25Z

This PR surfaced some wrong key sequences in customer test. See Flutter-Bounty-Hunters/flutter_test_robots#25 and superlistapp/super_editor#1853.

cc @matthew-carroll @angelosilvestre I'm not sure of the process on your side, if my PRs are not compliant with your process thank you for making those changes.

matthew-carroll · 2024-02-22T20:26:22Z

@bleroux we'll need someone to open a PR in the super_editor repo with the appropriate fix, as mentioned in the policies for customer tests and breaking changes.

bleroux · 2024-02-22T20:35:47Z

@bleroux we'll need someone to open a PR in the super_editor repo with the appropriate fix, as mentioned in the policies for customer tests and breaking changes.

@matthew-carroll
I filed two PRS:

Fix wrong key sequences Flutter-Bounty-Hunters/super_editor#1853 for super_editor
Fix some wrong key sequences Flutter-Bounty-Hunters/flutter_test_robots#25 on flutter_test_robots because it is a dependance use by super_editor.

bleroux · 2024-02-27T21:36:48Z

@matthew-carroll The PRs on super_editor and flutter_test_robots are merged, thanks. Unfortunately the super editor customer tests are still failing because super_editor depends on flutter_test_robots v0.22 which does not contain my fix. Do you plan to release a new version for flutter_test_robots?

@gspencergoog The Google testing step is failing, it is probably related to some internal tests simulating wrong key sequences. Can you have a look to those failures?

matthew-carroll · 2024-02-28T00:44:01Z

@bleroux for now you can put up a super_editor PR that adds a dependency override to use the GitHub version of flutter_test_robots.

Roll super editor tests to a commit with the fix related to flutter/flutter#143847

bleroux · 2024-02-29T19:57:35Z

@gspencergoog Customer tests are ok now, unfortunately Google testing step is failing, are they many internal test failures or just a few of them?

gspencergoog · 2024-03-04T17:19:21Z

It's just one. I'll see if I can fix it.

gspencergoog · 2024-03-04T17:28:33Z

Okay, I fixed it, but the change needs to be reviewed and submitted. I'll restart the check here once it has been submitted internally.

gspencergoog · 2024-03-04T17:29:52Z

(it's a trivial fix, but it'll probably be a day before approval because of the time zone the reviewers are in)

Co-authored-by: Greg Spencer <[email protected]>

gspencergoog · 2024-03-06T16:33:08Z

Okay, Google tests fixed.

…lutter/flutter#143847)

flutter/flutter@8f84f3f...471a828 2024-03-07 [email protected] [flutter_test] Use defaultTargetPlatform for key events simulation (flutter/flutter#143579) 2024-03-07 [email protected] Roll Packages from 9b88dbc to 6701c9e (3 revisions) (flutter/flutter#144772) 2024-03-07 [email protected] Fix frameworks added to bundle multiple times instead of lipo (flutter/flutter#144688) 2024-03-07 [email protected] Roll Flutter Engine from 40a42796b129 to f8c3b2db8cd1 (1 revision) (flutter/flutter#144766) 2024-03-07 [email protected] Roll Flutter Engine from 0246484d2bae to 40a42796b129 (1 revision) (flutter/flutter#144765) 2024-03-07 [email protected] Roll Flutter Engine from 6c1751bd774e to 0246484d2bae (1 revision) (flutter/flutter#144756) 2024-03-07 [email protected] [flutter_test] Change KeyEventSimulator default transit mode (flutter/flutter#143847) 2024-03-07 [email protected] Roll Flutter Engine from 03ebd6460b83 to 6c1751bd774e (2 revisions) (flutter/flutter#144747) 2024-03-07 [email protected] Roll Flutter Engine from 8a859c5b3a2d to 03ebd6460b83 (2 revisions) (flutter/flutter#144746) 2024-03-07 [email protected] Roll Flutter Engine from 4f6ea31d1f25 to 8a859c5b3a2d (2 revisions) (flutter/flutter#144743) 2024-03-07 [email protected] Fix memory leak in `editable_gesture_test.dart` (flutter/flutter#144691) 2024-03-07 [email protected] Roll Flutter Engine from 53ddbdfc24e5 to 4f6ea31d1f25 (2 revisions) (flutter/flutter#144741) 2024-03-07 [email protected] Roll Flutter Engine from b2adf7471d3d to 53ddbdfc24e5 (1 revision) (flutter/flutter#144735) 2024-03-06 [email protected] Roll Flutter Engine from 31bbe61dfa0d to b2adf7471d3d (1 revision) (flutter/flutter#144732) 2024-03-06 [email protected] Roll Flutter Engine from 5bbac1a5c576 to 31bbe61dfa0d (3 revisions) (flutter/flutter#144724) 2024-03-06 [email protected] Run macOS test on `dev/integration_tests/ui` (flutter/flutter#142735) 2024-03-06 [email protected] Use wasm-compatible conditional import in timeline.dart, avoid emitting timeline events in SchedulerBinding (flutter/flutter#144682) 2024-03-06 [email protected] Roll Flutter Engine from 44405aedba13 to 5bbac1a5c576 (2 revisions) (flutter/flutter#144714) 2024-03-06 [email protected] Bring back firebase tests to prod (flutter/flutter#144703) 2024-03-06 [email protected] Roll Flutter Engine from 20037e385bda to 44405aedba13 (3 revisions) (flutter/flutter#144710) 2024-03-06 [email protected] Fix code sample failing in smoke test (flutter/flutter#144709) 2024-03-06 [email protected] Remove deprecated `errorColor` from `ThemeData` (flutter/flutter#144078) 2024-03-06 [email protected] make DevFSContent descendants immutable (flutter/flutter#144664) 2024-03-06 [email protected] [Impeller] measure GPU memory usage. (flutter/flutter#144575) 2024-03-06 [email protected] Roll Flutter Engine from 9aad0e93899b to 20037e385bda (1 revision) (flutter/flutter#144707) 2024-03-06 [email protected] Roll Flutter Engine from b6efe0dd88fe to 9aad0e93899b (2 revisions) (flutter/flutter#144702) 2024-03-06 [email protected] Update android templates to use target sdk 34 (flutter/flutter#144641) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

auto-submit · 2024-03-08T18:39:27Z

Time to revert pull request flutter/flutter/143847 has elapsed.
You need to open the revert manually and process as a regular pull request.

itsjustkevin · 2024-03-08T18:43:22Z

Reverting due to issues reported in b/328788027.

itsjustkevin · 2024-03-08T18:57:31Z

@gspencergoog this PR does not revert cleanly can you take a look?

…lutter#143847)" This reverts commit 8ade81f.

…lutter/flutter#143847)

bleroux mentioned this pull request Feb 21, 2024

Fix some wrong key sequences Flutter-Bounty-Hunters/flutter_test_robots#25

Merged

bleroux requested a review from gspencergoog February 21, 2024 14:54

bleroux mentioned this pull request Feb 21, 2024

Fix wrong key sequences Flutter-Bounty-Hunters/super_editor#1853

Merged

gspencergoog approved these changes Feb 21, 2024

View reviewed changes

packages/flutter_test/test/event_simulation_test.dart Outdated Show resolved Hide resolved

bleroux force-pushed the change_key_simulation_default_transit_mode branch 2 times, most recently from b7c51f2 to cf8d870 Compare February 24, 2024 06:19

bleroux mentioned this pull request Feb 28, 2024

Use dependency_overrides for flutter_tests_robots Flutter-Bounty-Hunters/super_editor#1862

Merged

bleroux force-pushed the change_key_simulation_default_transit_mode branch from cf8d870 to e568575 Compare February 28, 2024 22:26

bleroux mentioned this pull request Feb 29, 2024

Update super_editor hash flutter/tests#349

Merged

auto-submit bot pushed a commit to flutter/tests that referenced this pull request Feb 29, 2024

Update super_editor hash (#349)

cc4c8d8

Roll super editor tests to a commit with the fix related to flutter/flutter#143847

bleroux force-pushed the change_key_simulation_default_transit_mode branch 2 times, most recently from 918ee51 to 16a5691 Compare February 29, 2024 15:44

[flutter_test] Change KeyEventSimulator default transit mode

a44c90a

Fix a typo

e512a93

Co-authored-by: Greg Spencer <[email protected]>

bleroux force-pushed the change_key_simulation_default_transit_mode branch from 16a5691 to e512a93 Compare March 6, 2024 08:13

bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 7, 2024

auto-submit bot merged commit 8ade81f into flutter:master Mar 7, 2024

bleroux deleted the change_key_simulation_default_transit_mode branch March 7, 2024 07:25

engine-flutter-autoroll mentioned this pull request Mar 7, 2024

Roll Flutter from 8f84f3f32ddc to 4d446622421f (24 revisions) flutter/packages#6279

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2024

8ade81f [flutter_test] Change KeyEventSimulator default transit mode (f…

b032798

…lutter/flutter#143847)

engine-flutter-autoroll mentioned this pull request Mar 7, 2024

Roll Flutter from 8f84f3f32ddc to de728320793d (25 revisions) flutter/packages#6280

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2024

8ade81f [flutter_test] Change KeyEventSimulator default transit mode (f…

5a36c57

…lutter/flutter#143847)

engine-flutter-autoroll mentioned this pull request Mar 7, 2024

Roll Flutter from 8f84f3f32ddc to 471a82856d86 (27 revisions) flutter/packages#6281

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2024

8ade81f [flutter_test] Change KeyEventSimulator default transit mode (f…

0e9635f

…lutter/flutter#143847)

itsjustkevin added the revert Autorevert PR (with "Reason for revert:" comment) label Mar 8, 2024

auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Mar 8, 2024

gspencergoog added a commit to gspencergoog/flutter that referenced this pull request Mar 8, 2024

Revert "[flutter_test] Change KeyEventSimulator default transit mode (f…

1b3f4ea

…lutter#143847)" This reverts commit 8ade81f.

bleroux mentioned this pull request Mar 9, 2024

[flutter_test] Default transit mode for key events simulation is not the same as framework #143845

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024

8ade81f [flutter_test] Change KeyEventSimulator default transit mode (f…

7681be7

…lutter/flutter#143847)

fernandomoraesvr mentioned this pull request May 16, 2024

simulateKeyDownEvent ENTER throwing exception in integration tests Desktop #148406

Closed

[flutter_test] Change KeyEventSimulator default transit mode #143847

[flutter_test] Change KeyEventSimulator default transit mode #143847

Uh oh!

Conversation

bleroux commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Tests

Uh oh!

bleroux commented Feb 21, 2024

Uh oh!

bleroux commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gspencergoog commented Feb 21, 2024

Uh oh!

bleroux commented Feb 22, 2024

Uh oh!

matthew-carroll commented Feb 22, 2024

Uh oh!

bleroux commented Feb 22, 2024

Uh oh!

bleroux commented Feb 27, 2024

Uh oh!

matthew-carroll commented Feb 28, 2024

Uh oh!

bleroux commented Feb 29, 2024

Uh oh!

gspencergoog commented Mar 4, 2024

Uh oh!

gspencergoog commented Mar 4, 2024

Uh oh!

gspencergoog commented Mar 4, 2024

Uh oh!

gspencergoog commented Mar 6, 2024

Uh oh!

auto-submit bot commented Mar 8, 2024

Uh oh!

itsjustkevin commented Mar 8, 2024

Uh oh!

itsjustkevin commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bleroux commented Feb 21, 2024 •

edited

Loading

bleroux commented Feb 21, 2024 •

edited

Loading

itsjustkevin commented Mar 8, 2024 •

edited

Loading