Closed Bug 972600 Opened 7 years ago Closed 7 years ago

[B2G][Contacts] After merging duplicate contacts selecting edit contact button brings up "Add contact" screen

Categories

(Firefox OS Graveyard :: Gaia::Contacts, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 verified)

RESOLVED FIXED
1.4 S3 (14mar)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- verified

People

(Reporter: bzumwalt, Assigned: gtorodelvalle)

References

()

Details

(Keywords: regression, Whiteboard: burirun1.3-3)

Attachments

(2 files, 1 obsolete file)

Description:
After merging two contacts as duplicates (both contacts have same phone number and similar names) tapping "edit contact" button on the resulting merged contact screen brings up an "Add Contact" page.

Repro Steps:
Prerequisite: Have two contacts with the same phone number and similar names (e.g. "Mar" & "Maria") present in address book. 
1) Updated Buri to BuildID: 20140212004003
2) Open Contacts app
3) Select one of the two available contacts
4) Touch "Find duplicate contacts" button
5) Select "merge"
6) Press "edit contact" button

Actual:
Editing merged duplicate contact brings up "Add contact" screen.

Expected:
Edit contact screen appears when user presses edit contact button.

Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20140212004003
Gaia: ce17d5eae7b1893ae4397c814b10ae598fcbdb58
Gecko: ab07e61c2eb0
Version: 28.0
Firmware Version: V1.2-device.cfg

Notes:
Repro frequency: 3/3, 100%
Link to failed test case: https://moztrap.mozilla.org/manage/case/9836/
See attached: Youtube video clip - http://youtu.be/iUjcCnRwHJY
What happens on 1.2?
Keywords: qawanted
Tested on today's Buri 1.2 and this behaviour did not occur. What instead occured is that the user, upon merging the duplicate contacts and hitting the edit contact button, was brought to the appropriate edit contact screen with all of the contact info correctly saved. (So I am saying that it appears to work correctly on 1.2)

Environmental Variables:
Device: Buri 1.2 MOZ
BuildID: 20140213004001
Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
Gecko: 4fd893896d02
Version: 26.0
Firmware Version: V1.2-device.cfg
Keywords: qawanted
QA Contact: pfield
Okay - so this is a regression that leads to wrong UX.
blocking-b2g: --- → 1.3?
triage: 1.3+ regression
blocking-b2g: 1.3? → 1.3+
I am sorry but I am not able to reproduce this issue using today's v1.3 (Gecko-0b4f0a8.Gaia-a43904d) :S

