Write a test to install & launch a packaged app

RESOLVED FIXED

Status

defect
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jsmith, Assigned: martijn.martijn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Reporter

Description

5 years ago
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).
Reporter

Updated

5 years ago
Summary: Write a test to install & launching a packaged app → Write a test to install & launch a packaged app
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

5 years ago
This is a smoketest so P1
Priority: -- → P1

Comment 3

5 years ago
I will start working on this
Assignee: nobody → andrei.hutusoru

Comment 4

5 years ago
Attachment #8420862 - Flags: review?(zcampbell)
Attachment #8420862 - Flags: review?(florin.strugariu)
Attachment #8420862 - Flags: review?(bob.silverberg)
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?
Flags: needinfo?(krupa.mozbugs)
Flags: needinfo?(jsmith)

Comment 8

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

Comment 9

5 years ago
(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.
Flags: needinfo?(jsmith)
Reporter

Comment 10

5 years ago
(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.
Reporter

Comment 11

5 years ago
(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?
Flags: needinfo?(cvan)
(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.
Flags: needinfo?(cvan)
+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
Yes, please limit the amount of testing we do on prod to a basic minimum. Thanks!

Comment 15

5 years ago
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.
Attachment #8420862 - Flags: review?(zcampbell) → review-

Comment 16

5 years ago
I will begin to update this.

Updated

5 years ago
Depends on: 1011016
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.
Attachment #8420862 - Flags: review?(bob.silverberg)

Updated

5 years ago
Flags: needinfo?(krupa.mozbugs)
As far as I know Andrei is not working at this removing Andrei from the default assignee.
Assignee: andrei.hutusoru → nobody
Assignee

Updated

5 years ago
Assignee: nobody → martijn.martijn
Assignee

Comment 19

5 years ago
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.
Attachment #8504059 - Flags: review?(zcampbell)
Attachment #8504059 - Flags: feedback?(florin.strugariu)
Assignee

Comment 20

5 years ago
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.
Assignee

Comment 21

5 years ago
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 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
Attachment #8504059 - Flags: feedback?(florin.strugariu) → feedback-

Comment 24

5 years ago
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)
Attachment #8504059 - Flags: review?(zcampbell) → review-
Assignee

Comment 25

5 years ago
Posted file 989562_v3 (obsolete) —
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.
Attachment #8504059 - Attachment is obsolete: true
Attachment #8505533 - Flags: review?(zcampbell)
Attachment #8505533 - Flags: feedback?(florin.strugariu)
Assignee

Comment 26

5 years ago
(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 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.
Attachment #8505533 - Flags: feedback?(florin.strugariu) → feedback-
Assignee

Comment 28

5 years ago
(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?
Flags: needinfo?(dave.hunt)
Assignee

Comment 29

5 years ago
(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.
Assignee

Comment 30

5 years ago
Posted file 989562_v4 (obsolete) —
Attachment #8505533 - Attachment is obsolete: true
Attachment #8505533 - Flags: review?(zcampbell)
Attachment #8506318 - Flags: review?(florin.strugariu)
Assignee

Comment 31

5 years ago
Posted patch mozqa.diff (obsolete) — Splinter Review
If the pull request gets reviewed+, then this is what needs to be uploaded to mozqa.com
Attachment #8506327 - Flags: review?(hskupin)
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.
Attachment #8506327 - Flags: review?(hskupin)
(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.
(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.
Flags: needinfo?(dave.hunt)
I've just raised bug 1084306, which would remove the requirement of hosting the packaged app on mozqa.com.
Assignee

Comment 36

5 years ago
Comment on attachment 8506327 [details] [diff] [review]
mozqa.diff

Considering bug 1084306, this is not needed.
Attachment #8506327 - Attachment is obsolete: true
Assignee

Comment 37

5 years ago
(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.
Depends on: 1084306
Assignee

Comment 38

5 years ago
Posted patch 989562.diffSplinter Review
This hg diff will remove these files from mozqa.com as they won't be necessary anymore, if 989562_v4 gets checked in.
Comment on attachment 8506318 [details] [review]
989562_v4

looks OK
Attachment #8506318 - Flags: review?(florin.strugariu) → review+
Assignee

Updated

5 years ago
Attachment #8506318 - Flags: feedback?(zcampbell)

Comment 40

5 years ago
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!
Attachment #8506318 - Flags: feedback?(zcampbell) → feedback+
Assignee

Comment 41

5 years ago
(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 :(
Assignee

Comment 42

5 years ago
Posted file 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
Attachment #8506318 - Attachment is obsolete: true
Attachment #8509977 - Flags: review?(zcampbell)
Assignee

Comment 43

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

Comment 44

5 years ago
Squashed and merged it for you Martijn!
https://github.com/mozilla-b2g/gaia/commit/2d2118ddb9ca916eac90f31bf955f4dfaab127de
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Attachment #8509977 - Flags: review?(zcampbell)
Assignee

Updated

5 years ago
Attachment #8506874 - Flags: review?(hskupin)
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.
Attachment #8506874 - Flags: review?(hskupin) → review+
Then it should better be added to https://github.com/mozilla/qa-testcase-data which seems to be more mobile/webapp oriented.
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.
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?
Flags: needinfo?(martijn.martijn)
(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".
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
Flags: needinfo?(hskupin)
Patch for testcase-data has been backed out:
https://hg.mozilla.org/qa/testcase-data/rev/1587fe8ff04b
Flags: needinfo?(martijn.martijn)
Flags: needinfo?(hskupin)
Thank you for all your help.

Have a nice day! :)
Assignee

Comment 55

5 years ago
Sorry about that!
Assignee

Comment 56

5 years ago
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.
QA Whiteboard: [fxosqa-auto-from-s1] [fxosqa-auto-s2]
You need to log in before you can comment on or make changes to this bug.