963674 - toolkit/identity xpcshell tests fail due to xul appinfo registration

Status: RESOLVED FIXED

(Core Graveyard :: Identity, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

toolkit/identity tests started failing on me this morning after all individual tests passed.  The error message is tantalizingly unhelpful:

 0:19.15 TEST-PASS | (xpcshell/head.js) | 1 (+ 0) check(s) passed
 0:19.15
 0:19.15 TEST-INFO | (xpcshell/head.js) | 0 check(s) todo
 0:19.27 ************************************************************
 0:19.27 * Call to xpconnect wrapped JSObject produced this error:  *

 0:46.35 TEST-UNEXPECTED-FAIL | /Users/zeus/code/firefox/build_unified_b2g/_tests/xpcshell/toolkit/identity/tests/unit/test_minimalidentity.js | test failed (with xpcshell return code: -11)

I find that I remove the following line from head_identity.js, all is well:

registrar.registerFactory(Components.ID("{fbfae60b-64a4-44ef-a911-08ceb70b9f31}"),
                          "XULAppInfo", "@mozilla.org/xre/app-info;1",
                          XULAppInfoFactory);

Why do we even have a xul factory of any kind in a headless xpcshell test to begin with?

Can we just remove this line?

Comment 1

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: Remove registration of xulappinfo

- removing xulappinfo registration allows tests to pass
- made some stylistic improvements to lists of tests (final comma)
- bonus: added shutdown() method to MinimalIdentity to match Identity.jsm

MattN, I don't understand what the xulappinfo business in head_identity was about.  As of yesterday, it started breaking.  This appears to fix the problem.  Is this an acceptable solution?

Thanks for looking
j

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8365221 [details] [diff] [review]
Remove registration of xulappinfo

can you provide more of the log for the failure as I think you are missing the relevant part? Please attach the whole log. Also, what kind of build is it failing on? It works for me on an OS X build.

Some components need the appinfo in order to work so I suspect it was required at one point. If we are going to remove it you should remove the XULAppInfoFactory object too.

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Matthew N. [:MattN] from comment #2)
> Comment on attachment 8365221 [details] [diff] [review]
> Remove registration of xulappinfo
> 
> can you provide more of the log for the failure as I think you are missing
> the relevant part? Please attach the whole log. Also, what kind of build is
> it failing on? It works for me on an OS X build.

That's the tantalizing part.  The important bits are not printed!

I'll attach the complete log from start to finish.

> Some components need the appinfo in order to work so I suspect it was
> required at one point. If we are going to remove it you should remove the
> XULAppInfoFactory object too.

Ok.

Comment 4

[Matthew N. [:MattN]]

(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #3)
> That's the tantalizing part.  The important bits are not printed!

Try running a single test in that directory.

> > Also, what kind of build is it failing on? It works for me on an OS X build.

I think you missed this question.

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: test_minimalidentity.log.txt

Here's the complete log I get (also building on osx, like you); latest m-c.

Comment 6

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: Remove registration of xulappinfo

Comment 7

[Matthew N. [:MattN]]

(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #5)
> Created attachment 8365270 [details]
> test_minimalidentity.log.txt
> 
> Here's the complete log I get (also building on osx, like you); latest m-c.

Is this a Firefox or B2G build?

Comment 8

[Matthew N. [:MattN]]

Also, is this only a local failure or does it happen on Try?

Comment 9

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

This is a b2g build.

I'm running locally.  (Attempts to push to try are just hanging on me this morning. :(

My mozconfig options:

ac_add_options --disable-crashreporter
ac_add_options --disable-optimize
ac_add_options --disable-static
ac_add_options --enable-debug
ac_add_options --enable-shared
ac_add_options --with-ccache
ac_add_options --enable-tests
ac_add_options --enable-warning-as-errors
ac_add_options --enable-application=b2g

My mozilla-central tip: bfe4ed6d47ce
I'm building on OSX 10.8.5, xcode 5.0

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8365271 [details] [diff] [review]
Remove registration of xulappinfo

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

The tests seem to pass without this now so I guess this is fine. Please check on Try before pushing.

::: toolkit/identity/MinimalIdentity.jsm
@@ +230,5 @@
>  
>    doLogin: function doLogin(aRpCallerId, aAssertion, aInternalParams) {
>      let rp = this._rpFlows[aRpCallerId];
>      if (!rp) {
> +      log("WARNING: doLogin found no rp to go with callerId " + aRpCallerId + "\n");

When switching to |log|, remove the "\n". This applies below as well.

Comment 11

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: Remove registration of xulappinfo

r=MattN

Thanks, Matthew!
j

Comment 12

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

try push is pleasantly green:

https://tbpl.mozilla.org/?tree=Try&rev=b66f64e1c288

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e19c247580d

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8e19c247580d