1086680 - Write an automated test for "Verify the user can import Facebook contacts from contacts app settings" moztrap smoketest

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Martijn Wargers (dead)]

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/

Comment 1

[Askeing Yen[:askeing][:fyen]]

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

Comment 2

[Martijn Wargers (dead)]

Thanks! Very useful. What were the problems with it?

Comment 3

[Johan Lorenzo [:jlorenzo]]

This is bug is about adding a new Gaia UI test => Moving to that component.

Comment 4

[Martijn Wargers (dead)]

(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?

Comment 5

[Askeing Yen[:askeing][: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.

Comment 6

[Martijn Wargers (dead)]

Attachment: facebook_all.diff

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.

Comment 7

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25651

Comment 8

[Martijn Wargers (dead)]

(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

[Zac C (:zac)]

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

Comment 10

[Martijn Wargers (dead)]

(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.

Comment 11

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25666

New branch with those changes.
Note that the name and password of the facebook account is not given in this patch (for obvious reasons).

Comment 12

[Florin Strugariu [:Bebe]]

Do we have a fb account setup and added to the credentials repo?

https://github.com/mozilla/webqa-credentials

Comment 13

[Florin Strugariu [:Bebe]]

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

Comment 14

[Martijn Wargers (dead)]

Attachment: 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.

Comment 15

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25780

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.

Comment 16

[Martijn Wargers (dead)]

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.

Comment 17

[Martijn Wargers (dead)]

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.

Comment 18

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25810

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.

Comment 19

[Martijn Wargers (dead)]

(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.

Comment 20

[Martijn Wargers (dead)]

(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.

Comment 21

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25859

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.

Comment 22

[Florin Strugariu [:Bebe]]

Attachment: 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.

Comment 23

[Martijn Wargers (dead)]

(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.

Comment 24

[Martijn Wargers (dead)]

Attachment: facebook_import5

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.

Comment 25

[Martijn Wargers (dead)]

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 26

[Martijn Wargers (dead)]

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.

Comment 27

[Johan Lorenzo [:jlorenzo]]

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.

Comment 28

[Martijn Wargers (dead)]

Attachment: (PNG Image, 320 × 569 pixels).png

In Portland, the automated test results in this problem. I guess that is impossible to circumvent when logging in in a different country.

Comment 29

[Martijn Wargers (dead)]

Attachment: (PNG Image, 320 × 569 pixels).png

Now I get this dialog again.

Comment 30

[Martijn Wargers (dead)]

Attachment: facebook_import6

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?

Comment 31

[Johan Lorenzo [: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: [e-mail]. I'll give you the password in private.

Comment 32

[Martijn Wargers (dead)]

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.

Comment 33

[Johan Lorenzo [: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.

Comment 34

[Martijn Wargers (dead)]

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.

Comment 35

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8530977 [details] [review]
facebook_import6

Looks good to me!

Comment 36

[Martijn Wargers (dead)]

Comment on attachment 8530977 [details] [review]
facebook_import6

Thanks, adding geo as second reviewer.

Comment 37

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8530977 [details] [review]
facebook_import6

Looks good!

Comment 38

[Martijn Wargers (dead)]

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.

Comment 39

[Martijn Wargers (dead)]

Kicked of a Jenkins run with 20 repeats: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/469/

Comment 40

[Martijn Wargers (dead)]

Attachment: facebook_import7

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).

Comment 41

[Martijn Wargers (dead)]

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 42

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

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.

Comment 43

[Martijn Wargers (dead)]

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/

Comment 44

[Martijn Wargers (dead)]

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.

Comment 45

[Martijn Wargers (dead)]

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

Comment 46

[Martijn Wargers (dead)]

Ok, Jenkins try runs all passed (21 times).

Comment 47

[Martijn Wargers (dead)]

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.

Comment 48

[Martijn Wargers (dead)]

Attachment: 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.

Comment 49

[Martijn Wargers (dead)]

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.

Comment 50

[Johan Lorenzo [: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.

Comment 51

[Johan Lorenzo [:jlorenzo]]

Merged in master https://github.com/mozilla-b2g/gaia/commit/3fdccaac6fb2de3f047204f9f749dabdb90efbba