Brogan, would you be so kind to retest it?, just in case the issue has been solved or I am doing something wrong regarding the STR. Thanks!
Flags: needinfo?(bzumwalt)
In fact, you'll notice using today's v1.3 build (Gecko-0b4f0a8.Gaia-a43904d) that the "Find duplicates contacts" does not respond to the clicking as it should :-O I am managing this issue with our QA team and I will fill a new bug if appropriate ;-)
Please, do not take comment 6 into consideration it seems it is related to my fingers :-O Not kidding! I am able to reproduce it but the QA team does not :-(
Assignee: nobody → gtorodelvalle
I'm not able to reproduce the bug on today's build as well.
I was able to reproduce this on the latest Buri 1.3 build.

Out of 10 tries using the steps provided in Comment 0, I saw the issue reproduce 5 times. It seems that the repro rate is now closer to 50%.

Environmental Variables:
Device: Buri
BuildID: 20140219004003
Gaia: a43904d9646109b48836d62f7aa51e499fbf4b2e
Gecko: 97922b6daad1
Version: 28.0
Base Image: V1.2-device.cfg

Re-adding regressionwindow-wanted tag.
Using same environmental variables as comment 9 on Buri 1.3 Mozilla Ril, I was able to get issue to reproduce 1 out of 3 tries.
Flags: needinfo?(bzumwalt)
I'm actually able to repro this issue on the earliest available 1.3 Buri build. I saw it on 3 out of 10 tries.

1.3 Environmental Variables:
Device: Buri 
BuildID: 20130919040201
Gaia: 0dedb5b3789afc638a0c7c67652937fcb30e77d2
Gecko: 803189f35921
Version: 27.0a1
Base Image: V1.2-device.cfg

This can also be reproduced on the latest 1.2, though the repro rate in that branch seems much lower. Out of 25 tries following the steps in Comment 0, I only saw the issue reproduce 6 times.

I checked with pfield, he had checked it 3 times on 1.2 and did not see the issue reproduce. Given that the repro rate initially seemed to be much higher it felt like a sufficient amount of tries at the time, which explains the differences in results.

1.2 Environmental Variables:
Device: Buri
BuildID: 20140213004001
Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
Gecko: 4fd893896d02
Version: 26.0
Base Image: V1.2-device.cfg
 
1.1 does not seem to have the merge duplicate contacts feature at all, this may have been introduced with that feature in 1.2.

Removing the regression tags for now.
(In reply to J Zimbrick from comment #11)
> I'm actually able to repro this issue on the earliest available 1.3 Buri
> build. I saw it on 3 out of 10 tries.
> 
> 1.3 Environmental Variables:
> Device: Buri 
> BuildID: 20130919040201
> Gaia: 0dedb5b3789afc638a0c7c67652937fcb30e77d2
> Gecko: 803189f35921
> Version: 27.0a1
> Base Image: V1.2-device.cfg
> 
> This can also be reproduced on the latest 1.2, though the repro rate in that
> branch seems much lower. Out of 25 tries following the steps in Comment 0, I
> only saw the issue reproduce 6 times.
> 
> I checked with pfield, he had checked it 3 times on 1.2 and did not see the
> issue reproduce. Given that the repro rate initially seemed to be much
> higher it felt like a sufficient amount of tries at the time, which explains
> the differences in results.
> 
> 1.2 Environmental Variables:
> Device: Buri
> BuildID: 20140213004001
> Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
> Gecko: 4fd893896d02
> Version: 26.0
> Base Image: V1.2-device.cfg
>  
> 1.1 does not seem to have the merge duplicate contacts feature at all, this
> may have been introduced with that feature in 1.2.
> 
> Removing the regression tags for now.

The analysis here isn't right. The fact that there was an increase of reproducibility from 6/25 to 100% implies a regression during the 1.3 timeframe. Please get the regression range that focuses on when the reproducibility went from being intermittent to being consistently reproducible.
Always scary when it seems we observe distinct things when "supposedly" doing the same :O

To try to narrow that "supposedly" down, would you you be so kind to share the concrete data you are using and which part are you repeating to get that reproducibility rate? I guess you just click on the "edit contact icon" and get the "Edit contact" ("Add contact" some times), close that page, click on the "edit contact icon" and see what you get, close that page, etc. Or do you create the second contact and merge the contacts again as part of the repetition?

Using today's v1.3 build (Gecko-7c32fd6.Gaia-a43904d) and following the next steps, this is what I get:
  1. Create a contact with name "Mar" and phone number "666666666".
  2. Create a contact with name "Maria" and phone number "666666666".
  3. The duplicate found page is shown.
  4. Close the "duplicate found" page clicking on the "X" icon.
  5. Click on "Mar" to get her contacts details.
  6. Click on "Find duplicate contacts".
  7. "Maria" is found as a duplicate contact.
  8. Click on "Merge with 1 contact".
  9. "Mar" contact details page is shown.
  10. Click on the "edit contact icon".
  11. The "Edit contact" page is shown.
  12. Close the "Edit contact" page.
  13. GOTO 10.

This is, I get the "Edit contact" page 100% of the cases. I guess we are doing something differently here.
Tested (02/20/2014) and working 
1.3
Gecko 7c32fd6
Gaia a4304d
Re: Comment 13

I believe we are following the same STR on our side with some exceptions:

On your step 13, we are deleting created contacts and going back to step 1. This also means that the contact list has no existing contacts at step 1. I have yet to reproduce this issue once it has occurred from your step 10 on the same contact.
Re-tested this issue on the 1.3 build listed in Comment 0 as repro rate seems to be of major concern regarding this bug. Was no longer able to get 100% repro rate experienced when bug was filed. Rate now seems to be in line with Comment 9, Comment 10, and Comment 11 using build 20140212004003.

Further investigation revealed that restarting the phone in between attempts may improve repro rate (observed 4/7; 57%)
Attached file Logcat
Attached logcat of issue occurring using environmental variables in Comment 0
What's the impact to the user here?

It doesn't seem easily reproducible so will need to unblock on 1.3 based on user impact.
Flags: needinfo?(bzumwalt)
(In reply to Preeti Raghunath(:Preeti) from comment #18)
> What's the impact to the user here?
> 
> It doesn't seem easily reproducible so will need to unblock on 1.3 based on
> user impact.

The impact is entirely wrong UX, which is likely to confuse someone significantly. The reproducibility is high enough that someone is likely to hit (57%), so I tend to disagree that this is safe to unblock.
Flags: needinfo?(bzumwalt)
Can we re-clarify the STR here & reproduction rate? I'm a bit lost in the confusion of the comments here.
I managed to reproduce it on my third attempt \o/

Let me try to clarify the STR here:

  0. Delete the contacts with name "Mar" and "Maria" and phone number "666666666" if they already exist.
  1. Create a contact with name "Mar" and phone number "666666666".
  2. Create a contact with name "Maria" and phone number "666666666".
  3. The duplicate found page is shown.
  4. Close the "duplicate found" page clicking on the "X" icon.
  5. Click on "Mar" to get her contacts details.
  6. Click on "Find duplicate contacts".
  7. "Maria" is found as a duplicate contact.
  8. Click on "Merge with 1 contact".
  9. "Mar" contact details page is shown.
  10. Click on the "edit contact icon".
  11. The "Edit contact" page is shown.
  12. Close the "Edit contact" page.
  13. GOTO 0.

As I said, in my case I reproduced it the third time I got to 11. In that case, I saw the "Add contact" page as the bug claims ;-)
First off, congrats to Brogan for detecting a pretty hidden run condition in the code ;-)

On the second hand and once I understood the problem, let me provide you with a new STR to increase the reproduction rate to 100% :-) :

