989562 - Write a test to install & launch a packaged app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect, P1)

Description

[Jason Smith [:jsmith]]

The current UI automated tests make mention of a packaged app, but they aren't actually testing the packaged app workflow. One of our smoketests is supposed to cover this.

Let's write an automated test to install & launch a packaged app from Marketplace (e.g. Calculator).

Comment 1

[[:fabrice] Fabrice Desré]

We had once a breakage where we launched the app but only opened the directory listing instead of the expected page. That would be ideal to check this, but I don't know if we can do that without a reftest.

Comment 2

[Zac C (:zac)]

This is a smoketest so P1

Comment 3

[[:AndreiH]]

I will start working on this

Comment 4

[[:AndreiH]]

Attachment: PR https://github.com/mozilla-b2g/gaia/pull/19147

Comment 5

[[:AndreiH]]

Adhoc job:

http://selenium.qa.mtv2.mozilla.com:8080/view/B2G%20Hamachi/job/b2g.hamachi.mozilla-central.ui.adhoc/395

Comment 6

[Florin Strugariu [:Bebe]]

Comment on attachment 8420862 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/19147

see comments

Comment 7

[Florin Strugariu [:Bebe]]

Are we sure that the "Calculator" app will always be there?
Will we affect the Marketplace statistics installing this app on a prod env with automated tests?

Comment 8

[Zac C (:zac)]

Personally I think this test should be kept in the marketplace-tests repo where this flow is tested thoroughly against v1.3

Comment 9

[Jason Smith [:jsmith]]

