Closed Bug 963674 Opened 6 years ago Closed 6 years ago

toolkit/identity xpcshell tests fail due to xul appinfo registration

Categories

(Core Graveyard :: Identity, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: jedp, Assigned: jedp)

References

Details

Attachments

(2 files, 2 obsolete files)

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?
- 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
Attachment #8365221 - Flags: review?(MattN+bmo)
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.
Attachment #8365221 - Flags: review?(MattN+bmo) → review-
(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.
(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.
Here's the complete log I get (also building on osx, like you); latest m-c.
Attachment #8365221 - Attachment is obsolete: true
(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?
Also, is this only a local failure or does it happen on Try?
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 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.
Attachment #8365271 - Flags: review+
r=MattN

Thanks, Matthew!
j
Attachment #8365271 - Attachment is obsolete: true
Keywords: checkin-needed
Summary: toolkit/identity xpcshell tests are hosed → toolkit/identity xpcshell tests fail due to xul appinfo registration
https://hg.mozilla.org/mozilla-central/rev/8e19c247580d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.