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

feat: Add a setting to respect system alerts while detecting active apps #907

Merged
merged 1 commit into from
Jun 1, 2024

Conversation

mykola-mokhnach
Copy link

Currently we detect the app under test as active if XCTest returns RunningInForeground state for it. In case the app is covered by a system alert from springboard this might be confusing as we cannot interact with it unless an alert is properly handled. The new setting called respectSystemAlerts being set to true (by default it is false) forces WDA to verify the presence of alerts shown by springboard and return the latter if an alert is shown. This affects the performance of active app detection, but might be more convenient for writing test script (e.g. eliminates the need of proactive switching between system and custom apps).

@@ -34,3 +34,4 @@
NSString* const FB_SETTING_WAIT_FOR_IDLE_TIMEOUT = @"waitForIdleTimeout";
NSString* const FB_SETTING_ANIMATION_COOL_OFF_TIMEOUT = @"animationCoolOffTimeout";
NSString* const FB_SETTING_MAX_TYPING_FREQUENCY = @"maxTypingFrequency";
NSString* const FB_SETTING_RESPECT_SYSTEM_ALERTS = @"respectSystemAlerts";
Copy link
Author

Choose a reason for hiding this comment

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

I was not quite sure about the setting name. Maybe you'd have better proposals

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.

the change itself lg, let me think of the naming

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.

lgtm. "system alert" naming is reasonable to imagine this setting.

Copy link

@eglitise eglitise left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. From what I can tell this only handles XCUIElementTypeAlert elements, which should cover most cases, but I vaguely recall (from my own projects) that there may be some other element types as well (e.g. picker wheels or file sharing modals). I will check this on Monday/Tuesday, but those can also be handled in a separate PR if necessary.

@mykola-mokhnach
Copy link
Author

Thanks for doing this. From what I can tell this only handles XCUIElementTypeAlert elements, which should cover most cases, but I vaguely recall (from my own projects) that there may be some other element types as well (e.g. picker wheels or file sharing modals). I will check this on Monday/Tuesday, but those can also be handled in a separate PR if necessary.

I don't remember any cases where springboard shows something different from an alert. Please provide examples of the system app page source for such cases.

@KazuCocoa
Copy link
Member

I'm also curious about other cases.

So far, I have checked like below share features before. They were not managed by springboard, so the same app scope worked. Default photos, a couple of 3rd party apps. Actually, they could depend on the app implementation so this might not be the all cases.

Screenshot 2024-06-01 at 10 54 29 AM Screenshot 2024-06-01 at 10 56 00 AM

After seeing the comment, I checked some data picker (XCUIElementTypeDatePicker, XCUIElementTypePickerWheel) implementations in a few apps like apple's calender and 3rd party apps I knew of, they worked in their own scope (so not springboard)

Screenshot 2024-06-01 at 11 43 26 AM

@eglitise
Copy link

eglitise commented Jun 1, 2024

I will check my project on Monday/Tuesday, but I don't think it should block this PR.

@mykola-mokhnach mykola-mokhnach merged commit 5c82d66 into master Jun 1, 2024
47 checks passed
@mykola-mokhnach mykola-mokhnach deleted the springboard_alerts branch June 1, 2024 20:58
github-actions bot pushed a commit that referenced this pull request Jun 1, 2024
## [8.7.0](v8.6.0...v8.7.0) (2024-06-01)

### Features

* Add a setting to respect system alerts while detecting active apps ([#907](#907)) ([5c82d66](5c82d66))
Copy link

github-actions bot commented Jun 1, 2024

🎉 This PR is included in version 8.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eglitise
Copy link

eglitise commented Jun 3, 2024

Okay, I have checked a bit and found 2 cases where springboard places something other than an alert over the app:

  • Floating Picture-in-Picture window
    • This case is interesting because the PiP is added to both the app and springboard XML sources, but only the springboard XML contains all the correct window details
    • XCUITest driver <6 did not detect this and remained in the same app
  • TestFlight's 'From the Developer' and 'Share Feedback' screens - here is an example with screenshots
    • XCUITest driver <6 detected this and switched to the springboard as the active app

I will have more time to investigate the XMLs tomorrow.

@eglitise
Copy link

eglitise commented Jun 4, 2024

@mykola-mokhnach here is a gist with XMLs for the two TestFlight screens mentioned above (edited for privacy): https://gist.github.com/eglitise/4704401148fef1db400e8e6be782973b

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.

None yet

3 participants