--------------------------------------------

STR:
1. Create a contact with name "M" and phone number "666666666".
2. Create a contact with name "Ma" and phone number "666666666".
3. The duplicate found page is shown.
4. Close the "duplicate found" page clicking on the "X" icon.
5. Create a contact with name "Mar" and phone number "666666666".
6. The duplicate found page is shown.
7. Close the "duplicate found" page clicking on the "X" icon.
8. Create a contact with name "Mari" and phone number "666666666".
9. The duplicate found page is shown.
10. Close the "duplicate found" page clicking on the "X" icon.
11. Create a contact with name "Maria" and phone number "666666666".
12. The duplicate found page is shown.
13. Close the "duplicate found" page clicking on the "X" icon.
14. Click on "M" to get her contacts details. If fact any of the created contacts is fine.
15. Click on "Find duplicate contacts".
16. The other 4 contacts are found as duplicate ones.
17. Click on "Merge with 4 contacts".
18. "M" contact details page is shown. Or the clicked contact in 14.
19. Click on the "edit contact icon".

OBSERVED:
20. The "Add contact" page is shown.

EXPECTED:
20. The "Edit contact" page is shown.

--------------------------------------------

To increase the reproduction rate, just increase the number of additional created duplicate contacts before finding the duplicate contacts and merging.

From this on it is a note to myself and for further discussion with José Manuel and Francisco :-) :

The problem was the following. When merging the contact with the other 4 duplicate ones, the contactchange event was notified 5 times: 1 with event.reason 'update' and 4 with event.reason 'remove'. The problem is that the processing of the 'update' event is completed before the last of the 'remove' events is notified and consequently this line of code https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L747 is executed before this one https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L769. The final consequence is that |currentContact| is left as {} instead as the updated/merged contact and when clicking on the 'edit contact icon' there's no contact data to load into the form.

A possible quick solution to the problem could be to delete this line https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L769 I have tested it and I haven't found any regressions but it is a risky move since we may be missing some buggy scenarios :-(

I will discuss it with José Manuel and Francisco to try to find the best solution to the problem.
Fantastic analysis.

To be sure we have no problem removing that line, the detail view has a method to setup the contact it's displaying.

We can do that when we receive an update and we are in the detail view (like is the case).

Then when we call:
|Contacts.showForm(true);| to display the edit form, we could add an extra parameter to indicate what's the current contact:
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/views/details.js#L92
(There we can tell the Contact class what's the current contact)

Unfortunately this is cause the way we were using the Contacts class a a hub for navigating. Something we need to change with haida for sure.

Cheers.
Attached file 16619.html (obsolete) —
Thanks Francisco! ;-)

I finally took Francisco's comment as a source of inspiration as a way to include as least changes as possible to cover the buggy scenario. As a consequence, the line I was proposing to delete when the 'remove' event was notified is not deleted. In fact, the only changes are to pass the updated/merged contact to the contacts.Details object when the 'update' event is notified and the contacts details view is visible (as Francisco suggests). This contact is passed back to the Contacts main object when the form has to be visualised.
Attachment #8381482 - Flags: review?(francisco.jordano)
Comment on attachment 8381482 [details]
16619.html

