Closed Bug 1086680 Opened 6 years ago Closed 6 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)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

References

()

Details

Attachments

(5 files, 9 obsolete files)

15.48 KB, patch
Details | Diff | Splinter Review
19.07 KB, text/html
Details
36.39 KB, image/png
Details
58.94 KB, image/png
Details
46 bytes, text/x-github-pull-request
jlorenzo
: review+
Details | Review
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/
Component: Gaia::UI Tests → Gaia::Contacts
Thanks! Very useful. What were the problems with it?
This is bug is about adding a new Gaia UI test => Moving to that component.
Component: Gaia::Contacts → Gaia::UI Tests
(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)
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)
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.
Attachment #8514247 - Flags: review?(florin.strugariu)
(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.
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
(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.
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)
Do we have a fb account setup and added to the credentials repo?

https://github.com/mozilla/webqa-credentials
Flags: needinfo?(martijn.martijn)
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-
Attached file facebook_login.html
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)
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8,
Flags: in-qa-testsuite?(martijn.martijn)
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)
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
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.
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)
(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.
Blocks: 1086687
(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.
Attachment #8516719 - Flags: review?(florin.strugariu) → review-
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)
Attachment #8517764 - Flags: review?(florin.strugariu) → review-
Attached image Facebook fail (obsolete) —
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.
(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
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8, → fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+
Depends on: 1100342
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+
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
Attached file facebook_import5 (obsolete) —
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
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.
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 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-
In Portland, the automated test results in this problem. I guess that is impossible to circumvent when logging in in a different country.
Now I get this dialog again.
Attached file facebook_import6 (obsolete) —
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)
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)
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 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.
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 on attachment 8530977 [details] [review]
facebook_import6

Looks good to me!
Flags: needinfo?(jlorenzo)
Attachment #8530977 - Flags: review?(jlorenzo) → review+
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+
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
Kicked of a Jenkins run with 20 repeats: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/469/
Attached patch facebook_import7 (obsolete) — Splinter Review
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)
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+
Attachment #8539383 - Flags: review?(jlorenzo)
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/
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.
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
Ok, Jenkins try runs all passed (21 times).
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.
Attached file facebook_import8
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)
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 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+
Merged in master https://github.com/mozilla-b2g/gaia/commit/3fdccaac6fb2de3f047204f9f749dabdb90efbba
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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+
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.