1231759 - --jsdebugger with xpcshell is broken for some tests

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[:Gijs (he/him)]

STR (on OSX):

1. ./mach xpcshell-test --jsdebugger toolkit/modules/tests/xpcshell/test_PermissionsUtils.js

ER:

get debugger

AR:

no debugger, following error in the console:

 0:00.56 LOG: Thread-1 INFO "Failed to initialize debugging: [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIXPCComponents_Utils.import]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: /Users/gkruitbosch/dev/fx-team/testing/xpcshell/head.js :: _setupDebuggerServer :: line 386"  data: no]
_setupDebuggerServer@/Users/gkruitbosch/dev/fx-team/testing/xpcshell/head.js:386:21
_initDebugging@/Users/gkruitbosch/dev/fx-team/testing/xpcshell/head.js:436:24
_execute_test@/Users/gkruitbosch/dev/fx-team/testing/xpcshell/head.js:475:7
@-e:1:1

That line is:

  let { require } = Components.utils.import("resource://devtools/shared/Loader.jsm", {});


so I'm assuming this is what busted it... Ryan, any chance you can look into this?

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

For xpcshell to access the resource://devtools files, it needs to have the "-a" argument (for app dir) to be set to the browser subdir of the install path.

It's possible to make this happen by setting:

  firefox-appdir = browser

in the xpcshell.ini file.  

This can also change the functionality of the test directory by altering what modules are loaded by the test environments in other ways as well, so it can't very easily be enabled everywhere unfortunately.

I investigated changing this globally, so that it would not need to be set in each xpcshell.ini in bug 1215378, but I ended up with too many test failures to sort through.

Editing the xpcshell.ini for the directory you mentioned appears to resolve the issue.

Comment 2

[:Gijs (he/him)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) (at work week until Dec. 14) from comment #1)
> For xpcshell to access the resource://devtools files, it needs to have the
> "-a" argument (for app dir) to be set to the browser subdir of the install
> path.
> 
> It's possible to make this happen by setting:
> 
>   firefox-appdir = browser
> 
> in the xpcshell.ini file.  
> 
> This can also change the functionality of the test directory by altering
> what modules are loaded by the test environments in other ways as well, so
> it can't very easily be enabled everywhere unfortunately.
> 
> I investigated changing this globally, so that it would not need to be set
> in each xpcshell.ini in bug 1215378, but I ended up with too many test
> failures to sort through.
> 
> Editing the xpcshell.ini for the directory you mentioned appears to resolve
> the issue.

Can we guard for this and print a nicer (and more visible) message? Right now we just continue running the test(s) without showing the debugger, which is unfortunate.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to :Gijs Kruitbosch from comment #2)
> Can we guard for this and print a nicer (and more visible) message? Right
> now we just continue running the test(s) without showing the debugger, which
> is unfortunate.

Sure, I'll take a look.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1231759 - Explain DevTools appdir issue when debugging xpcshell. r=Gijs

Bug 1231759 - Explain DevTools appdir issue when debugging xpcshell. r=Gijs

Comment 5

[:Gijs (he/him)]

Comment on attachment 8698470 [details]
MozReview Request: Bug 1231759 - Explain DevTools appdir issue when debugging xpcshell. r=Gijs

https://reviewboard.mozilla.org/r/28013/#review25073

Untested, but this looks great from code inspection, thanks!

::: testing/xpcshell/head.js:388
(Diff revision 1)
> +    ({ require } = Components.utils.import("resource://devtools/shared/Loader.jsm", {}));

Interesting syntax. I presume you need the extra () because JS doesn't like `try {{` for some reason?

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/28013/#review25073

> Interesting syntax. I presume you need the extra () because JS doesn't like `try {{` for some reason?

It's destructuring when not prefixed by `let` on the same line.  The braces alone can't be distinguished from a new block.

Took me a while to learn that one. :)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Syntax

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/71c45c3742f4

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/71c45c3742f4