Closed Bug 831474 Opened 8 years ago Closed 8 years ago

Update revision of Jetpack tests used in Firefox tests once bug 825471 lands

Categories

(Add-on SDK Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KWierso, Unassigned)

References

Details

(Whiteboard: [leave open])

Once the pull request in bug 825471 lands, we need to update the jetpack revision used in mozilla-central's tests to get them testing the new stuff, and to green up the tree a bit.
Does this apply to Aurora as well?  Do you expect this to fix the Jetpack perma-oranges on the Birch branch?
(In reply to :Ehsan Akhgari from comment #1)
> Does this apply to Aurora as well?  Do you expect this to fix the Jetpack
> perma-oranges on the Birch branch?

If we could get approval-aurora+, that'd be nice. But this only affects tests, not actual addon code, so it's not really urgent.

The failure on Birch isn't related to private-browsing, but something with the garbage collector:

error: addon-sdk: TEST FAILED: test-disposable.test disposables are GC-able (failure)
error: addon-sdk: fail: GC removed dispose listeners - 0 == 1
info: addon-sdk: Traceback (most recent call last):
  File "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/timers.js", line 31, in notify
    callback.apply(null, args);
  File "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/tests/test-disposable.js", line 121, in exports["test disposables are GC-able"]/<
    assert.equal(disposals, 0, "GC removed dispose listeners");
  File "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/test/assert.js", line 116, in equal
    operator: "=="
  File "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/test/assert.js", line 80, in fail
    this._log.fail(message);
  File "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/deprecated/unit-test.js", line 75, in fail
    this.console.trace();

Odd that this isn't showing up anywhere else, though. Irakli wrote those tests, maybe he knows?
Yeah it that's test actually is really tricky it tries to make GC collect listener
and see if listener is still invoked. Although I suspect on that specific setup GC does not actually claims listener and it's still called.

Have no idea how to actually force GC collection though :(
(In reply to comment #2)
> (In reply to :Ehsan Akhgari from comment #1)
> > Does this apply to Aurora as well?  Do you expect this to fix the Jetpack
> > perma-oranges on the Birch branch?
> 
> If we could get approval-aurora+, that'd be nice. But this only affects tests,
> not actual addon code, so it's not really urgent.

If it's test-only, you can land it with a=test-only.  No need to get approval.

> The failure on Birch isn't related to private-browsing, but something with the
> garbage collector:
> 
> error: addon-sdk: TEST FAILED: test-disposable.test disposables are GC-able
> (failure)
> error: addon-sdk: fail: GC removed dispose listeners - 0 == 1
> info: addon-sdk: Traceback (most recent call last):
>   File
> "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/timers.js",
> line 31, in notify
>     callback.apply(null, args);
>   File
> "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/tests/test-disposable.js",
> line 121, in exports["test disposables are GC-able"]/<
>     assert.equal(disposals, 0, "GC removed dispose listeners");
>   File
> "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/test/assert.js",
> line 116, in equal
>     operator: "=="
>   File
> "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/test/assert.js",
> line 80, in fail
>     this._log.fail(message);
>   File
> "resource://71bdea69-0efe-4ff3-ad65-ed54f22894c6-at-jetpack/addon-sdk/lib/sdk/deprecated/unit-test.js",
> line 75, in fail
>     this.console.trace();
> 
> Odd that this isn't showing up anywhere else, though. Irakli wrote those tests,
> maybe he knows?

Thanks for the explanation.  Do I need to file another bug for that?
(In reply to comment #3)
> Yeah it that's test actually is really tricky it tries to make GC collect
> listener
> and see if listener is still invoked. Although I suspect on that specific setup
> GC does not actually claims listener and it's still called.
> 
> Have no idea how to actually force GC collection though :(

Is that test orange everywhere or is it just Birch?
(In reply to :Ehsan Akhgari from comment #4)
> Thanks for the explanation.  Do I need to file another bug for that?

I just filed bug 831517 for it, and just pushed a one-line patch for it which seemed to fix it for me, testing locally. So once we get the pwpb stuff landed, and then land this bug on aurora, it should green up Birch on the following m-a->birch merge.
(In reply to :Ehsan Akhgari from comment #5)
> (In reply to comment #3)
> > Yeah it that's test actually is really tricky it tries to make GC collect
> > listener
> > and see if listener is still invoked. Although I suspect on that specific setup
> > GC does not actually claims listener and it's still called.
> > 
> > Have no idea how to actually force GC collection though :(
> 
> Is that test orange everywhere or is it just Birch?

I'm only seeing it on Birch at the moment.
OS: Windows 8 → All
Hardware: x86_64 → All
(In reply to comment #6)
> (In reply to :Ehsan Akhgari from comment #4)
> > Thanks for the explanation.  Do I need to file another bug for that?
> 
> I just filed bug 831517 for it, and just pushed a one-line patch for it which
> seemed to fix it for me, testing locally. So once we get the pwpb stuff landed,
> and then land this bug on aurora, it should green up Birch on the following
> m-a->birch merge.

Thanks a lot!  :-)
Landed this: http://hg.mozilla.org/integration/mozilla-inbound/rev/3ee6b0c9bab9
It doesn't include the new pwpb stuff, but it does green up most of our tests, so landing this now. 

Lets leave this open so we can get the pwpb stuff in once it lands.
Whiteboard: [leave open]
Bug 825471 landed a few weeks ago, but it didn't get marked as fixed until tonight. I landed Jetpack test updates to mozilla-central and mozilla-aurora last week, so I'm calling this fixed.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.