[share] Fix bug on iPad where `origin` is null and enable XCUITests #3188

cyanglaz · 2020-10-22T17:00:57Z

Creates a XCUITests target in the share plugin.

jmagman · 2020-10-26T17:15:22Z

packages/share/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme

@@ -1,87 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>


Did you mean to remove this?

jmagman · 2020-10-26T17:15:44Z

packages/share/example/ios/RunnerUITests/FLTShareExampleUITests.m

+      elementMatchingPredicate:[NSPredicate
+                                   predicateWithFormat:@"label == %@", @"Share With Empty Origin"]];
+  if (![shareWithEmptyOriginButton waitForExistenceWithTimeout:30]) {
+    NSLog(@"%@", app.debugDescription);


Remove the NSLog

This is beneficial so that when a failure happened and we can see what's in the a11y tree and why the particular item was not found in the a11y tree.

Can you print it when it fails? Your test results will get clogged pretty fast with a bunch of debugging lines.

Also NSLog is soft deprecated, should be using os_log.

The log only happens if ![shareWithEmptyOriginButton waitForExistenceWithTimeout:kSecondsToWaitWhenFindingElements] and we send a XCTFail signal afterwards. So it doesn't print unless the test fails.

Thanks for the info of os_log, updated!

Or did you mean to combine the "app.debugDescription" log within XCTFail?

jmagman · 2020-10-26T17:16:06Z

packages/share/example/ios/RunnerUITests/FLTShareExampleUITests.m

+  XCUIElement* shareWithEmptyOriginButton = [app.buttons
+      elementMatchingPredicate:[NSPredicate
+                                   predicateWithFormat:@"label == %@", @"Share With Empty Origin"]];
+  if (![shareWithEmptyOriginButton waitForExistenceWithTimeout:30]) {


30 seconds seems really long.

We used the same waiting length on scenario app tests https://github.com/flutter/engine/blob/master/testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenPlatformViewTests.m#L11
I think the rational is that on ci, the animation might take some time to run and might cause false negative to happen.
This returns immediately after finding the item, so I think setting time-out to be longer should be fine?

jmagman · 2020-10-26T17:16:26Z

packages/share/example/ios/RunnerUITests/FLTShareExampleUITests.m

+                                   predicateWithFormat:@"identifier == %@", @"ActivityListView"]];
+  if (![activityListView waitForExistenceWithTimeout:30]) {
+    NSLog(@"%@", app.debugDescription);
+    XCTFail(@"Failed due to not able to find activityListView with %@ seconds", @(30));


literals as @30

Fixed. Moved all the "30" out to a static

jmagman · 2020-10-26T17:16:31Z

packages/share/example/ios/RunnerUITests/FLTShareExampleUITests.m

+      elementMatchingPredicate:[NSPredicate
+                                   predicateWithFormat:@"identifier == %@", @"ActivityListView"]];
+  if (![activityListView waitForExistenceWithTimeout:30]) {
+    NSLog(@"%@", app.debugDescription);


jmagman · 2020-10-26T17:16:40Z

packages/share/example/ios/RunnerUITests/FLTShareExampleUITests.m

+  XCUIElement* activityListView = [app.otherElements
+      elementMatchingPredicate:[NSPredicate
+                                   predicateWithFormat:@"identifier == %@", @"ActivityListView"]];
+  if (![activityListView waitForExistenceWithTimeout:30]) {


Ditto, 30 seconds seems too long.

jmagman · 2020-10-26T17:17:24Z

packages/share/example/ios/share_exampleUITests/Info.plist

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>


Is this unused?

Thanks for the good catch! Removed.

jmagman

LGTM!

…n the repo (#3210) * enable xctest in .cirrus.yml (#3189) * [share] Fix bug on iPad where `origin` is null and enable XCUITests (#3188)

…n the repo (flutter#3210) * enable xctest in .cirrus.yml (flutter#3189) * [share] Fix bug on iPad where `origin` is null and enable XCUITests (flutter#3188)

Chris Yang added 13 commits October 8, 2020 11:10

fix

82622c5

tests

a9c4a0d

podspec for uitests

1184e36

fix

c5858a9

revert last

8b5cc16

foprmat

0f1017e

add uitest scheme

c69ba2a

add scheme

3056a46

remove podspec test

417c1d9

update ci

cef7ffe

remove dev team

6a9e475

clean up

2a52dce

merge master

0b2e4d2

google-cla bot added the cla: yes label Oct 22, 2020

Chris Yang added 10 commits October 22, 2020 10:04

remove other tasks

422d9ed

cirrus update

cac700e

update

5d1c002

update

9a33e6e

update

8761621

fix

70850c6

remove other tasks

cc1b113

fix

dd5ed17

skip

3346dd2

skip

e97548c

cyanglaz mentioned this pull request Oct 22, 2020

enable xctest in .cirrus.yml #3189

Merged

cyanglaz changed the title ~~Enable xcuitests~~ Enable xcuitests with share plugin Oct 23, 2020

Chris Yang added 3 commits October 26, 2020 09:11

merge cirrus file

0f69172

line EOF

9b8a67a

Merge branch 'xctest' into share_fix

06e22c8

cyanglaz changed the title ~~Enable xcuitests with share plugin~~ [share] Fix a small bug and enable XCUITests Oct 26, 2020

cyanglaz requested review from jmagman and amirh October 26, 2020 16:23

un skip share

9eea9c6

cyanglaz changed the title ~~[share] Fix a small bug and enable XCUITests~~ [share] Fix bug on iPad where origin is null and enable XCUITests Oct 26, 2020

jmagman reviewed Oct 26, 2020

View reviewed changes

Chris Yang added 2 commits October 26, 2020 10:29

review

dba083d

update to use os_log

4c9b8c7

jmagman approved these changes Oct 26, 2020

View reviewed changes

Chris Yang added 2 commits October 26, 2020 11:59

format

9558191

rerun ci

2fbaf56

cyanglaz merged commit dedeae0 into flutter:xctest Oct 27, 2020

cyanglaz pushed a commit that referenced this pull request Oct 27, 2020

[share] Fix bug on iPad where origin is null and enable XCUITests i…

642290b

…n the repo (#3210) * enable xctest in .cirrus.yml (#3189) * [share] Fix bug on iPad where `origin` is null and enable XCUITests (#3188)

This was referenced Oct 27, 2020

[share] Fix bug on iPad where origin is null and enable XCUITests in the repo #3210

Merged

The Share plugin does not work on iPad iOS 13.3 and above flutter/flutter#66485

Closed

[share] Fix bug on iPad where origin is null and enable XCUITests #3188

[share] Fix bug on iPad where origin is null and enable XCUITests #3188

Uh oh!

Conversation

cyanglaz commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[share] Fix bug on iPad where `origin` is null and enable XCUITests #3188

[share] Fix bug on iPad where `origin` is null and enable XCUITests #3188

cyanglaz commented Oct 22, 2020 •

edited

Loading