If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED

Status

Firefox OS
Gaia::UI Tests
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Martijn Wargers (dead))

Tracking

unspecified
x86
Mac OS X
Dependency tree / graph
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments, 9 obsolete attachments)

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 | Splinter Review
(Assignee)

Description

3 years ago
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

3 years ago
Component: Gaia::UI Tests → Gaia::Contacts
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

3 years ago
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
(Assignee)

Comment 4

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

3 years ago
Created attachment 8512840 [details] [diff] [review]
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.
(Assignee)

Comment 7

3 years ago
Created attachment 8514247 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25651
Attachment #8514247 - Flags: review?(florin.strugariu)
(Assignee)

Comment 8

3 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

3 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

3 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

3 years ago
Created attachment 8514454 [details] [review]
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).
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-
(Assignee)

Comment 14

3 years ago
Created attachment 8516047 [details]
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)
(Assignee)

Updated

3 years ago
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8,
Flags: in-qa-testsuite?(martijn.martijn)
(Assignee)

Comment 15

3 years ago
Created attachment 8516234 [details] [review]
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.
Attachment #8514454 - Attachment is obsolete: true
Attachment #8514454 - Flags: review?(viorela.ioia)
Attachment #8516234 - Flags: review?(florin.strugariu)
(Assignee)

Comment 16

3 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

3 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

3 years ago
Created attachment 8516719 [details] [review]
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.
Attachment #8516234 - Attachment is obsolete: true
Attachment #8516234 - Flags: review?(florin.strugariu)
Attachment #8516719 - Flags: review?(florin.strugariu)
(Assignee)

Comment 19

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

Updated

3 years ago
Blocks: 1086687
(Assignee)

Comment 20

3 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.
Attachment #8516719 - Flags: review?(florin.strugariu) → review-
(Assignee)

Comment 21

3 years ago
Created attachment 8517764 [details] [review]
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.
Attachment #8516719 - Attachment is obsolete: true
Attachment #8517764 - Flags: review?(florin.strugariu)
Attachment #8517764 - Flags: review?(florin.strugariu) → review-
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.
(Assignee)

Comment 23

3 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

3 years ago
QA Whiteboard: fxosqa-auto-s3, fxosqa-auto-points=8, → fxosqa-auto-dropped-s3, fxosqa-auto-points=8, fxosqa-auto-backlog+
(Assignee)

Updated

3 years ago
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+
(Assignee)

Updated

3 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

3 years ago
Created attachment 8528354 [details] [review]
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.
Attachment #8517764 - Attachment is obsolete: true
Attachment #8518185 - Attachment is obsolete: true
(Assignee)

Comment 25

3 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

3 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 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

3 years ago
Created attachment 8530951 [details]
(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.
(Assignee)

Comment 29

3 years ago
Created attachment 8530967 [details]
(PNG Image, 320 × 569 pixels).png

Now I get this dialog again.
(Assignee)

Comment 30

3 years ago
Created attachment 8530977 [details] [review]
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?
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)
(Assignee)

Comment 32

3 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 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

3 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 on attachment 8530977 [details] [review]
facebook_import6

Looks good to me!
Flags: needinfo?(jlorenzo)
Attachment #8530977 - Flags: review?(jlorenzo) → review+
(Assignee)

Comment 36

3 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

3 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

3 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

3 years ago
Created attachment 8539383 [details] [diff] [review]
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).
Attachment #8530977 - Attachment is obsolete: true
Attachment #8539383 - Flags: review?(gmealer)
(Assignee)

Comment 41

3 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

3 years ago
Attachment #8539383 - Flags: review?(jlorenzo)
(Assignee)

Comment 43

3 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

3 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

3 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

3 years ago
Ok, Jenkins try runs all passed (21 times).
(Assignee)

Comment 47

3 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

3 years ago
Created attachment 8541885 [details] [review]
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)
(Assignee)

Comment 49

3 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 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
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 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

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