Closed Bug 1458808 Opened 7 years ago Closed 6 years ago

Application panel: add test for debugging a service worker

Categories

(DevTools :: Application Panel, enhancement, P3)

Product:
DevTools
Component:
Application Panel
Version:
61 Branch
Type:
enhancement

Tracking

(firefox61 wontfix, firefox62 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1458808 - add test to debug serviceworker from application panel;
(deleted), text/x-review-board-request
ladybenko
: review+
jlast
: review+
Details
We should add a mochitest checking that we can: - list a service worker in the application panel - click on debug - set a breakpoint on the service worker script - hit the breakpoint - resume This was not tested so far in about:debugging and regressed regularly in the past months.
We need to split the debugger's head JS file in order to reuse the helper functions. In the meantime I could simply duplicate it in order to land this test. Opened PR https://github.com/devtools-html/debugger.html/pull/6207 to split head.js. Landing this might be complicated. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=15ff5a33c83a0527a6e5298a3bc8188c785bde65
Mass move to the new application panel component.
Component: Developer Tools → Developer Tools: Application Panel
head/helper.js split should land in v50
Depends on: 1460056
Attachment #8972834 - Attachment is obsolete: true
Comment on attachment 8972833 [details] Bug 1458808 - add test to debug serviceworker from application panel; https://reviewboard.mozilla.org/r/241400/#review249136 Thanks a lot! I have a few minor comments ::: devtools/client/application/test/browser_application_panel_debug-service-worker.js:20 (Diff revision 2) > + > + let { panel, tab, target } = await openNewTabAndApplicationPanel(TAB_URL); > + let doc = panel.panelWin.document; > + > + info("Wait until the service worker appears in the application panel"); > + await waitUntil(() => getWorkerContainers(doc).length == 1); `===` ::: devtools/client/application/test/browser_application_panel_debug-service-worker.js:24 (Diff revision 2) > + info("Wait until the service worker appears in the application panel"); > + await waitUntil(() => getWorkerContainers(doc).length == 1); > + > + let container = getWorkerContainers(doc)[0]; > + info("Wait until the debug link is displayed and enabled"); > + await waitUntil(() => container.querySelector(".debug-link:not(.disabled)")); A note here: In the UI patch (with BEM-like class names) we are also making the move to use prefixed class selectors as JS hooks to separate them for styled classes. If this patch gets merged first, I can always update this file in the other one. ::: devtools/client/application/test/service-workers/debug.html:1 (Diff revision 2) > +<!DOCTYPE HTML> We're putting a license header in the other test HTML files, we probably should include it here too ::: devtools/client/debugger/new/test/mochitest/helpers.js:13 (Diff revision 2) > > var { Toolbox } = require("devtools/client/framework/toolbox"); > var { Task } = require("devtools/shared/task"); > > const sourceUtils = { > - isLoaded: source => source.get("loadedState") === "loaded" > +isLoaded: source => source.get("loadedState") === "loaded" It seems most of the indentation in this file has been accidentally removed
Attachment #8972833 - Flags: review?(balbeza) → review+
Comment on attachment 8972833 [details] Bug 1458808 - add test to debug serviceworker from application panel; https://reviewboard.mozilla.org/r/241400/#review249136 Thanks a lot for the review! > A note here: In the UI patch (with BEM-like class names) we are also making the move to use prefixed class selectors as JS hooks to separate them for styled classes. > > If this patch gets merged first, I can always update this file in the other one. I plan to wait for your patch before landing this one, and will rebase on it as soon as I can. > We're putting a license header in the other test HTML files, we probably should include it here too Good catch, done! Would be great to see if we can lint this as well. > It seems most of the indentation in this file has been accidentally removed Good catch, probably a bad merge. Dropped the file I don't need to modify it here.
Comment on attachment 8972833 [details] Bug 1458808 - add test to debug serviceworker from application panel; https://reviewboard.mozilla.org/r/241400/#review249306 ::: devtools/client/application/test/browser_application_panel_debug-service-worker.js:24 (Diff revision 3) > + info("Wait until the service worker appears in the application panel"); > + await waitUntil(() => getWorkerContainers(doc).length === 1); > + > + let container = getWorkerContainers(doc)[0]; > + info("Wait until the debug link is displayed and enabled"); > + await waitUntil(() => container.querySelector(".debug-link:not(.disabled)")); it'd be nice to wait on something in the redux store here. I've found that to be more reliable ::: devtools/client/application/test/browser_application_panel_debug-service-worker.js:31 (Diff revision 3) > + info("Click on the debug link and wait for the new toolbox to be ready"); > + let onToolboxReady = new Promise(done => { > + gDevTools.once("toolbox-ready", function(toolbox) { > + done(toolbox); > + }); > + }); there should be a waitForEvent helper that does this for you `waitForEvent(gToolbox, "toolbox-ready")`
Attachment #8972833 - Flags: review?(jlaster) → review+
Comment on attachment 8972833 [details] Bug 1458808 - add test to debug serviceworker from application panel; https://reviewboard.mozilla.org/r/241400/#review249306 > there should be a waitForEvent helper that does this for you > > `waitForEvent(gToolbox, "toolbox-ready")` I can actually just do let onToolboxReady = gDevTools.once("toolbox-ready");
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/857fce2b698b add test to debug serviceworker from application panel;r=jlast,ladybenko
https://hg.mozilla.org/mozilla-central/rev/857fce2b698b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: