Closed Bug 914931 Opened 9 years ago Closed 9 years ago

Test packaged apps update

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: marco, Assigned: marco)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch test_packaged_apps_update (obsolete) — Splinter Review
This test is a simple test, quite similar to the hosted and cached app tests.
Attachment #802706 - Flags: review?(fabrice)
We already have some coming in bug 900553.
(In reply to Fabrice Desré [:fabrice] from comment #1)
> We already have some coming in bug 900553.

Yes, I've seen them, this one is a bit different because it actually loads the application in an iframe.
Sure, but that's not a good reason to have duplicate tests. Improve the existing ones if you wish, but duplication is not great.
Oh, yes, sure, I'll build on top of the test in bug 900553 when it will land! I just wanted to make sure that this test was actually useful, to avoid wasting time if it wasn't.

I've made this one a chrome mochitest because otherwise I couldn't load a app:// url in the iframe (because of the Ci.nsIProtocolHandler.URI_DANGEROUS_TO_LOAD flag, IIRC). Do you know of any workaround so that I can make it a plain mochitest?
This adds a small test to check that checkForUpdate doesn't find updates right after an application is installed and that mozIDOMApplication::download() doesn't download anything if no update is available.

The test that checks the package contents by loading the app in an iframe isn't yet present in this patch.
It needs to be in a chrome mochitest, do you want me to convert test_packaged_app_update.html to a chrome mochitest or do you prefer me to create a new test for that? (unless you know a workaround to allow the app:// protocol to be used in an iframe in a plain mochitest)
Attachment #802706 - Attachment is obsolete: true
Attachment #802706 - Flags: review?(fabrice)
Attachment #806976 - Flags: review?(fabrice)
Comment on attachment 806976 [details] [diff] [review]
Add simple test to test_packaged_app_update

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

lgtm, but please send to try before landing

::: dom/apps/tests/test_packaged_app_update.html
@@ +29,5 @@
>  var miniManifestURL;
>  
>  SimpleTest.waitForExplicitFinish();
>  
> +function checkForUpdate(aExpected, aOnSuccess, aOnApplied, aOnDownloadError, aLaunchDownload, aOnRequestError) {

nit: is this < 80 chars?
Attachment #806976 - Flags: review?(fabrice) → review+
Attached patch PatchSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=211e3df88180
Assignee: nobody → mcastelluccio
Attachment #806976 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #811003 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/83f7bb56300b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.