Hi German, thanks for the code, taking into account the current landing policy, could you provide some unit tests before Rick realizes the PR doesn't contain them ;) ?

Thanks!
Attachment #8381482 - Flags: review?(francisco.jordano)
(I did notice it :D )
Comment on attachment 8381482 [details]
16619.html

Hi guys!!! After fighting against the Marionette JS Runner :p I managed to create a passing integration test for the case reported in this bug. In fact, I have just confirmed that it does not pass in current master and it does pass when my patch is applied so I am happy about it ;-)

Francisco, would you be so kind to re-review the patch? Thanks!
Attachment #8381482 - Flags: review?(francisco.jordano)
Target Milestone: --- → 1.4 S3 (14mar)
Comment on attachment 8381482 [details]
16619.html

Perfect \o/

Thanks German, I'm just rerunning one unit test in travis to have all things green!
Attachment #8381482 - Flags: review?(francisco.jordano) → review+
Flags: in-testsuite+
Landed:

https://github.com/mozilla-b2g/gaia/commit/db3b537b4a29be4849f45fb7ec27b77bb778a2a6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Asking for verify to land this in 1.3
Keywords: verifyme
Comment on attachment 8381482 [details]
16619.html

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Run condition when merging "many" (around 4-5) contacts. The deletion of contacts was deleting information needed to show the merged contact details.
[User impact] if declined: Bad user experience. User may feel lost since the shown "Add contact" page has no sense.
[Testing completed]: Integration test for merging 15 distinct duplicate contacts included in the patch.
[Risk to taking this patch] (and alternatives if risky): Low. Locally tested by the Telefónica QA team and a integration test covering the buggy case is included.
[String changes made]: None
Attachment #8381482 - Flags: approval-gaia-v1.3?(fabrice)
Sorry, I've had to revert this due to causing an intermittent test failure on travis.

The reverted commit: https://github.com/mozilla-b2g/gaia/commit/3e6b11d903d861fbc102d8d269cbf22e4a80c05c

The details of the travis failure:

1) Contacts > Details Merging 15 contacts:

Error: timeout exceeded!

at Object.Client.waitForSync (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:679:16)

at Object.Client.waitFor (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:648:60)

at Object.Contacts.waitSlideLeft (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:110:17)

at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/details_test.js:65:13)

at Test.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:211:32)

at Runner.runTest (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:372:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:448:12

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:297:14)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:307:7

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:245:23)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:213:5)

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:257:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:197:9

at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)