(In reply to Florin Strugariu [:Bebe] from comment #7)
> Are we sure that the "Calculator" app will always be there?
> Will we affect the Marketplace statistics installing this app on a prod env
> with automated tests?

Should be - that's within Mozilla's control.

(In reply to Zac C (:zac) from comment #8)
> Personally I think this test should be kept in the marketplace-tests repo
> where this flow is tested thoroughly against v1.3

That would not be useful - the goal here is to test the gaia/gecko integration of packaged apps primarily, which would require running the test on each applicable branch we are actively maintaining.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #9)
> (In reply to Florin Strugariu [:Bebe] from comment #7)
> > Are we sure that the "Calculator" app will always be there?
> > Will we affect the Marketplace statistics installing this app on a prod env
> > with automated tests?
> 
> Should be - that's within Mozilla's control.

But yeah, that would affect Marketplace statistics though. If we're worried about that, then we'll likely need to test this against marketplace dev instead.

Comment 11

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #9)
> > (In reply to Florin Strugariu [:Bebe] from comment #7)
> > > Are we sure that the "Calculator" app will always be there?
> > > Will we affect the Marketplace statistics installing this app on a prod env
> > > with automated tests?
> > 
> > Should be - that's within Mozilla's control.
> 
> But yeah, that would affect Marketplace statistics though. If we're worried
> about that, then we'll likely need to test this against marketplace dev
> instead.

Or use an installation path that avoids having the app count in marketplace statistics.

Chris - When does marketplace register that a certain app has been installed in marketplace statistics? Would it be at the point of clicking the install button?

Comment 12

[Rob Hudson [:robhudson]]

(In reply to Jason Smith [:jsmith] from comment #11)
> Or use an installation path that avoids having the app count in marketplace
> statistics.
> 
> Chris - When does marketplace register that a certain app has been installed
> in marketplace statistics? Would it be at the point of clicking the install
> button?

We track 2 events (3 including the fail state). We send a tracking event when the user clicks the install button and also when the device says the app was successfully installed. Only the latter is used for statistics of app installs.

Testing against Marketplace dev would be best, IMO. We already have some apps that are used by tests on dev. The app they use is Wikipedia. We sometimes refresh our dev database from prod and this app will still be there, along with many others I'm sure.

Comment 13

[Bob Silverberg [:bsilverberg]]

+1 for using marketplace-dev, and if we are (mostly) assured of Wikipedia being there, then let's use that. We will want to try to keep the Marketplace app object in the gaia repo in sync with the one in marketplace-tests-gaia, so what I suggest for this PR is to take the app object that currently exists in marketplace-tests-gaia and copy it into gaia. If it works there then we can use it as a base object in marketplace-tests-gaia and only add what we need for the marketplace tests to the one in marketplace-tests-gaia.

I hope that's clearer than it sounds. :P

Comment 14

[krupa raj[:krupa]]

Yes, please limit the amount of testing we do on prod to a basic minimum. Thanks!

Comment 15

[Zac C (:zac)]

Comment on attachment 8420862 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/19147

r- there are quite a few places we can neaten this up.

Comment 16

[[:AndreiH]]

I will begin to update this.

Comment 17

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8420862 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/19147

I am cancelling my review on this until the PR is updated. Please flag me for review again when there is a new version to be reviewed.

Comment 18

[Florin Strugariu [:Bebe]]

As far as I know Andrei is not working at this removing Andrei from the default assignee.

Comment 19

[Martijn Wargers (dead)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/25090

I tried to address the review comments as much as possible from https://github.com/mozilla-b2g/gaia/pull/19147/files#diff-3

One problem is that I couldn't use Marketplace-Dev (named app is "Dev"). 
For some reason, using the Marketplace-dev app, doesn't fill in the text in the search field and then the test locically times out.
I don't know why that happens. I would have thought that the Marketplace app and the Marketplace-dev app are almost identical.

Note this change in test_marketplace_launch.py:
 _loading_fragment_locator = (By.CSS_SELECTOR, 'div.loading-fragment')
+ _loading_fragment_locator = (By.ID, 'splash-overlay')
This is not really related to this bug, but afaik, div.loading-fragment doesn't exist anymore, so this has to be fixed.

Comment 20

[Martijn Wargers (dead)]

Oh, and I wished I could do the confirm_uninstall() stuff inside of the self.apps.uninstall(app_name) function, but I don't know how to do that.

Comment 21

[Martijn Wargers (dead)]

And I'm also not totally sure about the reliability of the test. I also had instances with the Marketplace app, that nothing was typed in the search_box (although I was still tinkering with the test itself, then).
I did a repeat with this pull request 20 times and I got 1 failures where this was happening:
The B2G process has restarted after crashing during  the tests so Marionette can't respond due to either a Gecko, Gaia or Marionette error. Above, the 5 most recent errors are listed. Check logcat for all errors if these errors are not the cause of the failure.
TEST-UNEXPECTED-ERROR | test_marketplace_packaged_app.py TestSearchMarketplaceAndInstallApp.test_search_and_install_app | TimeoutException: TimeoutException: Connection timed out


Traceback (most recent call last):
  File "/Users/mwargers/.virtualenvs/gaia-py/lib/python2.7/site-packages/marionette/marionette_test.py", line 264, in run
    testMethod()
  File "/Users/mwargers/B2G/gaia_mwargers/tests/python/gaia-ui-tests/gaiatest/tests/functional/marketplace/test_marketplace_packaged_app.py", line 28, in test_search_and_install_app
    results = marketplace.search(self.app_search)
  File "/Users/mwargers/B2G/gaia_mwargers/tests/python/gaia-ui-tests/gaiatest/apps/marketplace/app.py", line 38, in search
    search_box.send_keys(Keys.RETURN)
  File "/Users/mwargers/.virtualenvs/gaia-py/lib/python2.7/site-packages/marionette/marionette.py", line 114, in send_keys
    return self.marionette._send_message('sendKeysToElement', 'ok', id=self.id, value=typing)
  File "/Users/mwargers/.virtualenvs/gaia-py/lib/python2.7/site-packages/marionette/decorators.py", line 35, in _
    return func(*args, **kwargs)
  File "/Users/mwargers/.virtualenvs/gaia-py/lib/python2.7/site-packages/marionette/marionette.py", line 621, in _send_message
    "Connection timed out", status=errors.ErrorCodes.TIMEOUT)

Afaik, I'm not doing anything wrong here.

Comment 22

[Florin Strugariu [:Bebe]]

Comment on attachment 8420862 [details] [review]
PR https://github.com/mozilla-b2g/gaia/pull/19147

>https://github.com/mozilla-b2g/gaia/pull/19147

Comment 23

[Florin Strugariu [:Bebe]]

Comment on attachment 8504059 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25090

I think we should avoid using Marketplace in this test.

We might see issues with the test as Martijn found because the interaction with MK.

As we can emulate a web server in our tests we mught be able tu use a local packaged app.

You can find test apps at: https://github.com/mozilla/qa-testcase-data

Comment 24

[Zac C (:zac)]

Comment on attachment 8504059 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25090

A good base but I think we can simplify it a bit more (with simplicity comes reliability too)

Comment 25

[Martijn Wargers (dead)]

Attachment: 989562_v3

This patch doesn't use the marketplace at all.

I modified test_homescreen_launch_app.py and test_homescreen_delete_app.py to add a packaged app for it. This seems to work fine and stable (running it 10 times didn't generate any error)
Some things/issues:
    I need to upload some files to http://mozqa.com/data/webapps/packaged1/ , because on desktop, it's using that url
    I wanted to move install_app to http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/app.py , but I couldn't get that working. For some reason, on the 2nd app install, the confirm button would not be tapped upon.

Comment 26

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #25)
>     I need to upload some files to http://mozqa.com/data/webapps/packaged1/
> , because on desktop, it's using that url

This needs to be done in the http://hg.mozilla.org/qa/testcase-data/ repository.
Henrik Skupin has access to that repo.

Comment 27

[Florin Strugariu [:Bebe]]

Comment on attachment 8505533 [details] [review]
989562_v3

I think we need 2 separate test cases here

1. test install/delete packaged app
2. test install/delete hosted app

Having these 2 in the same test might create confusion on which app is using and what is failing.

Comment 28

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #25)
>     I need to upload some files to http://mozqa.com/data/webapps/packaged1/
> , because on desktop, it's using that url

This is incorrect, this situation happens when the device is using cell data, but no wifi:
https://github.com/mozilla-b2g/gaia/blame/master/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_launch_app.py#L30
See bug 1032745, comment 9.

Dave, is that case actually being used in automated tests, currently?

Comment 29

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #25)
>     I wanted to move install_app to
> http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/
> homescreen/app.py , but I couldn't get that working. For some reason, on the
> 2nd app install, the confirm button would not be tapped upon.

I think I know the cause of this now.
When I make this:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/confirm_install.py
similar to this:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/confirm_dialog.py
..then that issue is not happening.

Comment 30

[Martijn Wargers (dead)]

Attachment: 989562_v4

Comment 31

[Martijn Wargers (dead)]

Attachment: mozqa.diff

If the pull request gets reviewed+, then this is what needs to be uploaded to mozqa.com

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8506327 [details] [diff] [review]
mozqa.diff

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

Oh, that's another repository then as I thought about. So you should find a reviewer for the github repo.

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #31)
> If the pull request gets reviewed+, then this is what needs to be uploaded
> to mozqa.com

Hm, maybe I misunderstood. Martijn, can you please clarify if this patch is targeted for http://hg.mozilla.org/qa/testcase-data/file/default/webapps or another repository? Thanks.

Comment 34

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

(In reply to Martijn Wargers [:mwargers] (QA) from comment #28)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #25)
> >     I need to upload some files to http://mozqa.com/data/webapps/packaged1/
> > , because on desktop, it's using that url
> 
> This is incorrect, this situation happens when the device is using cell
> data, but no wifi:
> https://github.com/mozilla-b2g/gaia/blame/master/tests/python/gaia-ui-tests/
> gaiatest/tests/functional/homescreen/test_homescreen_launch_app.py#L30
> See bug 1032745, comment 9.
> 
> Dave, is that case actually being used in automated tests, currently?

I'm not 100% sure what the question is here. If it's whether we expect to hit the mozqa.com address from our tests running in CI then the answer is no. This should only be used when running against a device or emulator without WiFi, and it really only likely to be hit by people running the tests locally without a reliable WiFi connection.

Comment 35

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

I've just raised bug 1084306, which would remove the requirement of hosting the packaged app on mozqa.com.

Comment 36

[Martijn Wargers (dead)]

Comment on attachment 8506327 [details] [diff] [review]
mozqa.diff

Considering bug 1084306, this is not needed.

Comment 37

[Martijn Wargers (dead)]

(In reply to Henrik Skupin (:whimboo) from comment #33)
> Hm, maybe I misunderstood. Martijn, can you please clarify if this patch is
> targeted for http://hg.mozilla.org/qa/testcase-data/file/default/webapps or
> another repository? Thanks.

Yeah, but plans have just been changed. Considering bug 1084306, this is not needed anymore.

Comment 38

[Martijn Wargers (dead)]

Attachment: 989562.diff

This hg diff will remove these files from mozqa.com as they won't be necessary anymore, if 989562_v4 gets checked in.

Comment 39

[Florin Strugariu [:Bebe]]

Comment on attachment 8506318 [details] [review]
989562_v4

looks OK

Comment 40

[Zac C (:zac)]

Comment on attachment 8506318 [details] [review]
989562_v4

f+!
If it were myself I'd leave the if condition that checks whether the app needs to be installed or not. You tend not to assume anything on Gaia :)
but I won't block this landing on that.

If you just squash these commits all into one and address bebe's PEP8 then this is ready to merge!

Comment 41

[Martijn Wargers (dead)]

(In reply to Zac C (:zac) from comment #40)
> If you just squash these commits all into one and address bebe's PEP8 then
> this is ready to merge!

I did address bebe's PEP8.
Sorry, I wasn't able to get all the commits squashed into 1. I somehow managed to get it squashed into 3 commits: https://github.com/mozilla-b2g/gaia/pull/25408
But after that, my github tree went really awry again. I'm really at a loss at this point :(

Comment 42

[Martijn Wargers (dead)]

Attachment: 989562_v7

Ok, finally managed to get a simple 1 commit in the pull request.
I just moved all the changed files into a new, clean branch.

Normally, nothing should have changed, but since I was tinkering so much with this. It might be good to formally request again a review on this. Sorry about that.
But if treeherder result looks good, I think it should be ok: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=4dec190f0899

Comment 43

[Martijn Wargers (dead)]

Treeherder result is good and I tested the patch on device again and it looks also good.

Comment 44

[Zac C (:zac)]

Squashed and merged it for you Martijn!
https://github.com/mozilla-b2g/gaia/commit/2d2118ddb9ca916eac90f31bf955f4dfaab127de

Comment 45

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8506874 [details] [diff] [review]
989562.diff

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

If it's no longer needed we can indeed remove it. Will land it in a bit.

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Removal of testcases has been landed as:
https://hg.mozilla.org/qa/testcase-data/rev/60a3ba64038f

Comment 47

[Martijn Wargers (dead)]

Yes, it can be safely removed. It was used in 
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_launch_app.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_delete_app.py
But it's not there anymore.
It might still be useful as an example of web install, though.

Comment 48

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Then it should better be added to https://github.com/mozilla/qa-testcase-data which seems to be more mobile/webapp oriented.

Comment 49

[William Hsu [:whsu]]

Please upleft this patch to v2.0 since this patch cause two failed cases (GU) on v2.0 branch.
- Error: test_homescreen_launch_app.TestLaunchApp
- Error: test_homescreen_delete_app.TestDeleteApp
Thanks.

Comment 50

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Martijn, it looks like that we shouldn't have removed those testcases yet. What do you think, shall we bring them back or update other branches to use something else?

Comment 51

[William Hsu [:whsu]]

(In reply to William Hsu [:whsu] from comment #49)
> Please upleft this patch to v2.0 since this patch cause two failed cases
Sorry for the typo. I meant "uplift".

Comment 52

[Florin Strugariu [:Bebe]]

Martijn Wiliam Henrik We use this app in some older branches of gaia UI test (V2.1, V2.0, V1.4).

As we don't want to uplift everything from master to the older branches (V2.1 v2.0 etc.)

Can we back out https://hg.mozilla.org/qa/testcase-data/rev/60a3ba64038f

Comment 53

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Patch for testcase-data has been backed out:
https://hg.mozilla.org/qa/testcase-data/rev/1587fe8ff04b

Comment 54

[William Hsu [:whsu]]

Thank you for all your help.

Have a nice day! :)

Comment 55

[Martijn Wargers (dead)]

Sorry about that!

Comment 56

[Martijn Wargers (dead)]

I filed bug 1094151 in case there is still need for a "install packaged app from the Marketplace" gaia-ui test. If there isn't, then I'll just close bug 1094151.