Closed
Bug 1150009
Opened 10 years ago
Closed 10 years ago
Add cleanup functions to the gaia UI tests that install webapps
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: martijn.martijn, Assigned: martijn.martijn)
References
Details
Attachments
(2 files)
There are some Gaia UI tests, that install webapps. They need to have a cleanup method that uninstalls the app afterwards (when it's still there):
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/marketplace/test_marketplace_packaged_app.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_launch_app_packaged.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_delete_app_packaged.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_delete_app.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/homescreen/test_homescreen_launch_app.py
When that is done, the "self.device.file_manager.remove('/data/local/webapps')" line at: https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L819
..can be removed, because:
14:27 jlorenzo: mwargers: because in the build I have (supposedly an engineering one), all the apps are in this folder
14:28 jlorenzo: mwargers: so basically, at the first run, Marionette deletes everything and you're not able to run anything
14:30 mwargers: jlorenzo: I remember it vaguely, I had some issue where webapps stayed in between tests or something
14:30 mwargers: oh, yes, bug 1080487
14:30 firebot: https://bugzil.la/1080487 — FIXED, nobody@mozilla.org — Revert bug 990214, to re-introduce deletion of webapps during cleanup
14:30 mwargers: oh, yes, when running the install_packaged_app tests, for example
14:31 mwargers: jlorenzo: hmm, why are the apps in that folder?
14:32 jlorenzo: mwargers: I asked a couple of guys in the Paris office, it supposed to be the default folder for eng build. But I don't know why we don't have this issue on the eng build from pvt builds
14:56 jlorenzo: mwargers: mwargers: our eng build from pvt are not pure eng build. This is the config file we use https://dxr.mozilla.org/build:mozharness/source/configs/b2g/releng-otoro-eng.py
15:00 jlorenzo: mwargers: it says it's an engineering, but they set "B2G_SYSTEM_APPS": "1" which changes the location of the webapps to the same as user builds
So a pure engineering build currently barfs while running the gaia UI tests.
B2G_SYSTEM_APPS moves apps from /data to /system, as I understand it. I think it also prevents the app code from being combined into the *defer*js files that concatenate all the app JS together.
I'm sure what it amounts to is A) making the apps easier to debug, and probably B) more changeable on-device at runtime.
So as mentioned, we need to have better handling of whether things are deleted from the device out of /data.
I think the origins for the apps (*.gaiamobile.org) might be sufficient to recognize a default app from a deployed one. Differentiating on origin would be the first thing I'd look at.
Alternately, very first setup could scan for installed apps, either via filesystem or app manager, and drop a dynamic whitelist on the device that we use during cleanup to decide what to delete.
Assignee | ||
Comment 2•10 years ago
|
||
19:16 geo: is origin a non-starter?
19:16 geo: or are the apps we install during tests also gaiamobile.org apps?
19:16 geo: there’s facility in GaiaApps to pull back all the app origins installed afaik
19:17 mwargers: geo, yeah, that would be a solution too, check for origin or so
19:17 geo: so if there’s a difference, we pull those back, filter on that origin, delete the others
19:17 geo: if that facility isn’t there I can add it
19:17 geo: it’s on the JS side, not sure what we did or didn’t expose on Python
19:18 geo: but I think there should be a get_installed_apps() or similar that gives back a list of dicts, including origins
19:19 geo: apps_to_delete = [app for app in installed_apps where not app[‘origin’].endswith(‘gaiamobile.org’)] or similar
19:20 geo: er, s/where/if
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
So something like this? This will make sure that only the apps installed by the tests will get removed.
Attachment #8593461 -
Flags: review?(dave.hunt)
Comment 5•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
See pull request for comments.
Attachment #8593461 -
Flags: review?(dave.hunt) → review-
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
From Dave Hunt on github:
"
I would prefer to explicitly uninstall any apps in the individual tests tear down, but I understand that if there's a failure in the set up or earlier in the tear down then the apps will not be uninstalled.
"
I've done that now with this pull request.
I tested it on b2g desktop and on device locally.
I guess we still want the clean up of the none bundled apps (if they were not cleaned up by the test themselves) in /data/local/webapps, right?
Attachment #8593461 -
Flags: review- → review?(dave.hunt)
Assignee | ||
Comment 7•10 years ago
|
||
If we want to clean up the none bundled apps on tear down (outside of the tests), then it's good to know what currently the apps are in the /data/local/webapps/ directory:
- "{14750155-5f3d-41ab-a527-f16adf4c3293}", "name: "Mochitest", "origin": "http://mochi.test:8888"
- "marketplace.firefox.com", "name": "Marketplace", "origin": "app://marketplace.firefox.com"
- "marketplace-dev.allizom.org", "name": "Dev" ,"origin": "app://marketplace-dev.allizom.org"
- "marketplace.allizom.org", "name": "Stage", "origin": "app://marketplace.allizom.org"
It's all mentioned in the /data/local/webapps/webapps.json file.
The bundled apps all have "basePath": "/system/b2g/webapps" (so you don't see them in the /data/local/webapps/ directory anyway) and with origin *.gaiamobile.org, it seems.
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
Something like this perhaps?
Attachment #8600443 -
Flags: review?(dave.hunt)
Comment 10•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
Looks good, but let's reduce the duplication by adding an uninstall method to GaiaApps.
Attachment #8593461 -
Flags: review?(dave.hunt) → review-
Comment 11•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
See comments in pull request.
Attachment #8600443 -
Flags: review?(dave.hunt) → review-
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
Updated pull request with your suggestion.
Attachment #8593461 -
Flags: review- → review?(dave.hunt)
Comment 13•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
Nice, thanks!
Attachment #8593461 -
Flags: review?(dave.hunt) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8593461 -
Flags: review?(jlorenzo)
Comment 14•10 years ago
|
||
Comment on attachment 8593461 [details] [review]
[gaia] mwargers:teardown > mozilla-b2g:master
r+ with one remaining question in the PR.
Updated•10 years ago
|
Attachment #8593461 -
Flags: review?(jlorenzo) → review+
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
Something like this? I'm using the system app for the installTime check.
Not sure how I can prevent the nested for..in loops.
I want to make sure I also delete the directories that are not mentioned in the webapps.json file (in case webapps.json file is not updated correctly with the latest installed webapps, I don't know if it is real time updated).
Attachment #8600443 -
Flags: review- → review?(dave.hunt)
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #14)
> Comment on attachment 8593461 [details] [review]
> [gaia] mwargers:teardown > mozilla-b2g:master
>
> r+ with one remaining question in the PR.
I followed this suggestion and also did git fetch and git rebase origin/master for this branch, because Marionette was upated.
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #16)
> (In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #14)
> > Comment on attachment 8593461 [details] [review]
> > [gaia] mwargers:teardown > mozilla-b2g:master
> >
> > r+ with one remaining question in the PR.
There was also a small JSLint issue that I had to fix (some missing semi-colons), after fixing that, Treeherder Try was green, so merged it now: https://github.com/mozilla-b2g/gaia/commit/b6ef0ba70185d6a687485672862742fc9e3b482c
Comment 18•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
I think this is pretty close. I added a suggestion in the pull request.
Attachment #8600443 -
Flags: review?(dave.hunt) → review-
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
This is now actually more your patch, than mine.
I tested this by running test_marketplace_packaged_app.py and then test_homescreen_launch_app (twice) and the marketplace app and homescreen installed app both would be deleted correctly when the next test would start up.
Attachment #8600443 -
Flags: review- → review?(dave.hunt)
Updated•10 years ago
|
Attachment #8600443 -
Flags: review?(dave.hunt) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8600443 [details] [review]
[gaia] mwargers:teardown2 > mozilla-b2g:master
With this pull request checked in, people that would build their own builds and upload them to the Flame device would get to see apps on the homescreen, instead of having all the apps wiped, as Johan was telling me.
Attachment #8600443 -
Flags: review?(jlorenzo)
Updated•10 years ago
|
Attachment #8600443 -
Flags: review?(jlorenzo) → review+
Assignee | ||
Comment 21•10 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #20)
> Comment on attachment 8600443 [details] [review]
> [gaia] mwargers:teardown2 > mozilla-b2g:master
Merged: https://github.com/mozilla-b2g/gaia/commit/252c00ce153bf6d635ca56cde63fc5555090b3dd
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•