Use local resources where possible when installing web apps

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: zcampbell, Assigned: davehunt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Using the framework and style established in bug 1027621, update test_homescreen_delete_app to install the test app from the local server.
The app will need to be put into the resources folder.

The test can then be enabled on CI/TBPL.
Assignee: nobody → viorela.ioia
(Reporter)

Comment 1

5 years ago
This can't be done because a manifest has to be served with the content type header of:
application/x-web-app-manifest+json

but the marionette server serves it with:
application/octet-stream
Depends on: 1034608
Stealing this, as I used it as my test case for bug 1034608.
Assignee: viorela.ioia → dave.hunt
Status: NEW → ASSIGNED
I'll cover test_homescreen_launch_app too.
Summary: Update test_homescreen_delete_app to use local content. → Use local resources where possible when installing web apps
Created attachment 8456075 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21750

Note that this depends on bug 1034608. The current patch for bug 1034608 is therefore included in this pull request. We should not merge as is, but wait for the dependency to be resolved. This shouldn't affect the review for this patch though.
Attachment #8456075 - Flags: review?(zcampbell)
The blocker has been fixed, and I've rebased the pull request. It looks like this may cause a failure on linux 64bit builds. Any idea why that might be? See https://tbpl.mozilla.org/?rev=7c35d8588bb0a69e9dd60f078f00565d021cbb03&tree=Gaia-Try
(Reporter)

Comment 6

5 years ago
A timing race perhaps. This is not a known bug, to me at least. Judging by the html source in output.html, the banner it's waiting for is in the DOM. Can't see from that debug info whether the app was actually installed correctly.
(Reporter)

Comment 7

5 years ago
Comment on attachment 8456075 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21750

r+ basically but I think we should make this just always use the local content.
Attachment #8456075 - Flags: review?(zcampbell) → review+
The Linux 64bit failure appears to be constant so I'd like to resolve this before we land. As Zac says it may be down to timing.
Flags: needinfo?(dave.hunt)
(In reply to Zac C (:zac) from comment #7)
> Comment on attachment 8456075 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21750
> 
> r+ basically but I think we should make this just always use the local
> content.

A user running this test with cell data but not wifi should be able to get a pass. That is why we distinguish tests that require wifi from those that just require an online connection.
Flags: needinfo?(dave.hunt)
Flags: needinfo?(dave.hunt)
(Reporter)

Comment 10

5 years ago
I just don't really think running from cell data as fallback is a priority scenario. It seemed like a sensible scenario in the early days but all it does is create separate code paths and confuse test results when test failures do occur. Generally people either do use cell data or don't at all.
(Reporter)

Comment 11

5 years ago
btw Dave I'm not blocking on comment #10, just discussing it.
(Reporter)

Comment 12

5 years ago
Bebe, can you have a look at the Try failure on behalf of Dave, to give him a helping hand?
Flags: needinfo?(florin.strugariu)
(In reply to Zac C (:zac) from comment #11)
> btw Dave I'm not blocking on comment #10, just discussing it.

I understand. I'd say it's outside the scope for this bug though. Let's open another bug if we want to further discuss changing this behaviour.
(Reporter)

Comment 14

4 years ago
Forgot about this.. Would like to get this merged in if we can so I've re-triggered the TBPL jobs just to see if the issue has 'gone away'.
(Reporter)

Comment 15

4 years ago
Bebe alreayd reviewed this so clearing his ni
Flags: needinfo?(florin.strugariu)
Clearing my needinfo pending Zac's findings. For what it's worth my next steps were going to be to try to replicate this on a local Ubuntu VM. If that passes, I suspect the next step would be to find out if we can get access to one of the build machines to see if we can replicate it there.
(Reporter)

Comment 17

4 years ago
Initial runs passed so I've triggered a few more to test for intermittents
Hmm.. I thought I cleared my needinfo, apparently not! :)
Flags: needinfo?(dave.hunt)
(In reply to Zac C (:zac) from comment #17)
> Initial runs passed so I've triggered a few more to test for intermittents

Zac: Is this still looking good? If so, we should get this landed. Let me know if I can help.
(Reporter)

Comment 20

4 years ago
Dave, yes IMO it's fine, it just slipped my memory. Please go ahead and land it.
I've rebased and updated due to the 'external' manifest tags introduced since the last patch. Try run is here: https://tbpl.mozilla.org/?rev=b9cb5f3c637832a9100dec9a38acbd000a155471&tree=Gaia-Try
Flags: needinfo?(dave.hunt)
Comment on attachment 8456075 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21750

Try was good, let's get a final review due to the manifest changes before landing this.
Attachment #8456075 - Flags: review+ → review?(zcampbell)
Flags: needinfo?(dave.hunt)
(Reporter)

Comment 23

4 years ago
Comment on attachment 8456075 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21750

r+! Sorry this took so long.
Attachment #8456075 - Flags: review?(zcampbell) → review+
(Reporter)

Comment 24

4 years ago
Merged:
https://github.com/mozilla-b2g/gaia/commit/e26cae049d81a4fb35da9964eda12921da0346bb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.