Closed Bug 914604 Opened 6 years ago Closed 6 years ago

Add test for app reinstall and redirects when installing an app via the app actor

Categories

(DevTools :: WebIDE, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(1 file, 2 obsolete files)

So far, testing app installation was challenging as xpcshell doesn't allow to create a mozbrowser frame... But thanks to some tweaks made to mochitest, like bug 894990, we should be able to write a mochitest against the webapps actor and test all its features!
Depends on: 914633, 894990
Comment on attachment 803013 [details] [diff] [review]
Updated after modification during review of bug 912475 and 894990

I haven't got a review for bug 914633, but receive positive feedback, so hopefully this patch won't have any other modification due to its dependencies.

I would like to land this patch in FF26 in order to finally have good unit tests for the app actor!
Attachment #803013 - Flags: review?(fabrice)
Blocks: 912475
Comment on attachment 803013 [details] [diff] [review]
Updated after modification during review of bug 912475 and 894990

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

::: toolkit/devtools/apps/tests/debugger-protocol-helper.js
@@ +67,5 @@
> +// Install a test packaged webapp from data folder
> +addMessageListener("install", function (aMessage) {
> +  let url = aMessage.url;
> +  let appId = aMessage.appId;
> + 

nit: whitespace

::: toolkit/devtools/apps/tests/redirect.sjs
@@ +1,1 @@
> +var gBasePath = "tests/dom/apps/tests/";

Unused.

::: toolkit/devtools/apps/tests/test_webapps_actor.html
@@ +41,5 @@
> +    ok(false, "Caught exception", ex);
> +  }
> +}
> +
> +function go() {

I would call that start(), but up to you.
Attachment #803013 - Flags: review?(fabrice) → review+
I fixed review comments, but also had to remove redirects test that triggers an assertion.
I'll open a bug to investigate that in a followup.
Attachment #803013 - Attachment is obsolete: true
Depends on: 916604
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/c63bb927cf78
Status: NEW → ASSIGNED
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c63bb927cf78
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 26
You're doing great work adding tests and features to SpecialPowers.
But instead of using SpecialPowers.addPermission, you should use SpecialPowers.pushPermissions, which automatically removes the permissions when the test file is done (similar to pushPrefEnv).

+  function() {
+    ok(true, "all done!\n");
+    SpecialPowers.popPrefEnv(finish);

This shouldn't be necessary, as prefs would be automatically be undone after the test file has been run with pushPrefEnv. When you would test it standalone, there is bug 914890, when that bug is fixed, it would also be fixed for standalone tests.
Summary: Add test for app resinstall and redirects when installing an app via the app actor → Add test for app reinstall and redirects when installing an app via the app actor
This needed build peer review...
Flags: needinfo?(poirot.alex)
Comment on attachment 805045 [details] [diff] [review]
Adressed comments and removed redirects test

Oops, this patch already landed but we forgot to flag a build peer during review.

Mike, could you do a buildwise review?
Attachment #805045 - Flags: review?(mh+mozilla)
Flags: needinfo?(poirot.alex)
Comment on attachment 805045 [details] [diff] [review]
Adressed comments and removed redirects test

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

::: toolkit/devtools/apps/tests/Makefile.in
@@ +7,5 @@
> +srcdir          = @srcdir@
> +VPATH           = @srcdir@
> +relativesrcdir = @relativesrcdir@
> +
> +include $(DEPTH)/config/autoconf.mk

Please remove this boilerplate, and the rules.mk include.
Attachment #805045 - Flags: review?(mh+mozilla)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.