Closed
Bug 1439711
Opened 4 years ago
Closed 4 years ago
Refactor debugger server stepping tests
Categories
(DevTools :: Debugger, enhancement)
DevTools
Debugger
Tracking
(firefox60 fixed)
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(1 file, 1 obsolete file)
30.26 KB,
patch
|
jlast
:
review+
|
Details | Diff | Splinter Review |
It would be nice if the stepping tests used async/await.
Assignee | ||
Comment 1•4 years ago
|
||
Attachment #8952498 -
Flags: review?(ttromey)
Comment 2•4 years ago
|
||
Comment on attachment 8952498 [details] [diff] [review] ref-tests.patch Review of attachment 8952498 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for the patch. I think this makes the code much clearer. I found a few nits but nothing serious. ::: devtools/server/tests/unit/test_stepping-01.js @@ +13,4 @@ > var gCallback; > > function run_test() { > + do_test_pending(); I don't know the rationale for moving this. It's fine if it all works though. I noticed this is done in some files but not all of them... ::: devtools/server/tests/unit/test_stepping-02.js @@ +43,3 @@ > > + dumpn("Step Over to line 3"); > + const step1 = await stepIn(gClient, threadClient); The dump says "Step Over" but the code says step in. This appears in a few spots in this file. ::: devtools/server/tests/unit/test_stepping-03.js @@ +55,5 @@ > + this.a = 1; // 4 > + this.b = 2; // 5 > + } // 6 > + f(); // 7 > + `, // 4 The "// 4" on this line is mysterious. ::: devtools/server/tests/unit/test_stepping-05.js @@ +57,5 @@ > > + dumpn("Step In to line 5"); > + const step3 = await stepIn(gClient, threadClient); > + equal(step3.type, "paused"); > + equal(step3.frame.where.line, 4); The dump here says "line 5", but according to the code, there is no line 5 and the expected line is actually 4. And, the previous step went to line 4. This seems strange to me, I think at least the dump should be updated, and perhaps a comment explaining what is going on.
Attachment #8952498 -
Flags: review?(ttromey) → review+
Assignee | ||
Comment 3•4 years ago
|
||
Thanks for the feedback! https://treeherder.mozilla.org/#/jobs?repo=try&revision=34db41515eb139b6fcceb4f37c6d38b393cd436f
Assignee | ||
Comment 4•4 years ago
|
||
Attachment #8952498 -
Attachment is obsolete: true
Attachment #8953212 -
Flags: review+
Assignee | ||
Updated•4 years ago
|
Assignee: nobody → jlaster
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/d3084609beef Refactor debugger server stepping tests. r=tromey
Keywords: checkin-needed
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d3084609beef
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Assignee | ||
Updated•4 years ago
|
Blocks: dbg-help-wanted
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•