1036847 - Add a Install/Cancel/Download test to the dom:app test suite

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Antonio Manuel Amaya Calvo (:amac)]

Bug 1036143 detected a hole on the test suite for DOM:Apps. Currently we don't have tests for cancelling/retrying the download. This bug is just to add that test.

Comment 1

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V1. Test the install/cancel/download cycle.

Initial version. The test is completed, not asking for review because I want to add a downloadapplied test also.

Comment 2

[Antonio Manuel Amaya Calvo (:amac)]

Try run (including the patch from bug 903291, and the patch I proposed to that bug also!) on  https://tbpl.mozilla.org/?tree=Try&rev=5c29dc8da9d6

Comment 3

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V2. With an apply test

Comment 4

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V3. Without settimeout on the "client" part

Sorry for the spam. It's failing on TBPL and I think it might be because of the timer. I've removed the timer on the test_packaged*html, but can't remove it on the .sjs (and in any case it doesn't really matter if it's slower). I have one problem, though, and is that if I set a timer higher than 3 seconds it never fires.

Comment 5

[Antonio Manuel Amaya Calvo (:amac)]

Comment on attachment 8453776 [details] [diff] [review]
V3. Without settimeout on the "client" part

And again... /facepalm. I don't know how I did it but I lost the changes of a file on the patch. Removing the review request till this *$·"! thing passes consistently on the TBPL.

Comment 6

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V4. Use a trick so the 'server' timeout doesn't matter as much

Try run for this is at https://tbpl.mozilla.org/?tree=Try&rev=30da92be1a30

Comment 7

[Antonio Manuel Amaya Calvo (:amac)]

Comment on attachment 8453880 [details] [diff] [review]
V4. Use a trick so the 'server' timeout doesn't matter as much

Try run is green finally, requesting review.

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 8453880 [details] [diff] [review]
V4. Use a trick so the 'server' timeout doesn't matter as much

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

lgtm, but explain me your choice for alreadyDeferred.

::: dom/apps/tests/file_packaged_app.sjs
@@ +26,5 @@
>    var devUrl = ("devUrl" in query) ? query.devUrl : gDevUrl;
> +  // allowCancel just means deliver the file slowly so we have time to cancel it
> +  var allowCancel = "allowCancel" in query;
> +  var getPackage = "getPackage" in query;
> +  var alreadyDeferred = +getState("alreadyDeferred");

why not use a boolean for alreadyDeferred?

@@ +32,5 @@
> +  if (allowCancel && getPackage && !alreadyDeferred) {
> +    // Only do this for the actual package delivery.
> +    response.processAsync();
> +    // And to avoid timer problems, only do this once.
> +    setState("alreadyDeferred", "1");

s/"1"/true ?

Comment 9

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V5, with review comments addressed

r=fabrice

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28664fb86b3c

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28664fb86b3c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Backed out for frequent test_packaged_app_install.html timeouts after this landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/66651fb7bca3

https://tbpl.mozilla.org/php/getParsedLog.php?id=43727836&tree=Mozilla-Inbound

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/66651fb7bca3

Comment 14

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: Refactored the delay and wrote some data before waiting

This is basically the same patch that was r+ed before. I don't know why it was failing randomly on try, and I don't really think it's a problem of the test. In any case, I've modified a little the delayed download, to send some data before waiting (in case there's something closing the connection if no data has been seen) and reduced the delay.

The tries I've send are all green... but they were before also :)

Comment 15

[[:fabrice] Fabrice Desré]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #14)
> Created attachment 8456851 [details] [diff] [review]
> Refactored the delay and wrote some data before waiting
> 
> This is basically the same patch that was r+ed before. I don't know why it
> was failing randomly on try, and I don't really think it's a problem of the
> test. In any case, I've modified a little the delayed download, to send some
> data before waiting (in case there's something closing the connection if no
> data has been seen) and reduced the delay.
> 
> The tries I've send are all green... but they were before also :)

Please post the try links, and retrigger the test_packaged_app_install.html test 20 times to make sure it's solid.

Comment 16

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: V3, adjusted timing

Relaunched the tests a bunch of times on several platforms:

https://tbpl.mozilla.org/?tree=Try&rev=ef444b87bdba

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/358263de7795

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/358263de7795