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

Conversation

@jpnurmi
Copy link
Member

@jpnurmi jpnurmi commented May 27, 2022

This PR is a continuation of #33111 and includes necessary changes to test the TextInputType.setClient() platform channel call. Testing the actual text input flow requires a mock IM context that will follow in a separate PR.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

jpnurmi added 3 commits May 27, 2022 13:31
This allows passing an offscreen window for testing purposes.
Keep track of registered message handlers and allow tests to simulate
receiving messages.
Copy link
Contributor

@robert-ancell robert-ancell left a comment

Choose a reason for hiding this comment

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

Code LGTM.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm with a minor suggestion.

@cbracken
Copy link
Member

Looks like there's a legitimate test failure to fix in FlTextInputPluginTest.SetClient (maybe due to running headless?).

@chinmaygarde
Copy link
Member

@jpnurmi Any updates on the test failures?

jpnurmi added 3 commits June 10, 2022 13:27
GdkOffscreenWindow requires display & screen which is a problem in
headless mode. Luckily, we don't really need a window instance in tests
because its only passed as a client window to the IM context which will
be mocked out when we get to tests that depend on it.
@jpnurmi
Copy link
Member Author

jpnurmi commented Jun 10, 2022

I was silly and ignored the harmless warnings from GdkWindow but the warnings would become a problem when made fatal. Luckily, we don't really need a window instance in the text input tests because it's only passed as a client window to the IM context which will be mocked out when we get to tests that depend on it.

@cbracken cbracken merged commit bae667e into flutter:main Jun 10, 2022
@jpnurmi jpnurmi deleted the text-input-window branch June 10, 2022 18:19
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2022
@jpnurmi jpnurmi mentioned this pull request Jun 21, 2022
8 tasks
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
This PR is a continuation of flutter#33111 and includes necessary changes to test the `TextInputType.setClient()` platform channel call. Testing the actual text input flow requires a mock IM context that will follow in a separate PR.

* [Linux] pass GdkWindow instead of FlView to FlTextInputPlugin
* [Linux] add MockBinaryMessenger::ReceiveMessage()
* [Linux] add test for TextInputType.setClient
* Drop the offscreen window

GdkOffscreenWindow requires display & screen which is a problem in
headless mode. Luckily, we don't really need a window instance in tests
because its only passed as a client window to the IM context which will
be mocked out when we get to tests that depend on it.
@jpnurmi
Copy link
Member Author

jpnurmi commented Jun 21, 2022

I've pushed the next step: #34186

cbracken pushed a commit that referenced this pull request Jun 21, 2022
This PR finishes what #33661 and #33111 started - most of `FlTextInputPlugin` is now covered by tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants