Closed Bug 1478245 Opened 2 years ago Closed 2 years ago

Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s

Categories

(DevTools :: Debugger, enhancement, P3)

Product:
DevTools
Component:
Debugger
Type:
enhancement

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: jdescottes, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details 
Tests skipped in e10s are no longer running and should be enabled or removed.
Priority: -- → P3
Assignee: nobody → poirot.alex
Depends on: 1478244
Comment on attachment 8996395 [details]
Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.

https://reviewboard.mozilla.org/r/260500/#review267724

Looks like one more test passing on e10s :) 
If you feel like cleaning up, part of the test is actually not testing anything and can be fixed. 
Otherwise we can have it as a followup.

::: devtools/client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js:47
(Diff revision 2)
>  
> -  gClient.request({ to: aGrip.testOneActor, type: "ping" }, aResponse => {
> -    is(aResponse.pong, "pong",
> +  const response = await client.request({ to: grip.testOneActor, type: "ping" });
> +  is(response.pong, "pong",
> -      "Actor should respond to requests.");
> +     "Actor should respond to requests.");
>  
> -    deferred.resolve(aResponse.actor);
> +  return response.actor;

"ping" does not return an actor property so this is undefined, and the rest of the test is invalid.

::: devtools/client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js:57
(Diff revision 2)
> -      gClient.request({ to: aTestActor, type: "ping" }, aResponse => {
> +    const response = await client.request({ to: actor, type: "ping" });
> -        ok(false, "testTargetScopedActor1 didn't go away with the tab.");
> +    ok(false, "testTargetScopedActor1 didn't go away with the tab.");
> -        deferred.reject(aResponse);
> +    throw response;
> -      });
> -    } catch (e) {
> +  } catch (e) {
> -      is(e.message, "'ping' request packet has no destination.", "testOnActor went away.");
> +    is(e.message, "'ping' request packet has no destination.", "testOnActor went away.");

If we target the correct actor, the error message we will get is:

  'ping' active request packet to 'server1.conn0.child2/testOneActor2' can't be sent as the connection just closed.
  
We can also leverage Assert.rejects here:

  async function closeTab(client, grip) {
    await removeTab(gBrowser.selectedTab);
    await Assert.rejects(
      client.request({ to: grip.testOneActor, type: "ping" }),
      err => err.message === `'ping' active request packet to '${grip.testOneActor}' ` +
                             `can't be sent as the connectiond just closed.`,
      "testOneActor went away.");
  }
Attachment #8996395 - Flags: review?(jdescottes) → review+
Comment on attachment 8996395 [details]
Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.

https://reviewboard.mozilla.org/r/260500/#review267724

> If we target the correct actor, the error message we will get is:
> 
>   'ping' active request packet to 'server1.conn0.child2/testOneActor2' can't be sent as the connection just closed.
>   
> We can also leverage Assert.rejects here:
> 
>   async function closeTab(client, grip) {
>     await removeTab(gBrowser.selectedTab);
>     await Assert.rejects(
>       client.request({ to: grip.testOneActor, type: "ping" }),
>       err => err.message === `'ping' active request packet to '${grip.testOneActor}' ` +
>                              `can't be sent as the connectiond just closed.`,
>       "testOneActor went away.");
>   }

Argh don't copy paste my exact code! I was testing the Assert.rejects api and left an intentional error ("connectiond" instead of "connection"). 

async function closeTab(client, grip) {
  await removeTab(gBrowser.selectedTab);
  await Assert.rejects(
    client.request({ to: grip.testOneActor, type: "ping" }),
    err => err.message === `'ping' active request packet to '${grip.testOneActor}' ` +
                           `can't be sent as the connection just closed.`,
    "testOneActor went away.");
}
(In reply to Julian Descottes [:jdescottes][:julian] from comment #4)
> "ping" does not return an actor property so this is undefined, and the rest
> of the test is invalid.

The previous may have meant to use response.from...
But I'm not sure there is any value to use that instead of grip.testOneActor.
So I just used your Assert.rejects code as-is.
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8134023cb87
Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/a8134023cb87
Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.