1035941 - Add test for loading dialer

Status: RESOLVED FIXED

(Testing Graveyard :: Eideticker, defect, P2)

Description

[William Lachance (:wlach)]

We currently have a bunch of eideticker loading tests, but none for the dialer. We should have one.

Comment 1

[William Lachance (:wlach)]

This might make a good first bug for Eideticker, since it's fairly simple and self-contained, and doesn't actually require a camera setup. Process:

* Checkout and setup eideticker per https://github.com/mozilla/eideticker#installation
* Edit src/tests/b2g/launching/manifest.ini and add a section for [dialer.py], modelled on the existing tests
* Add a dialer.py file to the same directory, model it on browser.py. Edit the wait_for_content_ready method to use the logic here to wait for things to be displayed: https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/phone/regions/keypad.py#L28 
* Test your changes by running runtest.py --no-sync-time --no-capture b2g-dialer-startup

I may be missing a step or two, feel free to ask either davehunt (when he's back) or myself if you have questions. :)

Comment 2

[Gautam Akiwate [:gakiwate]]

Attachment: Bug-1035941-Add-test-for-loading-dialer.patch

Comment 3

[William Lachance (:wlach)]

Comment on attachment 8458215 [details] [diff] [review]
Bug-1035941-Add-test-for-loading-dialer.patch

Review of attachment 8458215 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 4

[William Lachance (:wlach)]

https://github.com/mozilla/eideticker/commit/c6231a0978addd6a7f23c55816ab68d93f47f959

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8458215 [details] [diff] [review]
Bug-1035941-Add-test-for-loading-dialer.patch

Review of attachment 8458215 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the post review/merge comments. I'm sure the code works as-is, but we might want to do some tidy up.

::: src/tests/b2g/launching/dialer.py
@@ +11,5 @@
> +class Test(B2GAppStartupTest):
> +
> +    def wait_for_content_ready(self):
> +        Wait(self.device.marionette).until(lambda m: self.device.gaiaApps.displayed_app.name == 'Phone')
> +        self.device.gaiaApps.switch_to_displayed_app()

These shouldn't be necessary, as this is done before we call wait_for_content_ready. See https://github.com/mozilla/eideticker/blob/c6231a0978addd6a7f23c55816ab68d93f47f959/src/eideticker/eideticker/test.py#L595

@@ +13,5 @@
> +    def wait_for_content_ready(self):
> +        Wait(self.device.marionette).until(lambda m: self.device.gaiaApps.displayed_app.name == 'Phone')
> +        self.device.gaiaApps.switch_to_displayed_app()
> +        keypad_toolbar_button = self.device.marionette.find_element(*Phone._keypad_toolbar_button_locator)
> +        Wait(self.device.marionette).until(lambda m: 'toolbar-option-selected' in keypad_toolbar_button.get_attribute('class'))

This feels a little brittle to me. As the dialer app does not load content after launching, I think it would be safe to have a no-op for wait_for_content.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

After speaking to Will, let's reopen this and follow-up with another patch based on comment 5. Would you mind taking this Gautam?

Comment 7

[Gautam Akiwate [:gakiwate]]

(In reply to Dave Hunt (:davehunt) from comment #6)
> After speaking to Will, let's reopen this and follow-up with another patch
> based on comment 5. Would you mind taking this Gautam?

Sure. So tentatively this is what we want?
    def wait_for_content_ready(self):
        pass

Comment 8

[Gautam Akiwate [:gakiwate]]

Attachment: Bug-1035941-Add-test-for-loading-dialer-no-op.patch

Comment 9

[William Lachance (:wlach)]

Comment on attachment 8460323 [details] [diff] [review]
Bug-1035941-Add-test-for-loading-dialer-no-op.patch

Review of attachment 8460323 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this is right. Could you put a quick comment in the method about this though?

Comment 10

[Gautam Akiwate [:gakiwate]]

Attachment: Bug-1035941-Add-test-for-loading-dialer-no-op.patch

Added comments explaining why we modified the test.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8460979 [details] [diff] [review]
Bug-1035941-Add-test-for-loading-dialer-no-op.patch

Review of attachment 8460979 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, thanks!

Comment 12

[William Lachance (:wlach)]

Pushed: https://github.com/mozilla/eideticker/commit/fc402f22c7a83997e1c74e3bfdca234d86d25f9b

Comment 13

[William Lachance (:wlach)]

I've enabled the test in the dashboard.

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to William Lachance (:wlach) from comment #13)
> I've enabled the test in the dashboard.

Checked into eideticker-ci:
https://github.com/mozilla/eideticker-ci/commit/1b71f45f15695201439e10035b6563e3373c4531