1032745 - Use local resources where possible when installing web apps

Status: RESOLVED FIXED

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

Description

[Zac C (:zac)]

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.

Comment 1

[Zac C (:zac)]

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

Comment 2

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

Stealing this, as I used it as my test case for bug 1034608.

Comment 3

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

I'll cover test_homescreen_launch_app too.

Comment 4

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

Attachment: 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.

Comment 5

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

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

Comment 6

[Zac C (:zac)]

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.

Comment 7

[Zac C (:zac)]

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.

Comment 8

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

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.

Comment 9

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

(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.

Comment 10

[Zac C (:zac)]

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.

Comment 11

[Zac C (:zac)]

btw Dave I'm not blocking on comment #10, just discussing it.

Comment 12

[Zac C (:zac)]

Bebe, can you have a look at the Try failure on behalf of Dave, to give him a helping hand?

Comment 13

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

(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.

Comment 14

[Zac C (:zac)]

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'.

Comment 15

[Zac C (:zac)]

Bebe alreayd reviewed this so clearing his ni

Comment 16

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

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.

Comment 17

[Zac C (:zac)]

Initial runs passed so I've triggered a few more to test for intermittents

Comment 18

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

Hmm.. I thought I cleared my needinfo, apparently not! :)

Comment 19

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

(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.

Comment 20

[Zac C (:zac)]

Dave, yes IMO it's fine, it just slipped my memory. Please go ahead and land it.

Comment 21

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

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

Comment 22

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

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.

Comment 23

[Zac C (:zac)]

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.

Comment 24

[Zac C (:zac)]

Merged:
https://github.com/mozilla-b2g/gaia/commit/e26cae049d81a4fb35da9964eda12921da0346bb