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

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Tests skipped in e10s are no longer running and should be enabled or removed.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.

Review commit: https://reviewboard.mozilla.org/r/260500/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260500/

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8996395 [details]
Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260500/diff/1-2/

Comment 3

[Alexandre Poirot [:ochameau]]

try:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=5afa1ee8e8f3d925307f78773c17c74fca413388

Comment 4

[Julian Descottes [:jdescottes]]

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.");
  }

Comment 5

[Julian Descottes [:jdescottes]]

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.");
}

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 8996395 [details]
Bug 1478245 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/260500/diff/2-3/

Comment 7

[Alexandre Poirot [:ochameau]]

(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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8134023cb87
Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-02.js in e10s. r=jdescottes

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/a8134023cb87