at process._tickCallback (node.js:415:13)
Status: RESOLVED → REOPENED
Flags: needinfo?(gtorodelvalle)
Resolution: FIXED → ---
This issue does not occur in the latest v1.4 build.

Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140304040200
Gaia: 6df4533f14ec2645bb13d8a803a5151583ca13a8
Gecko: 529b86b92b1d
Version: 30.0a1
Firmware: V1.2-device.cfg
Keywords: verifyme
Attached file 16910.html
Hi Kevin, thanks for the info ;-)
The point is that there is no clear cause of the intermittent failures to me :-( The code seems legit. Anyhow, I "improved" the already existent code in /marionette/lib/contacts.js in case that was the cause of the issue. Some examples of those improvements are extracting the finding of Marionette elements from the waitFor() invocations (please see https://github.com/mozilla-b2g/gaia/pull/16910/files#diff-93f450be0058b99f97a02c6657b29880R110) as well as using <= instead of the === operator (see https://github.com/mozilla-b2g/gaia/pull/16910/files#diff-93f450be0058b99f97a02c6657b29880R113).
On the other hand, I really do not know how to check if the intermittent failures persist since every time I run the integration tests locally in my laptop they pass. What do you suggest? Thanks!
Attachment #8381482 - Attachment is obsolete: true
Attachment #8381482 - Flags: approval-gaia-v1.3?(fabrice)
Attachment #8386212 - Flags: review?(kgrandon)
Attachment #8386212 - Flags: feedback?(jmcf)
Flags: needinfo?(gtorodelvalle)
Attachment #8386212 - Flags: feedback?(jmcf) → feedback?(francisco.jordano)
Hey - have you tried running them on travis because it's a slower machine? 

I would recommend running it 100 times when it's slow to see if there's any intermittent failures after your patch. You can use something like this in a pull request: https://gist.github.com/KevinGrandon/8047042

Also you may want to add a bit of logging during the waitFor() changes to ensure that we are waiting properly for all transitions.
Comment on attachment 8386212 [details]
16910.html

If you can show me 50+ green travis runs in a job, I will gladly R+ this. Thanks!
Attachment #8386212 - Flags: review?(kgrandon)
Hei,

just an idea, why don't you enable travis in your own repo, and we run this let's say not 50 times but 10?
Kevin will you like that solution? So we could try in the same kind of environment.

This is a 1.3+ bug, and despite that I agree with Kevin about don't landing buggy tests, we need the proper fix :_)

Thanks folks!
Flags: needinfo?(kgrandon)
Comment on attachment 8386212 [details]
16910.html

LGTM, please follow Kevin's advice, let's run this several times in your repo and let's check if we get several greens in a row :)
Attachment #8386212 - Flags: feedback?(francisco.jordano) → feedback+
I'm fine with that - TBH 10 times is not enough. If we have a 5% failure rate, it's likely that it won't show up in that run. Ultimately I leave it up to the reviewier - but if the test has an intermittent failure anytime within the next week it will probably be backed out - oranges really hurt us so we need to do everything we can to avoid them. (Much less ideal, but if it's necessary for 1.3 I suppose we could spin the tests off into a follow-up bug and land the original fix still).
Flags: needinfo?(kgrandon)
@German can we make it 50?

Not a hard request, but Kevin will be happier here (and me too to be honest). Now that you can run it against your fork (and don't compete with other instances), we can give it a try isnt?

Thanks!
Hi again! I made it 20 :-) https://travis-ci.org/gtorodelvalle/gaia/builds/20276340 As you can see there are 3 failing runs but the failing tests are not related to any of the updates included in this patch :-)

For some strange reason I do not see the result of Travis in the PR (attachment 8386212 [details]) :-( I guess it is a matter of time :-)
Ups, forgot the ni :-(
Uhmmm... I have an issue with the nis... :-) (yeah, I am using Chrome on this laptop and these are the consequences... :p)
Flags: needinfo?(kgrandon)
I just launched 50 integration test runs as requested by José Manuel... :-) I'll come back to you with the results ;-) Thanks!
Not sure if there's anything relevant for me here, but please let me know if there is - and how the test run goes. Thanks!
Flags: needinfo?(kgrandon)
Hi guys! And finally we have a winner :-) As you can see at https://travis-ci.org/gtorodelvalle/gaia/builds/20514078 , I ran 50 integration test runs in a row and although there are many which are failing, none of them is related to the updates included in this patch. Anyhow, it would be great if you could check it too, please.

Ready to merge attachment 8386212 [details], José Manuel?
Flags: needinfo?(jmcf)
Glad your tests passed! That is way too much red for typical use though, so I'm going to go through and disable/back out some of the newer tests. Thanks for handling this!
Attachment #8386212 - Flags: review?(francisco.jordano)
Comment on attachment 8386212 [details]
16910.html

Nice, 50 executions and working well ;)
Attachment #8386212 - Flags: review?(francisco.jordano) → review+
Thanks Francisco ;-)

Merged in master: https://github.com/mozilla-b2g/gaia/commit/0b7491cc15bb1a8b6de425d2cc23e7d5b474593d
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Comment on attachment 8386212 [details]
16910.html

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Run condition when merging "many" (around 4-5) contacts. The deletion of contacts was deleting information needed to show the merged contact details.
[User impact] if declined: Bad user experience. User may feel lost since the shown "Add contact" page has no sense.
[Testing completed]: Integration test for merging 15 distinct duplicate contacts included in the patch.
[Risk to taking this patch] (and alternatives if risky): Low. Locally tested by the Telefónica QA team and a integration test covering the buggy case is included.
[String changes made]: None
Attachment #8386212 - Flags: approval-gaia-v1.3?(fabrice)
Attachment #8386212 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
v1.3: bfc6b263d296bd32422164d8f42097a4dc52c39c
*cry* 

 1) Contacts > Details Merging 15 contacts:

Error: timeout exceeded!

at Object.Client.waitForSync (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:679:16)

at Object.Client.waitFor (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:648:60)

at Object.MarionetteHelper.waitForElement (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-helper/index.js:131:12)

at Object.Contacts.waitForFormShown (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:130:35)

at Object.Contacts.enterContactDetails (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:159:10)

