1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.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 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.

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

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8994761 [details]
Bug 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.

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

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8994761 [details]
Bug 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.

https://reviewboard.mozilla.org/r/259294/#review266266

Looks good to me and passes locally! Thanks for cleaning up the test at the same time.
I assume you want to reuse this for browser_dbg_target-scoped-actor-02.js and browser_dbg_globalactor.js

::: devtools/client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js:30
(Diff revision 2)
> -  let transport = DebuggerServer.connectPipe();
> -  gClient = new DebuggerClient(transport);
> -  gClient.connect().then(([aType, aTraits]) => {
> -    is(aType, "browser",
> -      "Root actor should identify itself as a browser.");
> -
> +  const transport = DebuggerServer.connectPipe();
> +  const client = new DebuggerClient(transport);
> +  const [ type ] = await client.connect();
> +  is(type, "browser", "Root actor should identify itself as a browser.");
> +
> +  const [ grip ] = await attachTargetActorForUrl(client, TAB_URL)

missing semicolon

::: devtools/client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js:32
(Diff revision 2)
> -  gClient.connect().then(([aType, aTraits]) => {
> -    is(aType, "browser",
> -      "Root actor should identify itself as a browser.");
> -
> -    addTab(TAB_URL)
> -      .then(() => attachTargetActorForUrl(gClient, TAB_URL))
> +  const [ type ] = await client.connect();
> +  is(type, "browser", "Root actor should identify itself as a browser.");
> +
> +  const [ grip ] = await attachTargetActorForUrl(client, TAB_URL)
> +  await testTargetScopedActor(client, grip);
> +  closeTab();

we could inline `removeTab(gBrowser.selectedTab)`

::: devtools/client/shared/test/shared-head.js:727
(Diff revision 2)
>        "chrome://mochikit/content/tests/SimpleTest/EventUtils.js", EventUtils);
>    });
>  }
> +
> +/**
> + * Register an actor in the content procecss of the current tab.

nit: procecss -> process

Comment 4

[Alexandre Poirot [:ochameau]]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #3)
> Comment on attachment 8994761 [details]
> Bug 1478244 - Enable
> client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.
> 
> https://reviewboard.mozilla.org/r/259294/#review266266
> 
> Looks good to me and passes locally! Thanks for cleaning up the test at the
> same time.
> I assume you want to reuse this for browser_dbg_target-scoped-actor-02.js
> and browser_dbg_globalactor.js

Yes, and also in bug 1477988, where I originaly crafted that code.
I thought there was more tests using registerModule, but it looks like it is mostly xpcshell ones:
  https://searchfox.org/mozilla-central/search?q=DebuggerServer.registerModule&path=
And they don't care about e10s turned on/off as there is to browser UI...

I'll try to land this first.

Comment 5

[Alexandre Poirot [:ochameau]]

Ah, there is a conflict with this method, with the same name:
  https://searchfox.org/mozilla-central/source/devtools/client/shared/test/test-actor-registry.js#16
injected into shared-head scope over here:
  https://searchfox.org/mozilla-central/source/devtools/client/inspector/test/shared-head.js#12-15

I imagine I should name the new one differently...

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 8994761 [details]
Bug 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.

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

Comment 7

[Alexandre Poirot [:ochameau]]

New patch with "registerActorInContentProcess". Tell me if you have better naming in mind.

Comment 8

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s c561a298736c7ec4c25c6aaba24c0b7b164e5e38 -d c79c7af9fa0e: rebasing 474554:c561a298736c "Bug 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s. r=jdescottes" (tip)
merging devtools/client/shared/test/shared-head.js
warning: conflicts while merging devtools/client/shared/test/shared-head.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 9

[Alexandre Poirot [:ochameau]]

Comment on attachment 8994761 [details]
Bug 1478244 - Enable client/debugger/test/mochitest/browser_dbg_target-scoped-actor-01.js in e10s.

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

Comment 10

[Pulsebot]

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

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/80a3f2a159e7