1242899 - Migrate dom/tests/mochitest/webapps to dom/apps/tests

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Marco Castelluccio [:marco]]

Some of the tests in dom/tests/mochitest/webapps depend on the Web Apps desktop implementation, but are not actually related to it (they are valid for b2g as well).
We should rewrite the ones we still need and move them to dom/apps/tests.

Comment 1

[(no longer active)]

Out of curiosity, why does this block bug 1238576?

Comment 2

[Marco Castelluccio [:marco]]

I think these tests are going to start failing if we remove navigator.mozApps. If we can still expose it for tests, then this should block bug 1238079 and not bug 1238576.

Comment 3

[(no longer active)]

Stopping to expose it is essentially hiding it behind a pref that is disabled by default.  :-)

Comment 4

[Myk Melez [:myk] [@mykmelez]]

(In reply to Marco Castelluccio [:marco] from comment #0)
> We should rewrite the ones we still need and move them to dom/apps/tests.

Which tests do we still need?

(In reply to :Ehsan Akhgari from comment #3)
> Stopping to expose it is essentially hiding it behind a pref that is
> disabled by default.  :-)

There's desktop/Android-specific code in Webapps.jsm that we will remove once we no longer support those platforms, at which point it won't be sufficient to merely disable mozApps on desktop/Android.  But that might be a sufficient first step.

Comment 5

[(no longer active)]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #4)
> (In reply to :Ehsan Akhgari from comment #3)
> > Stopping to expose it is essentially hiding it behind a pref that is
> > disabled by default.  :-)
> 
> There's desktop/Android-specific code in Webapps.jsm that we will remove
> once we no longer support those platforms, at which point it won't be
> sufficient to merely disable mozApps on desktop/Android.  But that might be
> a sufficient first step.

Yeah, agreed.  My point was that bug 1238576 is about hiding the non-standard API from Web content.  That doesn't need to happen at the same time as removing the underlying code that is no longer going to be used.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

The tests in question are:

* test_bug_765063.xul
* test_bug_771294.xul
* test_cross_origin.xul
* test_getNotInstalled.xul
* test_install_app.xul
* test_install_errors.xul
* test_install_utf8.xul
* test_launch_paths.xul
* test_list_api.xul

Of these, only test_bug_771294.xul is specific to the desktop runtime.  The others are generic tests of the mozApps API.  I'm not yet sure how much overlap there is between them and the tests in dom/apps/tests/.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Attachment: consolidate test files in one directory

Upon further reflection, it isn't necessary to carefully identify overlap and refactor test files in order to resolve this bug.  Since both sets of tests are currently being run, putting them all in one directory is a neutral change.  Any further refactoring (and disabling the desktop-specific test) can be a followup.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fa96808fdfe

Comment 8

[Marco Castelluccio [:marco]]

Comment on attachment 8713913 [details] [diff] [review]
consolidate test files in one directory

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

LGTM.

We can figure out how to make these tests run also without the desktop implementation
when we remove it.

I think the only things we need to rewrite are the things in head.js (e.g. we won't
be able to use confirmNextPopup anymore, since there won't be a popup), although
I haven't checked in detail.

There are definitely some tests that are redundant with the ones in dom/apps/tests,
we could remove them.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0dbae286703

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c0dbae286703