Refactor debugger server stepping tests

RESOLVED FIXED in Firefox 60

Status

enhancement
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: jlast, Assigned: jlast)

Tracking

unspecified
Firefox 60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

It would be nice if the stepping tests used async/await.
Posted patch ref-tests.patch (obsolete) — Splinter Review
Attachment #8952498 - Flags: review?(ttromey)
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+
Attachment #8952498 - Attachment is obsolete: true
Attachment #8953212 - Flags: review+
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
https://hg.mozilla.org/mozilla-central/rev/d3084609beef
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.