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)
Firefox Graveyard
Web Apps
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 47
People
(Reporter: marco, Assigned: myk)
References
Details
Attachments
(1 file)
35.47 KB,
patch
|
marco
:
review+
|
Details | Diff | Splinter Review |
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•8 years ago
|
||
Out of curiosity, why does this block bug 1238576?
Reporter | ||
Comment 2•8 years ago
|
||
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•8 years ago
|
||
Stopping to expose it is essentially hiding it behind a pref that is disabled by default. :-)
Assignee | ||
Comment 4•8 years ago
|
||
(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•8 years ago
|
||
(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.
Assignee | ||
Comment 6•8 years ago
|
||
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/.
Assignee | ||
Comment 7•8 years ago
|
||
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
Reporter | ||
Comment 8•8 years ago
|
||
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+
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c0dbae286703
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Updated•7 years ago
|
Product: Firefox → Firefox Graveyard
Reporter | ||
Updated•7 years ago
|
status-firefox47:
fixed → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•