603907 - test-panel.testResizePanel fails "panel was resized" test

Status: RESOLVED FIXED

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

Description

[Myk Melez [:myk] [@mykmelez]]

On the latest Firefox 4 nightly, test-panel.testResizePanel fails the "panel was resized" test:

error: TEST FAILED: test-panel.testResizePanel (failure)
error: fail: The panel was resized.
info: Traceback (most recent call last):
  File "resource://addon-kit-addon-kit-lib/panel.js", line 226, in _onHide
    this._emit('hide', this._public);
  File "resource://addon-kit-jetpack-core-lib/events.js", line 142, in _emit
    listener.apply(null, params);
  File "resource://addon-kit-addon-kit-tests/test-panel.js", line 54, in anonymous
    "The panel was resized.");
  File "resource://addon-kit-jetpack-core-lib/unit-test.js", line 196, in assert
    this.fail(message);
  File "resource://addon-kit-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();

Hernán: do you know what might be going on here?

Comment 1

[Myk Melez [:myk] [@mykmelez]]

The line number for the failing test in test-panel.js is slightly different now, but I suspect the failure is the same:

error: TEST FAILED: test-panel.testResizePanel (failure)
error: fail: The panel was resized.
info: Traceback (most recent call last):
  File "resource://addon-kit-addon-kit-lib/panel.js", line 226, in _onHide
    this._emit('hide', this._public);
  File "resource://addon-kit-jetpack-core-lib/events.js", line 142, in _emit
    listener.apply(null, params);
  File "resource://addon-kit-addon-kit-tests/test-panel.js", line 62, in anonymous
    "The panel was resized.");
  File "resource://addon-kit-jetpack-core-lib/unit-test.js", line 196, in assert
    this.fail(message);
  File "resource://addon-kit-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();


Note: I can only reproduce this failure when I run all tests.  When I only run the panel tests, it doesn't show up.  And I can't reproduce on Mac or Windows.

Comment 2

[Andreas Gal :gal]

Blocks a b7 blocker.

Same here, how do I run this test?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

I see this when I run |cfx test| in the packages/addon-kit/ directory.

Comment 4

[Johnny Stenback (:jst)]

Over to Myk for now, we need a reduced testcase here, please reassign appropriately.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

I've since run this test a bunch more times and observed test runs where the test did not fail, even when running all tests.  I have also observed a test run where the test did fail when running only panel tests.

So this looks like an intermittent timing-related failure rather than a regression from recent changes to GC compartments.  I'm not sure why it shows up on trunk but not b6.  Perhaps differences in the speed of interpretation under GC compartments reveal the timing-related error?

In any case, given the intermittence, this seems like a non-blocker and an SDK issue rather than a JS/XPConnect one.  Modifying and moving bug accordingly.

Comment 6

[Johnny Stenback (:jst)]

What I now see is this:

(jetpack-sdk)[jst@gandalf addon-kit]$ cfx test -F test --binary=/home/jst/work/tip/fb-rel/dist/bin/firefox
Using binary at '/home/jst/work/tip/fb-rel/dist/bin/firefox'.
Using profile at '/tmp/tmprvRQYd.mozrunner'.
Traceback (most recent call last):
  File "resource://addon-kit-addon-kit-lib/selection.js", line 233, in 
    off: function() this._removeAllListeners.apply(this, arguments)
  File "resource://addon-kit-jetpack-core-lib/traits.js", line 118, in Trait
    return self.constructor.apply(self, arguments) || self._public;
TypeError: self.constructor is undefined
No tests were run
FAIL
Total time: 4.091101 seconds
Program terminated unsuccessfully.

Which looks entirely different, and worse (?). I have fixes for bug 596031 and bug 607767 (newer patch than what's in the bug) in my tree.

Comment 7

[Tomas]

(In reply to comment #5)

> In any case, given the intermittence, this seems like a non-blocker and an SDK
> issue rather than a JS/XPConnect one.  Modifying and moving bug accordingly.

This bug still is marked as b7 blocker -> https://bugzilla.mozilla.org/query.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=cf_blocking_20&query_format=advanced&type0-0-0=substring&value0-0-0=beta7&known_name=ff%20b7

Comment 8

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 9

[Myk Melez [:myk] [@mykmelez]]

I'm seeing this consistently again on Linux.

Ayan: can you reproduce on your Linux box?

Comment 10

[Myk Melez [:myk] [@mykmelez]]

I dug into this a bit more, and one combination of tests that causes the test failure is context-menu and panel, i.e.:

  cfx test -F 'context-menu|panel'

There seem to be other combinations that cause the same failure, however, since merely excluding context-menu from the list of tests to run doesn't cause the failure to go away.

Also, it looks like the failure happens because the onHide handler gets called without the onShow handler ever getting called.  This suggests to me that a test in another test file is doing some work after that test has supposedly finished running, and that work has the side-effect of aborting the process of the panel being shown, such that the panel never finishes being shown (and thus never calls the onShow handler), but it does get hidden (and thus calls the onHide handler).

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem

It turns out that the test fails when the window in which the panel is being shown is not the active window.  Activating the window before running the test solves the problem.

Comment 12

[Drew Willcoxon :adw]

Comment on attachment 491881 [details] [diff] [review]
patch v1: fixes problem

Looks great.  But how do you know the browser in which the panel is displayed will be the most recent?  Could it be the case that the panel is shown in some other browser window that happens to be open?  Our tests make me weep.  Why does this one need a focused window but the others don't?  Or do they?  Should the harness automatically focus the most recent browser window before starting each test?

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #12)
> Looks great.  But how do you know the browser in which the panel is displayed
> will be the most recent?  Could it be the case that the panel is shown in some
> other browser window that happens to be open?

The Panel API automatically uses the most recent browser window if an anchor is not specified.


> Our tests make me weep.  Why
> does this one need a focused window but the others don't?  Or do they?  Should
> the harness automatically focus the most recent browser window before starting
> each test?

*sign* Indeed.  Such good questions.

https://github.com/mozilla/addon-sdk/commit/2d5d0b113476e111b51476eb69468c1e3cdacb54

https://github.com/mozilla/addon-sdk/commit/a617526a619c13296fbe168acca1fd97b4856c9a

(rebase/merge bungling)