1232680 - Repair and reactivate call_log_test.js

Status: RESOLVED FIXED

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

Description

[Johan Lorenzo [:jlorenzo]]

Found while :justinpotts tried to implement bug 1219650. The code to inject mocked entries is outdated and doesn't work on mulet (but does on device).

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] JohanLorenzo:bug-1232680 > mozilla-b2g:master

Comment 2

[Johan Lorenzo [:jlorenzo]]

The logs are correctly injected in the current PR. I'm polishing it up.

Comment 3

[Johan Lorenzo [:jlorenzo]]

The PR is not really ready yet. I'm currently stuck on an error and I'd like to understand the DOM a bit better.

Issue: I don't manage to retrieve the element: <gaia-header id="edit-mode-header">
Potential explanation: This element is under: <form hidden="" id="edit-mode">. The thing I don't understand is I can graphically see the header being changed on Mulet. I also reproduced the issue manually with WebIDE. Even though hidden="" remains, the form is shown as visible in the form. Then I wonder if we shouldn't update the DOM accordingly to its state. What do you think, Gabriele?

If you want to try it out: You'll need first the PR from bug 1240699.

More details about the PR:
* Each region of the UI is split in views and accessors.
** The accessors expose the element of a regions, like labels and buttons. They make sure the element is ready to be used by its consumers
** The view uses these elements, interact with them and return results to the test. For example: The test asks to open the call log tab, it receives the corresponding view, ready to be used.

* As a consequence:
** The test doesn't deal with the marionette API anymore, but only with abstractions.
** Each of these abstractions represents a user's intent. That is to say, in the test, you'll see: The user enters the Edit Mode (instead of the user taps the edit button). How an action is done is an "implementation detail" which resides in the view.
** No wait or switch to frame should be done in the test, as the view should be "ready to be used".

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

We figured this out on IRC, clearing the NI.

Comment 5

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8709106 [details] [review]
[gaia] JohanLorenzo:bug-1232680 > mozilla-b2g:master

Like discussed on IRC, call_log.js had to be modified. <form id='edit-mode'> had the attribute "hidden" set which made the innerHtml hidden to Marionette.
After making "hidden" disappear, I introduced some visual regressions: if the edit mode was closed, the header and footer remained. In the code, I found 3 different ways to close edit mode:
1. By dismissing it (close action on the header)
2. After all the call log entries are deleted
3. After some, but not all, are deleted. 

I implemented extra tests for each of these 3 cases.

Manually, I tried some other scenarios. They didn't show any bugs:
* Go to Cards view and come back => OK. Edit mode remained (just like before)
* Close the app => OK. Edit mode was gone (just like before)
* Open rocketbar => OK. Edit mode remained

Gabriele, are you aware of some other ways to close it? 

On another note, I left a small workaround, until bug 1241813 lands. The PR is already big enough, and I'm afraid the changes in bug 1241813 will be as about the same size.

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8709106 [details] [review]
[gaia] JohanLorenzo:bug-1232680 > mozilla-b2g:master

Excellent, this covers most of the scenarios that I was fixing in bug 1239636. I've blocked on that because it's (almost) ready so we can realistically land them both today. You'll only need to rebase your patch on top of that one; the only lines that are going to be rejected are the following:

https://github.com/mozilla-b2g/gaia/pull/33901/files#diff-f70ff14b804b9dcffe477bb56b6a7e97R802
https://github.com/mozilla-b2g/gaia/pull/33901/files#diff-f70ff14b804b9dcffe477bb56b6a7e97R829

You can drop both of them since my patch did something very similar to ensure hideEditMode() is always called when exiting.

Comment 7

[Johan Lorenzo [:jlorenzo]]

Rebased the patch, based on comment 6.

Gij11 and 18 have been failing on the master branch [1]. Gij12 is currently failing in some other PR[2]. The reason of the failures are related to audio, which is not a part of the PR. 

I landed the patch in master at https://github.com/mozilla-b2g/gaia/commit/b4a6a62f3509e03b0cca402dae93d6455b8eaf87

[1] https://treeherder.mozilla.org/#/jobs?repo=gaia-master&fromchange=4c80dd0309b6
[2] https://treeherder.mozilla.org/#/jobs?repo=gaia&fromchange=59956f8b592e

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

reverted for causing Gu7 bustage:

TEST-UNEXPECTED-FAIL | null | [communications-dialer/test/unit/call_log_test.js] dialer/call_log Edit mode > Entering edit mode "before each" hook
TypeError: this.editMode is null 
https://treeherder.mozilla.org/#/jobs?repo=gaia-master&revision=b4a6a62f3509

master: https://github.com/mozilla-b2g/gaia/commit/502c60f9719729fe625f4314e8bfc5032693581b

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] JohanLorenzo:bug-1232680-2 > mozilla-b2g:master

Comment 10

[Johan Lorenzo [:jlorenzo]]

Thank you for the revert, Michael. My apologies, how ironic it is when you repair call_log_test.js in Gij but break call_log_test.js in GU.

One line was missing to instantiate the mock[1]. Apart from that, the patch remains the same.

Gu doesn't show any failure anymore[2]. Gij11 and Gij18 are still failing like said in comment 7. Gij12 didn't permafail, though. GBI7 failed once (container time out). Gij22 and Gij25 failed once too (intermittent failure in non-dialer tests).

Landed in master at: https://github.com/mozilla-b2g/gaia/commit/9f0655e4af240206d2ca7d19efaf2f04be5eb191

[1] https://github.com/mozilla-b2g/gaia/pull/33997/files#diff-cff418f2b1545155fd099c7ace3dd76eR88
[2] https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=5d58706d0702da18e10e843eb363881378f550d9