-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
💡 feat: auto alert configuration with setSettings #20867
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
Comments
When the if (capabilities[FB_SETTING_DEFAULT_ALERT_ACTION]) {
[FBSession initWithApplication:app
defaultAlertAction:(id)capabilities[FB_SETTING_DEFAULT_ALERT_ACTION]];
} else { The method does: + (instancetype)initWithApplication:(nullable XCUIApplication *)application
defaultAlertAction:(NSString *)defaultAlertAction
{
FBSession *session = [self.class initWithApplication:application];
session.alertsMonitor = [[FBAlertsMonitor alloc] init];
session.alertsMonitor.delegate = (id<FBAlertsMonitorDelegate>)session;
session.defaultAlertAction = [defaultAlertAction lowercaseString];
[session.alertsMonitor enable];
return session;
} When the session ends, the alert monitor will be killed like: if (nil != self.alertsMonitor) {
[self.alertsMonitor disable];
self.alertsMonitor = nil;
} So, in this issue, we want to achieve:
When the settings endpoint is called, https://github.com/appium/WebDriverAgent/blob/c296efabe83bf9b2fcd1650c06b363344c5535e0/WebDriverAgentLib/Commands/FBSessionCommands.m#L363 runs. It means we want to define a new settings name to configure the behavior. |
In summary: What we need to do here is when the settings endpoint is called,
All work can be done in WebDriverAgent project. @mykola-mokhnach any opinion especially the settings' naming and the spec in 2)...? [update] |
This makes sense, although the above approach is not very flexible. In many situations accept/dismiss does not project well into alert buttons because of how WDA logic is implemented (and because the limited context that XCTest provides). Eventually, alerts might have 1, 2 or 3 buttons, where "accept"/"dismiss" don't make much sense to me. I was thinking about a possibility to provide a predicate to press a button when an alert pops up for the most flexible approach. For example:
|
Why not maintain some default names for accept/dismiss if not defined, example: for accept alerts : name IN {allow, yes, ok, accept} for dismiss : name IN {no, dismiss, dont} |
I was thinking of using the existing |
This is also not flexible as button names vary as well as could be localized |
Above settings have different purpose. Also they use class chains rather than predicates. |
edited
I probably understood. I was talking about https://github.com/appium/WebDriverAgent/blob/master/WebDriverAgentLib/Routing/FBSession.m#L54-L72 such as So, |
Not really. The alert detection logic remains the same. The given predicate must be executed inside of the alert element.
yes, correct. I assume it should also have priority over other alert-related capabilities |
Gotcha, I am getting a better understanding right now. It makes sense to accept the predicate idea itself for me, btw. Then, what about:
So, What about this...? Actually, my original intention in #20867 (comment) to accept a string as a parameter was to accept a more complex rule for future implementation. |
I would not use accept/dismiss in that context because it might be confusing. This predicate is supposed to click the matched element on an alert as soon as it is detected, whether it would be accept or dismiss or anything else is up to the user to decide based on the provided predicate. If no element has been matched then no click is going to happen alas |
Oh, yea. The method naming example for WDA, when “autoClickAlert” gives predicate, was not intended. E.g. clickPredicateWithError, like that |
Modified the naming in #20867 (comment) as clickAlertPredicateWithError. Also, have created #20938 to do the
If we'd like to add only predicate string only as settings API, I'll close this issue as rejected (since this is to enable |
Resolved by appium/a 6627 ppium-xcuitest-driver#2550 |
[update]
Please take a look at #20867 (comment) comment about this issue's scope.
Is your feature request related to a problem?
autoDismissAlerts feature is currently working with capabilities only and required only during launch and is not allowing to disable it between the tests or at certain steps as some native alert validation is important
Describe the solution you'd like.
set settings to set "autoDismissAlerts", true/false or "appium:autoDismissAlerts", true/false using setSettings as well:
driver.setSetting("autoDismissAlerts", true); driver.setSetting("appium:autoDismissAlerts", true);
driver.setSetting("autoDismissAlerts", false); driver.setSetting("appium:autoDismissAlerts", false);
Describe alternatives you've considered.
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: