Closed Bug 999854 Opened 6 years ago Closed 6 years ago

Rewrite browser_dbg_pretty-print-on-paused.js to use Task.spawn

Categories

(DevTools :: Debugger, defect)

x86
macOS
defect
Not set

Tracking

(firefox30 fixed, firefox31 fixed)

RESOLVED FIXED
Firefox 31
Tracking Status
firefox30 --- fixed
firefox31 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(1 file, 2 obsolete files)

This test is a little funky at the moment and I suspect it might have latent races that are contributing to ejpbruel's headaches in bug 859372.
Attached patch rewrite-test.patch (obsolete) — Splinter Review
Can't push to try right now for some reason, will do it ASAP when I can.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attachment #8410700 - Flags: review?(vporof)
Attached patch rewrite-test.patch (obsolete) — Splinter Review
...and I messed that patch and try push up.

https://tbpl.mozilla.org/?tree=Try&rev=fc1ff1815d8c
Attachment #8410700 - Attachment is obsolete: true
Attachment #8410700 - Flags: review?(vporof)
Attachment #8410727 - Flags: review?(vporof)
The suite is passing fine locally; I think this is an infrastructure failure. Looks like the try tree is closed:

  CLOSED. Error 500 on git.m.o - Bug 985864
Comment on attachment 8410727 [details] [diff] [review]
rewrite-test.patch

Review of attachment 8410727 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/debugger/test/browser_dbg_pretty-print-on-paused.js
@@ +22,4 @@
>      gSources = gDebugger.DebuggerView.Sources;
>  
> +    Task.spawn(function* () {
> +      try {

I think you actually don't need to do this when using Task, since exceptions or failed promises are logged automatically. Ah, the glory of Task.

I don't mind having this here though.

@@ +40,3 @@
>  
> +        info("Switch to the second source.");
> +        const sourceShown = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.SOURCE_SHOWN);

Could use waitForSourceShown here which has the added benefit of also checking if the correct source was shown.

@@ +42,5 @@
> +        const sourceShown = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.SOURCE_SHOWN);
> +        gSources.selectedValue = SECOND_SOURCE_VALUE;
> +        yield sourceShown;
> +        is(gSources.selectedValue, SECOND_SOURCE_VALUE,
> +           "The second source should be selected.");

...in which case you don't need this anymore.

@@ +47,3 @@
>  
> +        info("Pretty print the source.");
> +        const prettyPrinted = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.SOURCE_SHOWN);

Ditto.
Attachment #8410727 - Flags: review?(vporof) → review+
Updated based on review comments.
Attachment #8410727 - Attachment is obsolete: true
Attachment #8412094 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/06d54318284e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Flags: needinfo?(ryanvm)
https://hg.mozilla.org/mozilla-central/rev/06d54318284e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.