Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Handle app startup errors as session creation exceptions #855

Merged
merged 8 commits into from
Mar 3, 2024

Conversation

mykola-mokhnach
Copy link

@mykola-mokhnach mykola-mokhnach commented Feb 28, 2024

.

@mykola-mokhnach
Copy link
Author

@KazuCocoa @Dan-Maor
Could you please help to test this scenario on a real device?

@KazuCocoa
Copy link
Member

i tried to get this situation, but the [self launch]; succeeded without issues, so almost immediately self.fb_didStartWithoutBlockingAlert = @YES; was set (and succeeded).

My tested case was to launch a safari while showing a location permission alert.

If that is the case then calling [XCUIApplication launch] blocks forever.

Do you have any idea what app/alert could cause this case? @mykola-mokhnach

@KazuCocoa
Copy link
Member

when i forcefully removed self.fb_didStartWithoutBlockingAlert = @YES; and kept running the alert check, actually the The application '%@' cannot be launched because it is blocked by an unexpected system alert: %@ was printed. So maybe when the [self launch]; was actually blocked by alert, this should print the error message properly

@mykola-mokhnach
Copy link
Author

i tried to get this situation, but the [self launch]; succeeded without issues, so almost immediately self.fb_didStartWithoutBlockingAlert = @YES; was set (and succeeded).

My tested case was to launch a safari while showing a location permission alert.

If that is the case then calling [XCUIApplication launch] blocks forever.

Do you have any idea what app/alert could cause this case? @mykola-mokhnach

Like I described you could try it with any app signed with enterprise signature, whose enterprise profile has not been accepted on the device yet.

Another possible test case: make springboard to show any alert, for example ios update one and try to install an app without accepting the alert manually. WDA must be already running on the device while doing this

@KazuCocoa
Copy link
Member

let me find possible alerts. Permission dialogs by springboard did not block [self launch]; on my env, so self.fb_didStartWithoutBlockingAlert = @YES; was called almost immediately. Actually free account's code sign could be the candidate as a handy method.

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

lg, i have checked a couple of possible error cases with alert I could occur, but still no blocking behavior by alert for launch API call yet, so I only saw The application '%@' cannot be launched because of an unexpected error: %@ error case without code modification to forcefully reach the blocking code.

the terminate method was better than crashing WDA for me

WebDriverAgentLib/Categories/XCUIApplication+FBLaunch.m Outdated Show resolved Hide resolved
WebDriverAgentLib/Categories/XCUIApplication+FBLaunch.m Outdated Show resolved Hide resolved
@Dan-Maor
Copy link
Collaborator

Dan-Maor commented Mar 3, 2024

I've ran some tests this morning by triggering an untrusted enterprise certificate popup and it didn't block session creation for other applications (tested on a real device running iOS 17.4), so I think this functionality may not be necessary, at least for real devices. Given that it generates additional payload via accessibility operations, I think this mechanism - if the issue can be reproduced on simulators - should be limited for simulators only. What do you think?

@KazuCocoa
Copy link
Member

KazuCocoa commented Mar 3, 2024

it is reasonable for me, or I wonder if this Pr can be just one time alert existence check in https://github.com/appium/WebDriverAgent/pull/855/files#diff-468cd0130bf820fda63ed058220b6b951c0a59ad6e6ad009b1d67daa8b037ca7R97-R99 and add the alert existence in the error response. Then users can notice something alert could remain on the device since such trusted popup could remain on the device, and next time should be "cancel"ed

@mykola-mokhnach
Copy link
Author

I've ran some tests this morning by triggering an untrusted enterprise certificate popup and it didn't block session creation for other applications (tested on a real device running iOS 17.4), so I think this functionality may not be necessary, at least for real devices. Given that it generates additional payload via accessibility operations, I think this mechanism - if the issue can be reproduced on simulators - should be limited for simulators only. What do you think?

Thanks for the feedback @Dan-Maor
I will change the PR to reflect this.

@mykola-mokhnach mykola-mokhnach changed the title feat: Launch app under test with check for a blocking alert presence chore: Handle app startup errors as session creation exceptions Mar 3, 2024
@mykola-mokhnach
Copy link
Author

I have removed the alert presence check. Looks like it was probably a bug in earlier iOS versions.
The only change now is that app launch errors are mapped to a webdriver session creation exception

@mykola-mokhnach mykola-mokhnach merged commit 0ec5398 into master Mar 3, 2024
46 checks passed
@mykola-mokhnach mykola-mokhnach deleted the launch_alert branch March 3, 2024 17:31
github-actions bot pushed a commit that referenced this pull request Mar 3, 2024
## [7.0.6](v7.0.5...v7.0.6) (2024-03-03)

### Miscellaneous Chores

* Handle app startup errors as session creation exceptions ([#855](#855)) ([0ec5398](0ec5398))
Copy link

github-actions bot commented Mar 3, 2024

🎉 This PR is included in version 7.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants