-
-
Notifications
You must be signed in to change notification settings - Fork 420
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
test: Update e2e-test:native-web-tap tests for WebDriverIO compatibility #1490
Conversation
… comply with the other specs
while removing Travis envionment variable, assuming appium-xcuitest-driver does not use Travis anymore
@mwakizaka Shall we merge this PR or you'd like to do more changes (I can observe some failures during the most recent CI run)? |
oh, let me check. thank you for letting me know. |
'appium:fullReset': true, | ||
'appium:noReset': false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is an appropriate way to avoid ci tests fail (I'd like to just close all tabs in mobile safari before running these tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a possibility to influence global safari settings in Simulator: https://appium.github.io/appium-xcuitest-driver/4.16/execute-methods/#mobile-updatesafaripreferences
Maybe this could help.
Enabling fullReset may significantly slow down tests as it resets the whole simulator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry to be late, so far I haven't managed to find a way to close tabs via mobile: updateSafariPreferences
. To me it looks infeasible.
If no problem, I'm thinking about trying to open Preferences app and close the safari tabs via GUI at before
mocha function.
{w: 1080, h: 2340}, // 13 mini, 12 mini | ||
{w: 1170, h: 2532}, // 14, 13, 13 Pro, 12, 12 Pro | ||
{w: 1284, h: 2778}, // 14 Plus, 13 Pro Max, 12 Pro Max | ||
{w: 1179, h: 2556}, // 14 Pro | ||
{w: 1290, h: 2796}, // 14 Pro Max |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need this to fix nativeWebTapStrict
feature for these devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for sure we need: appium/appium#17014
Maybe we can merge this once..? |
sorry, but maybe I don't have enough time to examine further. if no problem, I'd like to merge this once. |
I would propose to simply mark unstable tests with |
before(async function () { | ||
skipped = false; | ||
try { | ||
await closeAllTabsViaSettingsApp(deviceName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed my mind and modified to close the safari tabs via GUI.
[MEMO]
- As for https://appium.github.io/appium-xcuitest-driver/4.16/execute-methods/#mobile-updatesafaripreferences, I tried removing the contents of
BrowserControllersSavedState
property inLibrary/Preferences/com.apple.mobilesafari.plist
file but it doesn't help. - While playing with Preferences app, I saw a line of device log like
default 22:07:24.666146+0900 Preferences -[UIApplication _beginBackgroundTaskWithName:expirationHandler:]: Will add backgroundTask with taskName: com.apple.mobilesafari.settings.DeleteAllDataAndCachesTask, expirationHandler: <__NSMallocBlock__: 0x600001680a20>
. I guess it is probably a function embedded incom.apple.Preferences
app which closes safari tabs. If it's possible to kick via CUI, it's much better but I don't have any good idea.
The idb test failures were invalid Xcode path things. Maybe we should fix the path. You can ignore it in this PR. |
thank you. I'll check |
I checked the log and concluded that it is not likely related to my changes (most likely similar to appium/appium#15325?). so I'd like to merge this as is once. In case when it turns out to be too flaky, then I'd like to apply #1490 (comment).
|
Hm, yea. https://github.com/appium/appium-remote-debugger/blob/8ad46f6f12b4860670d273335757ba9ee40b2e78/lib/mixins/navigate.js#L102 |
thanks for the update. Re-ran a couple of times, then it passed. I have disabled idb tests as preparation failures. |
I see. thank you for your cooperation! |
#1490 adds `NOTCHED_DEVICE_SIZES`
Ref: appium/appium#16403