Closed Bug 1242899 Opened 8 years ago Closed 7 years ago

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

Categories

(Firefox Graveyard :: Web Apps, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 47

People

(Reporter: marco, Assigned: myk)

References

Details

Attachments

(1 file)

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.
Out of curiosity, why does this block bug 1238576?
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.
Stopping to expose it is essentially hiding it behind a pref that is disabled by default.  :-)
(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.
(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.
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/.
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
Assignee: nobody → myk
Status: NEW → ASSIGNED
Attachment #8713913 - Flags: review?(mcastelluccio)
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.
Attachment #8713913 - Flags: review?(mcastelluccio) → review+
https://hg.mozilla.org/mozilla-central/rev/c0dbae286703
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.