at Object.Contacts.addContact (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:180:10)

at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/details_test.js:43:15)

at Test.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:211:32)

at Runner.runTest (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:372:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:448:12

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:297:14)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:307:7

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:245:23)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:213:5)

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:257:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:269:7

at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:197:9

at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)

at process._tickCallback (node.js:415:13)
Flags: needinfo?(jmcf)
Geez!!! :-) I do not know if these intermittent failures may have something to do with this: https://bugzilla.mozilla.org/show_bug.cgi?id=944322 Not probably the best solution but do you want me to increase the element searching timeout? Anyhow, right now it is set to 10000 and consequently I wouldn't bet this is the cause of the issue.

According to Kevin's logs, the issue is related to the lazy loading templates (more concretely, https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/index.html#L178) which Marionette seems not to be a fan of :-) What I will do next is instead of waiting for the concrete lazy loading section (##view-contact-form) to be included in the DOM and displayed (since it already is but its contents are not) I will include a condition to wait for the section's header title to check when the lazy loaded section contents are really available :s

I'll come back to you with the results ;-)

The issue does not seem to have anything to do with Kevin's patch to solve the JSHint issues on the updated files: https://github.com/mozilla-b2g/gaia/commit/488670bc36e3cff5b2ebc749d48f9dc26897914e :-p
Depends on: 982260
This is perma-red on 1.3 in travis:

https://travis-ci.org/mozilla-b2g/gaia/jobs/20718997

 1) Contacts > Details Merging 15 contacts:

TypeError: Cannot read property '1' of null

 at Object.Contacts.l10n (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:107:33)

at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/details_test.js:85:34)
Should we also back this out of master? It's currently intermittent there as well.
Hi guys! There is something weird going on here :-S

Ryan, would you be so kind of pasting the link to the commit which uplifted the patch to v1.3 according to your last comment (sorry but it does not have a link to refer to it).

The weird thing is that the error Michael mentions in comment 54 has no sense if we had uplifted all dependant patches. See https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/test/marionette/lib/contacts.js#L100 in master versus https://github.com/mozilla-b2g/gaia/blob/v1.3/apps/communications/contacts/test/marionette/lib/contacts.js#L103 in v1.3. That line should have been updated by some commit.

I will uplift the proposed patch to v1.3 locally and run the tests to check which may have been the issue.

Thanks!
First dependency found... ;-)
Depends on: 982084
bug 957497 is the one which updates the l10n function in contacts.js (see https://github.com/mozilla-b2g/gaia/blob/v1.3/apps/communications/contacts/test/marionette/lib/contacts.js#L90) So definitely another dependency.

The point is that this bug's patch includes many other changes which are not needed, so I suggest to prepare a specific PR to v1.3 including all the updates needed to solve the issue this bug reports and of course passing all the tests. What do you think?
(In reply to gtorodelvalle from comment #59)
> bug 957497 is the one which updates the l10n function in contacts.js (see
> https://github.com/mozilla-b2g/gaia/blob/v1.3/apps/communications/contacts/
> test/marionette/lib/contacts.js#L90) So definitely another dependency.
> 
> The point is that this bug's patch includes many other changes which are not
> needed, so I suggest to prepare a specific PR to v1.3 including all the
> updates needed to solve the issue this bug reports and of course passing all
> the tests. What do you think?

Yes, I agree with Germán. We need to be very pragmatic here and to opt in for a solution that is a quick win. This bug is blocking and has been there for ages. It is time to fix it and move forward
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #51)
> v1.3: bfc6b263d296bd32422164d8f42097a4dc52c39c

(In reply to gtorodelvalle from comment #57)
> Ryan, would you be so kind of pasting the link to the commit which uplifted
> the patch to v1.3 according to your last comment (sorry but it does not have
> a link to refer to it).

https://github.com/mozilla-b2g/gaia/commit/bfc6b263d296bd32422164d8f42097a4dc52c39c
Ups Ryan, missed that. Thanks! ;)

I am currently running the tests in my laptop including the patch I have prepared for v1.3 and waiting for Travis to be up an running :-)
This is the PR I prepared for v1.3: https://github.com/mozilla-b2g/gaia/pull/17197

The tests are passing in my laptop but we'll have to wait for Travis' final verdict.
v1.3: 17597e6280b149edbc528f59720acc24a5f5cdb5
Depends on: 983777
You need to log in before you can comment on or make changes to this bug.