Closed
Bug 1086680
Opened 10 years ago
Closed 9 years ago
Write an automated test for "Verify the user can import Facebook contacts from contacts app settings" moztrap smoketest
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: martijn.martijn, Assigned: martijn.martijn)
References
()
Details
Attachments
(5 files, 9 obsolete files)
This moztrap test https://moztrap.mozilla.org/manage/case/5857/ This test case is intended to verify that facebook contacts are correctly imported from Contacts app settings Open the contacts app. Tap on the settings icon in the top right. (Gear Icon) Toggle on Enable Facebook option Fill in a correct username and password Tap on login Wait for the list of friends to be completely loaded Select all friends Press import After waiting for import process to be completed, verify that every contact you have imported is in the contact list with the correct information (photo, first and last names, birthdate...). Here are already some tests that might be useful to look at: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/contacts/
Assignee | ||
Updated•10 years ago
|
Component: Gaia::UI Tests → Gaia::Contacts
Comment 1•10 years ago
|
||
I try to implement this test case last year, for your reference. https://github.com/askeing/gaia-ui-tests/blob/Import_facebook_contacts/gaiatest/tests/contacts/test_import_facebook.py
Assignee | ||
Comment 2•10 years ago
|
||
Thanks! Very useful. What were the problems with it?
Comment 3•10 years ago
|
||
This is bug is about adding a new Gaia UI test => Moving to that component.
Component: Gaia::Contacts → Gaia::UI Tests
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Askeing Yen[:askeing] from comment #1) > I try to implement this test case last year, for your reference. > https://github.com/askeing/gaia-ui-tests/blob/Import_facebook_contacts/ > gaiatest/tests/contacts/test_import_facebook.py Askeing, thanks for this. What problems did you find with this? What Facebook credentials did you use in testvars.json? Do we have a special qa account for this?
Flags: needinfo?(fyen)
Comment 5•10 years ago
|
||
Hi Martijn, In my impression, there is no problem with this test. Zac and I had some discussion about it, but I forgot the reason why we don't merge it last year. Since we close the gaia-ui-tests github repo, so we lose the issue/pr history. I don't have special qa account, I use my FB account.
Flags: needinfo?(fyen)
Assignee | ||
Comment 6•10 years ago
|
||
This is WIP, although it works fine on device. On desktop, the Facebook import dialog doesn't work, so this test has to be disabled for desktop b2g. I get Invalid app ID: 123456 there. Askeing, thans, it turned out, the test had to be changed quite a bit to be useful. In the end, test_import_contacts_from_gmail.py was more useful as a template. There might be a problem when Facebook loads very slowly, although when repeating this test 10 times, it didn't fail.
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8514247 -
Flags: review?(florin.strugariu)
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #7) > Created attachment 8514247 [details] [review] > https://github.com/mozilla-b2g/gaia/pull/25651 I forgot to add skip-if = device == "desktop" to the manifest.ini file, because the facebook site doesn't work for some reason.
Comment 9•10 years ago
|
||
Are you going to use a live facebook account for this? Are you sure it won't get blocked and or be unreliable? Also you should add the default fields for the username and password into the testvars_template.json
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Zac C (:zac) from comment #9) > Are you going to use a live facebook account for this? Are you sure it won't > get blocked and or be unreliable? Yes, that's a risk. Otoh, this is tied into the current phone experience. If Facebook is sending something bad, then this should still be reported, right? > Also you should add the default fields for the username and password into > the testvars_template.json Yeah, I forgot that too.
Assignee | ||
Comment 11•10 years ago
|
||
New branch with those changes. Note that the name and password of the facebook account is not given in this patch (for obvious reasons).
Attachment #8514247 -
Attachment is obsolete: true
Attachment #8514247 -
Flags: review?(florin.strugariu)
Attachment #8514454 -
Flags: review?(florin.strugariu)
Comment 12•10 years ago
|
||
Do we have a fb account setup and added to the credentials repo? https://github.com/mozilla/webqa-credentials
Flags: needinfo?(martijn.martijn)
Comment 13•10 years ago
|
||
Comment on attachment 8514454 [details] [review] https://github.com/mozilla-b2g/gaia/pull/25666 Looks good but some updates are needed Also make sure you ask at least 2 reviews
Attachment #8514454 -
Flags: review?(viorela.ioia)
Attachment #8514454 -
Flags: review?(florin.strugariu)
Attachment #8514454 -
Flags: review-
Assignee | ||
Comment 14•10 years ago
|
||
I'm not able to access the facebook login dialog page, using webIDE, so I'm just posting the contents of that page here, in case I need it further.
Flags: needinfo?(martijn.martijn)
Assignee | ||
Updated•10 years ago
|
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8,
Flags: in-qa-testsuite?(martijn.martijn)
Assignee | ||
Comment 15•10 years ago
|
||
I updated the pull request with your review comments from https://github.com/mozilla-b2g/gaia/pull/25666/files I'm now using much better selectors for the Facebook login page, I think.
Attachment #8514454 -
Attachment is obsolete: true
Attachment #8514454 -
Flags: review?(viorela.ioia)
Attachment #8516234 -
Flags: review?(florin.strugariu)
Assignee | ||
Comment 16•10 years ago
|
||
Sometimes the gaia-ui-test is stalling on line 23 of facebook.py in __init__: email = self.wait_for_element_present(*self._email_locator) This is happening when the Facebook login page doesn't appear as quickly. In that case, you'll see a black page for at least 5 seconds, then the Facebook login page appears. Then the gaia-ui-test keeps on stalling, because it can't seem to find the email login element, even though it has appeared by then. I tried to get a reproducable case out of this, that resulted in bug 1093386. Though, it's not exactly the same issue, because here, the Facebook login page appears, but Marionette somehow can't find the element, even though it has become visible.
Depends on: 1093386
Assignee | ||
Comment 17•10 years ago
|
||
I also encountered this problem (which I mentioned in bug 989562, comment19), when I went the Marketplace route of the "write a test for installing a packaged app" test.
Assignee | ||
Comment 18•10 years ago
|
||
Ok, doing this seems to work reliably: + self.marionette.execute_async_script(""" + setInterval(function() { + if (content.document.querySelector('input[placeholder^="Email"]')) + marionetteScriptFinished() + }, 1000); + """, script_timeout=30000) With self.wait_for_element_present(*self._email_locator), I just get timeouts sometimes, even though the email text input is clearly visible.
Attachment #8516234 -
Attachment is obsolete: true
Attachment #8516234 -
Flags: review?(florin.strugariu)
Attachment #8516719 -
Flags: review?(florin.strugariu)
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #16) > This is happening when the Facebook login page doesn't appear as quickly. In > that case, you'll see a black page for at least 5 seconds, then the Facebook I filed bug 1093671 for the black page issue. It doesn't seem to be related to the "marionette not able to find element" issue.
Assignee | ||
Comment 20•10 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #18) > With self.wait_for_element_present(*self._email_locator), I just get > timeouts sometimes, even though the email text input is clearly visible. I tried to use the new methods mentioned in the gaia-ui-automation mailing list (Changes to how we wait_for things in Gaia tests), e.g.: element = Wait(self.marionette).until(expected.element_present(*self._email_locator)) Wait(self.marionette).until(expected.element_displayed(element)) That also didn't help with these timeouts that sometimes occur.
Updated•10 years ago
|
Attachment #8516719 -
Flags: review?(florin.strugariu) → review-
Assignee | ||
Comment 21•10 years ago
|
||
This is using the Wait() methods that are mandated now. Note that settings_form.py needs these changes for existing stuff, which Dave Hunt will do in bug 1074117.
Attachment #8516719 -
Attachment is obsolete: true
Attachment #8517764 -
Flags: review?(florin.strugariu)
Updated•10 years ago
|
Attachment #8517764 -
Flags: review?(florin.strugariu) → review-
Comment 22•10 years ago
|
||
Looks like Facebook is not letting me use the same account don different devices We might see this issue a lot when we run this test on the grid.
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to Florin Strugariu [:Bebe] from comment #22) > Created attachment 8518185 [details] > Facebook fail > > Looks like Facebook is not letting me use the same account don different > devices > > We might see this issue a lot when we run this test on the grid. Ok, so we really need to use developer Facebook accounts, bug 1018307. But we need to be able to change the appId for the Facebook import dialog app for that, which is currently not possible.
Depends on: 1018307
Assignee | ||
Updated•10 years ago
|
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8, → fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+
QA Whiteboard: fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+ → fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+
Assignee | ||
Updated•10 years ago
|
QA Whiteboard: fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+ → fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-s5
Assignee | ||
Comment 24•10 years ago
|
||
Ok, this makes use of the Facebook developer account (e.g. appId 323630664378726). It is done in a really hacky way, though. I don't really know of any other way to change the Facebook applicationId.
Attachment #8517764 -
Attachment is obsolete: true
Attachment #8518185 -
Attachment is obsolete: true
Assignee | ||
Comment 25•10 years ago
|
||
I would have thought, this would work when executing while the Contacts app is open: self.marionette.execute_script("""window.wrappedJSObject.oauthflow.params.facebook.applicationId = 323630664378726;""") But it didn't.
Assignee | ||
Comment 26•10 years ago
|
||
Comment on attachment 8528354 [details] [review] facebook_import5 What do you guys think of the hack? I added you the credentials to the Facebook account that you need to add to your testvars.json file, in order for this test to try out.
Attachment #8528354 -
Flags: review?(jlorenzo)
Attachment #8528354 -
Flags: review?(florin.strugariu)
Comment 27•10 years ago
|
||
Comment on attachment 8528354 [details] [review] facebook_import5 This hacks seems better than nothing to me. We can clean things though. I left some comments in the PR.
Attachment #8528354 -
Flags: review?(jlorenzo) → review-
Assignee | ||
Comment 28•10 years ago
|
||
In Portland, the automated test results in this problem. I guess that is impossible to circumvent when logging in in a different country.
Assignee | ||
Comment 29•10 years ago
|
||
Now I get this dialog again.
Assignee | ||
Comment 30•10 years ago
|
||
This verification ID dialog problem needs to be solved, because otherwise it might pop up in the automated testing machines. I thought Facebook developer accounts wouldn't show these verification ID dialogs. Is there a way to turn it off for those?
Attachment #8528354 -
Attachment is obsolete: true
Attachment #8528354 -
Flags: review?(florin.strugariu)
Flags: needinfo?(jlorenzo)
Comment 31•10 years ago
|
||
The account you use is a real account from Facebook's perspective. You need to use a test account created with the Facebook developer page (or API). You can use for instance: gaia_dxnzuri_tests@tfbnw.net. I'll give you the password in private.
Flags: needinfo?(jlorenzo)
Assignee | ||
Comment 32•10 years ago
|
||
Comment on attachment 8530977 [details] [review] facebook_import6 Ok, Malina pointed me out to self.marionette.navigate, which seems to work very reliable and makes it much more cleaner.
Attachment #8530977 -
Flags: review?(jlorenzo)
Comment 33•10 years ago
|
||
Comment on attachment 8530977 [details] [review] facebook_import6 Waw! The navigate function makes this test way cleaner! I don't see any major issue on the test. I think we should add/clean some comments in the code before merging the PR. See the details there.
Assignee | ||
Comment 34•10 years ago
|
||
Yeah, it sometimes really helps if you can ask the guru (mdas) in person. I think I followed all of your suggestions, let me know if this is good enough.
Flags: needinfo?(jlorenzo)
Comment 35•10 years ago
|
||
Comment on attachment 8530977 [details] [review] facebook_import6 Looks good to me!
Flags: needinfo?(jlorenzo)
Attachment #8530977 -
Flags: review?(jlorenzo) → review+
Assignee | ||
Comment 36•10 years ago
|
||
Comment on attachment 8530977 [details] [review] facebook_import6 Thanks, adding geo as second reviewer.
Attachment #8530977 -
Flags: review?(gmealer)
Comment on attachment 8530977 [details] [review] facebook_import6 Looks good!
Attachment #8530977 -
Flags: review?(gmealer) → review+
Assignee | ||
Comment 38•10 years ago
|
||
Ok, this will have to wait for bug 1108845 to be fixed. And then I would like to test it on treeherder a whole bunch of times, to make sure it doesn't cause intermittent failures.
Depends on: 1108845
Assignee | ||
Comment 39•10 years ago
|
||
Kicked of a Jenkins run with 20 repeats: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/469/
Assignee | ||
Comment 40•10 years ago
|
||
I had some small issue with updating this pull request to latest master, in settings_form.py and manifest.ini. But really, it's almost nothing. I wonder if I should ask review for it. I needed to update my pull request, because I wanted to kick off a Jenkins ad hoc test run, but that didn't work, because Marionette was updated, so I had to update the branch, but I don't know currently how to update the remote branch (and I think I would have gotten these merge conflicts too, then).
Attachment #8530977 -
Attachment is obsolete: true
Attachment #8539383 -
Flags: review?(gmealer)
Assignee | ||
Comment 41•10 years ago
|
||
I kicked of a new Jenkins adhoc test run, hopefully it works this time: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/483/
Comment on attachment 8539383 [details] [diff] [review] facebook_import7 It still looks ok to me, though I'm so far away from the last review I can't really correlate the two. Assuming you have no problem passing try, I'm fine with checking it in.
Attachment #8539383 -
Flags: review?(gmealer) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8539383 -
Flags: review?(jlorenzo)
Assignee | ||
Comment 43•10 years ago
|
||
I kicked of a new Jenkins try run, I did something wrong with the previous one: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/506/
Assignee | ||
Comment 44•10 years ago
|
||
Hrm, it failed 3 times out of 21. Perhaps I should first do the Jenkins try runs before asking review, especially for these kind of external tests. I think I know this problem. The screenshots show a white page, I recognize that problem. That sometimes happens when you quickly reload into a different url. I got the impression, I didn't had that problem with navigate(). I guess I was wrong. The solution is to wait for the first page to be loaded, and only then load the page from the developer account.
Assignee | ||
Comment 45•10 years ago
|
||
I've updated the pull request with these changes: --- a/tests/python/gaia-ui-tests/gaiatest/apps/contacts/regions/facebook.py +++ b/tests/python/gaia-ui-tests/gaiatest/apps/contacts/regions/facebook.py @@ -14,7 +14,7 @@ class FacebookLogin(Base): _title_locator = (By.CSS_SELECTOR, 'title') _email_locator = (By.CSS_SELECTOR, 'input[placeholder^="Email"]') _password_locator = (By.CSS_SELECTOR, 'input[placeholder^="Password"]') - _submit_locator = (By.CSS_SELECTOR, 'button[value^="Log In"]') + _submit_locator = (By.CSS_SELECTOR, '*[type="submit"]') I had to do this, because the button on the facebook login page was changed to an input with value "Log in" for me. So I'm using type="submit" now as selector, since there is only 1 submit on that page and it seems more stable as a selector that way. Not sure why this didn't cause problems in the previous Jenkins test run, btw. Perhaps, in the US, that page still looks like it used to? def __init__(self, marionette): Base.__init__(self, marionette) @@ -34,6 +34,8 @@ class FacebookLogin(Base): str = str.replace('395559767228801', '323630664378726') self.marionette.switch_to_frame(view) + # Wait until the original page has loaded first, otherwise trying to load the 2nd page causes a blank page + Wait(self.marionette, timeout=60).until(expected.element_present(*self._email_locator)) self.marionette.navigate(str) Wait(self.marionette, timeout=60).until(expected.element_present(*self._email_locator)) That change should ensure that no blank page appear in the facebook import dialog. Locally, it's passing on device 21 times, just fine: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/510/ https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=58651b278b35
Assignee | ||
Comment 46•10 years ago
|
||
Ok, Jenkins try runs all passed (21 times).
Assignee | ||
Comment 47•10 years ago
|
||
Sigh, while testing the previous pull request, treeherder try was busted for some reason. But now I noticed that I did something wrong, causing the test to fail on desktop. I updated the pull request also with this: --- a/tests/python/gaia-ui-tests/gaiatest/apps/contacts/regions/facebook.py +++ b/tests/python/gaia-ui-tests/gaiatest/apps/contacts/regions/facebook.py @@ -11,7 +11,7 @@ from gaiatest.apps.base import Base class FacebookLogin(Base): _iframe_locator = (By.CSS_SELECTOR, 'iframe[data-url*="m.facebook.com"]') - _title_locator = (By.CSS_SELECTOR, 'title') + _div_locator = (By.CSS_SELECTOR, 'div') _email_locator = (By.CSS_SELECTOR, 'input[placeholder^="Email"]') _password_locator = (By.CSS_SELECTOR, 'input[placeholder^="Password"]') _submit_locator = (By.CSS_SELECTOR, '*[type="submit"]') @@ -34,8 +34,9 @@ class FacebookLogin(Base): str = str.replace('395559767228801', '323630664378726') self.marionette.switch_to_frame(view) - # Wait until the original page has loaded first, otherwise trying to load the 2nd page causes a blank page - Wait(self.marionette, timeout=60).until(expected.element_present(*self._email_locator)) + # Wait until the original page has loaded a bit, because sometimes, + # trying to load the 2nd page directly after the first, causes a blank page + Wait(self.marionette, timeout=60).until(expected.element_present(*self._div_locator)) self.marionette.navigate(str) Wait(self.marionette, timeout=60).until(expected.element_present(*self._email_locator)) This should make it work on desktop and device.
Assignee | ||
Comment 48•9 years ago
|
||
Unfortunately, I had to update my pull request, because apparently, treeherder try runs were broken when I created the previous pull request. In any case, it doesn't matter that much, because this is an "external = true" test, which is skipped by treeherder try.
Attachment #8539383 -
Attachment is obsolete: true
Attachment #8539383 -
Flags: review?(jlorenzo)
Assignee | ||
Comment 49•9 years ago
|
||
Comment on attachment 8541885 [details] [review] facebook_import8 New Jenkins adhoc test run: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/522/ All 30 runs passed. Sorry for all the churn here, see comment 45 and further for the changes I made further. Note that this test only will run on Jenkins, because this is an "external = true" test.
Attachment #8541885 -
Flags: review?(jlorenzo)
Comment 50•9 years ago
|
||
Comment on attachment 8541885 [details] [review] facebook_import8 Still looks good to me after these small changes. Thank you for fixing the intermittent failure, that test was definitely a tough one.
Attachment #8541885 -
Flags: review?(jlorenzo) → review+
Comment 51•9 years ago
|
||
Merged in master https://github.com/mozilla-b2g/gaia/commit/3fdccaac6fb2de3f047204f9f749dabdb90efbba
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•9 years ago
|
QA Whiteboard: fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-s5 → fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-s7
Flags: in-qa-testsuite?(martijn.martijn) → in-qa-testsuite+
Assignee | ||
Updated•9 years ago
|
QA Whiteboard: fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-s7 → fxosqa-auto-from-s2, fxosqa-auto-dropped-s3, fxosqa-auto-points=8, [fxosqa-auto-s7]
You need to log in
before you can comment on or make changes to this bug.
Description
•