924274 - [Contacts] Add testing to catch recent regressions in the future

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

There have been reports of regressions since bug 907907 landed. It's apparent that we did not have a proper testing suite or we would have caught these issues.

I am more than happy to write tests for these regressions if we have not done so - please let me know what regressions we've been seeing.

Comment 1

[Kevin Grandon :kgrandon]

Hi Jose, Francisco - Can you make note of any recent contacts regressions here? I want to make sure we cover every single one of these cases with either a unit or integration test.

Thanks!

Comment 2

[Kevin Grandon :kgrandon]

This was spawned from bug 923718. Please add any other bug numbers. Thanks!

Comment 3

[Jose Manuel Cantera]

918582 918390 922037 918451

It is easy to find, just go to the list of recent PRs done by me :)

Comment 4

[Jose Manuel Cantera]

one more for the collection in the regression festival caused by DOM Lazy Loading

Comment 5

[Kevin Grandon :kgrandon]

Attachment: Contacts Marionette Tests - First Set

Hi Gareth,

I'm also going to have contacts peers review this, but if you could take a quick look at these first few tests to make sure everything is in order it would be appreciated. Thanks!

Comment 6

[Gareth Aye [:gaye] (back from PTO)]

Np I'll take a look tonight

Comment 7

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 816791 [details]
Contacts Marionette Tests - First Set

Nice job! This is great and I left some nits on github.

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

Thanks a lot Kevin for this work.

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 816791 [details]
Contacts Marionette Tests - First Set

Hi Francisco, Jose - 

I have the first set of contacts tests passing constantly in travis. If you could take a look it would be greatly appreciated. Once these land I will begin working on the next set. These tests cover the following regressions:

918582: [B2G] [Buri] [Contacts] User sees no response when tapping contact phone number with no SIM in the device 
  • Covered by details_test > Shows error /w no sim

922037: Contacts email, phone or address type cannot be selected 
  • Covered by details_test > Shows error /w no sim

924903: [Contacts] Custom tag for mobile, email an address can be edited but it is not saved 
  • Covered By form_test -> Can create custom label

918390: [Contacts] No error when selecting a contact with no number for Messages app contact requests 
  • Covered By activities_test -> Error message selected contact has no number

Additionally there are a few other simple tests as well.

Comment 10

[Jose Manuel Cantera]

I left some substantial comments on GH

thanks!

Comment 11

[Kevin Grandon :kgrandon]

(In reply to Jose M. Cantera (on PTO until Oct 28th) from comment #10)
> I left some substantial comments on GH

Thanks, tried to address them. Review ping? :)

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 816791 [details]
Contacts Marionette Tests - First Set

Thanks a lot Kevin!

Comment 13

[Kevin Grandon :kgrandon]

First set has landed in master: https://github.com/mozilla-b2g/gaia/commit/ac5d06e882944ba3c8d54b5b1c981166b44b5c6b

Comment 14

[Kevin Grandon :kgrandon]

Attachment: Contacts Marionette Tests - Verify that merge contact screen is shown

Waiting for a marionette-apps patch to land before putting this one up for review.

Comment 15

[Anthony Ricaud (:rik)]

We're seeing a lot of intermittents so I had to back out commit fec050c7f463ec9645c69158027a5998b293ea99. BTW, this commit is not listed here.

https://github.com/mozilla-b2g/gaia/commit/fed6c8acb0bc312d57efed5323b999559a1e4e30

1) Contacts > Details Click phone number Shows error /w no sim:
ElementNotVisible: (11) Element is not visible
Remote Stack:
<none>
at Error.MarionetteError (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/error.js:67:13)
at Object.Client._handleCallback (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:472:19)
at /home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:506:21
at TcpSync.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:94:10)
at Object.send (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:453:36)
at Object.Client._sendCommand (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:499:19)
at Object.Element._sendCommand (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/element.js:49:21)
at Object.sendKeys (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/element.js:158:19)
at Object.Contacts.addContact (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/lib/contacts.js:119:10)
at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/apps/communications/contacts/test/marionette/details_test.js:20: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:355:10)
at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:401:12
at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:281:14)
at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:290:7
at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:234:23)
at Object._onImmediate (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:258:5)
at processImmediate [as _immediateCallback] (timers.js:330:15)

Comment 16

[Kevin Grandon :kgrandon]

Obsoleted old tests and will work on a new PR. I'm curious if this wasn't caused by some recent checkin? Seems odd that I never saw this message when doing the original tests and I restarted several times to verify that there were no intermittent failures.

Comment 17

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Combined tests and will track in this new PR.

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 826564 [details] [review]
Github pull request

Hi Gareth!

Wondering if you could again look at these tests. I've made some updates for the intermittent test fails, which you can find here: https://github.com/mozilla-b2g/gaia/pull/13335/files#diff-93f450be0058b99f97a02c6657b29880R103

I think that the root cause is that I wasn't waiting for the panel to complete it's transition before filling out the form. In most cases it was fine, but I'm unable to reproduce the failure with this change.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 826564 [details] [review]
Github pull request

Hi Francsico,

I made a few changes to the test as listed above to fix the intermittent tests. If you have a minute would you feel comfortable reviewing the changes? Thanks!

Comment 20

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 826564 [details] [review]
Github pull request

Everything still looks good! I had a few small comments about css selectors being abstracted, but nothing major. We've struggled with ci flakes that we can't reproduce locally in email as well. My best suggestion is just to have logging in places where flaking is happening to help debug.

Comment 21

[Kevin Grandon :kgrandon]

Most nits addressed, but will have some followups to do. I'm going to resolve this one so it doesn't get too confusing with the backouts. I will be opening another bug for work on gmail/facebook integration testing. Thanks all!

This set has landed in master: https://github.com/mozilla-b2g/gaia/commit/5814d4b8cf3e4eb4ebc097894fe25344749a8c2a