-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #7563 - Sync integration logins #7564
For #7563 - Sync integration logins #7564
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7564 +/- ##
=========================================
Coverage ? 19.35%
Complexity ? 456
=========================================
Files ? 300
Lines ? 11435
Branches ? 1538
=========================================
Hits ? 2213
Misses ? 9053
Partials ? 169 Continue to review full report at Codecov.
|
app/src/androidTest/java/org/mozilla/fenix/syncintegration/test_logins.js
Show resolved
Hide resolved
mDevice.waitNotNull(Until.findObjects(By.text("On")), TestAssetHelper.waitingTime) | ||
} | ||
} | ||
|
||
/* These tests will be running in the future |
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 this are we waiting on a feature to land?
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.
Not really, I just need to work on those one which are in the direction Fenix -> Desktop
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.
once @KMaragh approves and firebase passing, 👍 Let's also make sure we have manual steps recoreded in TestRail
}.openThreeDotMenu { | ||
}.openSettings { | ||
// Necessary to scroll a little bit for all screen sizes | ||
TestHelper.scrollToElementByText("Logins and passwords") |
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.
Maybe move this scroll portion into the openLoginsAndPasswordSubMenu
method, since any time you need to open that subMenu, you will most likely have to scroll.
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.
Makes sense, I added it to Settings Robot though since it is there where we need to scroll so that the Logins And Passwords menu is accessible
settingsSubMenuLoginsAndPassword { | ||
mDevice.waitNotNull(Until.findObjects(By.text("Sync logins")), TestAssetHelper.waitingTime) | ||
verifyDefaultView() | ||
mDevice.waitNotNull(Until.findObjects(By.text("Off")), TestAssetHelper.waitingTime) |
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.
Can both of these object checks for 'sync logins' and 'Off' be moved into the verifyDefaultView
method?
I'm not sure what the method does but it might fit better in there, or in their own methods to maintain readability of this test file.
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.
Sure, moved there. The verifyDefaultView
checks the UI, that the options are correct and so
// Discard the secure your device message | ||
tapSetupLater() | ||
// Check the logins synced | ||
mDevice.waitNotNull(Until.findObjects(By.text("https://accounts.google.com")), TestAssetHelper.waitingTime) |
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.
Same as above. This feels like it can be placed within the 'verifySavedLoginsAfterSync' method
Fixes #7563
Pull Request checklist
After merge
To download an APK when reviewing a PR: