Delete scenario test TextSemanticsFocusTest by jmagman · Pull Request #29370 · flutter/engine

jmagman · 2021-10-28T00:37:44Z

This test was previously marked flaky due to a test harness crash flutter/flutter#55778. Xcode has since been updated, but this test is still failing when unskipped. Delete, since it's been off for awhile.

Fixes flutter/flutter#55778

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. See [testing the engine] for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

jmagman · 2021-10-28T00:37:56Z

...ing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/xcshareddata/xcschemes/Scenarios.xcscheme

 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1020"
+   LastUpgradeVersion = "1300"


Let Xcode update this.

chinmaygarde · 2021-11-11T21:18:28Z

Let's see if test is still flaky.

Looks like the test is failing still. Should we close this?

Also, clean up the XCUITest element queries a bit.

Perhaps just the cleanups can be isolated for a patch?

jmagman · 2021-11-11T23:16:02Z

Let's see if test is still flaky.

Looks like the test is failing still. Should we close this?

Also, clean up the XCUITest element queries a bit.

Perhaps just the cleanups can be isolated for a patch?

This was supposed to be a draft, I added a bunch of logging to see why it's failing but I haven't had a chance to look at it.

jmagman · 2022-02-17T02:56:44Z

testing/scenario_app/lib/src/send_text_focus_semantics.dart

  /// Constructor for `SendTextFocusScemantics`.
  SendTextFocusSemantics(PlatformDispatcher dispatcher) : super(dispatcher);

+  bool _firstFrameDrawn = false;


From flutter/flutter#55778 (comment)

It looks like the first frame of the test is supposed to fake a text field in onBeginFrame

engine/testing/scenario_app/lib/src/send_text_focus_semantics.dart

Lines 39 to 41 in 2962099

// On the first frame, also pretend that it drew a text field. Send the

// corresponding semantics tree comprised of 1 node for the text field.

window.updateSemantics((SemanticsUpdateBuilder()

Then the test taps somewhere:

engine/testing/scenario_app/ios/Scenarios/ScenariosUITests/TextSemanticsFocusTest.m

Lines 66 to 67 in 2962099

// The tap location doesn't matter. The mock framework just sends a focused text field on tap.

[textInputSemanticsObject tap];

Which triggers onPointerDataPacket, which "focuses" the fake text field:

engine/testing/scenario_app/lib/src/send_text_focus_semantics.dart

Lines 108 to 109 in 2962099

// SemanticsFlag.isTextField and SemanticsFlag.isFocused.

flags: 48,

When I attach, I right after the tap I can see node 0 correctly being updated with flutter::SemanticsFlags::kIsFocused.

engine/lib/ui/semantics/semantics_update_builder.cc

Line 135 in 2962099

nodes_[id] = node;

But then onBeginFrame is immediately called again (maybe because of the keyboard animating in?) and it resets node 0 back to an unfocused text field.

The test passes when I make this change to only call the onBeginFrame logic to create the fake text input only once, the first time it's called.

The test passes

Well, on my machine... it failed in CI.
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8822017057875020497/+/u/test:_Scenario_App_Integration_Tests/stdout

chunhtai

LGTM

cyanglaz

LGTM

…9370)

jmagman added the platform-ios label Oct 28, 2021

jmagman self-assigned this Oct 28, 2021

google-cla bot added the cla: yes label Oct 28, 2021

jmagman commented Oct 28, 2021

View reviewed changes

jmagman marked this pull request as draft November 11, 2021 23:16

CaseyHillers changed the base branch from master to main November 15, 2021 18:13

fluttergithubbot force-pushed the master branch from b1a8771 to 2962099 Compare November 15, 2021 18:30

godofredoc changed the base branch from master to main November 24, 2021 07:11

jmagman force-pushed the unskip-semantics branch from 7f27f22 to 4b0771b Compare February 10, 2022 02:01

jmagman added 3 commits February 16, 2022 17:25

Unskip scenario test TextSemanticsFocusTest

322188e

log

56f722c

Do not clobber first frame after tap

ce055ad

jmagman force-pushed the unskip-semantics branch from 4b0771b to ce055ad Compare February 17, 2022 02:45

jmagman commented Feb 17, 2022

View reviewed changes

Delete

b28d14d

jmagman changed the title ~~Unskip scenario test TextSemanticsFocusTest~~ Delete scenario test TextSemanticsFocusTest Feb 17, 2022

jmagman marked this pull request as ready for review February 17, 2022 20:00

jmagman requested review from chunhtai and cyanglaz February 17, 2022 20:00

chunhtai approved these changes Feb 17, 2022

View reviewed changes

cyanglaz approved these changes Feb 17, 2022

View reviewed changes

jmagman added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 17, 2022

fluttergithubbot merged commit c5e89cb into flutter:main Feb 17, 2022

jmagman deleted the unskip-semantics branch February 17, 2022 23:34

engine-flutter-autoroll mentioned this pull request Feb 18, 2022

Roll Engine from aa07e8f9304d to c5e89cbb7377 (6 revisions) flutter/flutter#98713

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 18, 2022

c5e89cb Delete scenario test TextSemanticsFocusTest (flutter/engine#2…

ef245be

…9370)

jmagman added the affects: tests label Feb 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete scenario test TextSemanticsFocusTest#29370

Delete scenario test TextSemanticsFocusTest#29370
fluttergithubbot merged 4 commits intoflutter:mainfrom
jmagman:unskip-semantics

jmagman commented Oct 28, 2021 •

edited

Loading

Uh oh!

jmagman Oct 28, 2021

Uh oh!

chinmaygarde commented Nov 11, 2021 •

edited

Loading

Uh oh!

jmagman commented Nov 11, 2021

Uh oh!

jmagman Feb 17, 2022

Uh oh!

jmagman Feb 17, 2022

Uh oh!

chunhtai left a comment

Uh oh!

cyanglaz left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	// On the first frame, also pretend that it drew a text field. Send the
	// corresponding semantics tree comprised of 1 node for the text field.
	window.updateSemantics((SemanticsUpdateBuilder()

	// The tap location doesn't matter. The mock framework just sends a focused text field on tap.
	[textInputSemanticsObject tap];

	// SemanticsFlag.isTextField and SemanticsFlag.isFocused.
	flags: 48,

Conversation

jmagman commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

jmagman Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

chinmaygarde commented Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman commented Nov 11, 2021

Uh oh!

jmagman Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

jmagman Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jmagman commented Oct 28, 2021 •

edited

Loading

chinmaygarde commented Nov 11, 2021 •

edited

Loading