Closed Bug 882867 Opened 11 years ago Closed 11 years ago

TEST-UNEXPECTED-FAIL | tests/test-tabs.testOpenInNewWindow, test-tabs.testOpenInNewWindowOnOpen | new window is active (({}) != ({}))

Categories

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

Product:
Add-on SDK Graveyard
Component:
General
Platform:
All
Linux
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: KWierso, Assigned: evold)

Details

(Keywords: intermittent-failure, Whiteboard: [test disabled on Linux][leave open])

Attachments

(3 files)

Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1041/files#diff-1
397 bytes, text/html
mossop
: review+
Details
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1051
358 bytes, text/html
mossop
: review+
Details
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1069
358 bytes, text/html
KWierso
: review+
Details 
https://tbpl.mozilla.org/php/getParsedLog.php?id=24107759&tree=Fx-Team#error0
https://tbpl.mozilla.org/php/getParsedLog.php?id=24114526&tree=Mozilla-Inbound#error0

TEST-UNEXPECTED-FAIL | tests/test-tabs.testOpenInNewWindow | new window is active (({}) != ({}))
TEST-INFO | Traceback (most recent call last):
  File "resource://gre/modules/commonjs/sdk/tabs/tab-firefox.js", line 98, in _onReady
    this._emit(EVENTS.ready.name, this._public);
  File "resource://gre/modules/commonjs/sdk/deprecated/events.js", line 123, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://gre/modules/commonjs/sdk/deprecated/events.js", line 153, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://extensions.modules.4d09848c-afa4-4968-b79f-9c604754b36a-at-jetpack.commonjs.path.tests/tabs/test-firefox-tabs.js", line 451, in exports.testOpenInNewWindow/</<.onReady
    test.assertEqual(activeWindow, newWindow, "new window is active");
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 189, in assertEqual
    this.fail(message);
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 84, in fail
    this.console.testMessage(false, false, this.test.name, message);
  File "resource://gre/modules/commonjs/sdk/test/harness.js", line 523, in testMessage
    this.trace();


And a possibly related failure: 
https://tbpl.mozilla.org/php/getParsedLog.php?id=24107759&tree=Fx-Team#error1

TEST-UNEXPECTED-FAIL | tests/test-tabs.testOpenInNewWindow | URL of activeTab matches ("about:blank" != "data:text/html;charset=utf-8,newwindow")
TEST-INFO | Traceback (most recent call last):
  File "resource://gre/modules/commonjs/sdk/tabs/tab-firefox.js", line 98, in _onReady
    this._emit(EVENTS.ready.name, this._public);
  File "resource://gre/modules/commonjs/sdk/deprecated/events.js", line 123, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://gre/modules/commonjs/sdk/deprecated/events.js", line 153, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://extensions.modules.4d09848c-afa4-4968-b79f-9c604754b36a-at-jetpack.commonjs.path.tests/tabs/test-firefox-tabs.js", line 454, in exports.testOpenInNewWindow/</<.onReady
    test.assertEqual(tabs.activeTab.url, url, "URL of activeTab matches");
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 189, in assertEqual
    this.fail(message);
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 84, in fail
    this.console.testMessage(false, false, this.test.name, message);
  File "resource://gre/modules/commonjs/sdk/test/harness.js", line 523, in testMessage
    this.trace();
Summary: TEST-UNEXPECTED-FAIL | tests/test-tabs.testOpenInNewWindow | new window is active (({}) != ({})) → TEST-UNEXPECTED-FAIL | tests/test-tabs.testOpenInNewWindow, test-tabs.testOpenInNewWindowOnOpen | new window is active (({}) != ({}))
Assignee: nobody → evold
Hi Erik, have you had a chance to look at this top orange yet? :-)
Flags: needinfo?(evold)
Wes, please can you disable this test until the intermittent failures are resolved (currently a top3 orange) - thank you :-)
Flags: needinfo?(kwierso)
(In reply to Ed Morley [:edmorley UTC+1] from comment #174)
> Hi Erik, have you had a chance to look at this top orange yet? :-)

I took a quick look a few days ago, not sure how best to fix this yet.. we should disable it for now I think.
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #215)
> I took a quick look a few days ago, not sure how best to fix this yet.. we
> should disable it for now I think.

Ok, thank you. Given that + the chat with Wes the other day, have disabled this test on Linux, by modifying downstream temporarily:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6adf3351d77d

Wes, can you upstream the test disable for me please?

Also, I know the preferred way of test disabling is to reset all exports at end of file (https://wiki.mozilla.org/Jetpack/Testing#Disabling_tests), however in this case we only really needed to disable two of the subtests. Would it be worth coming up with an approved pattern for disabling subtests (and adding to the wiki)?
Whiteboard: [test disabled on Linux][leave open]
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/729c145d37612b7cd98cd400957779a384280c66
Bug 882867 - Disable tests on Linux due to frequent intermittent test failures. r=edmorley
Ah ok I see the issue now.  The tab is opened correctly, but the new window is not yet focused when you do our activeTab tests.

This has been a recurring issue for us, I remember dealing with this issue first with Matteo on the private browsing selection tests.  Then on other tests intermittent bugs that I've patched I fixed by merely adding a hold for the window we expect to be focused, and continue when it actually focuses.

We could do that here and I expect the test would work again.. but this is the wrong approach I think, it feels like a platform race condition bug that we've uncovered, and we've been either bypassing it or gotten lucky.

I think I made a bug for this already, back at the last work week when we were doing the pb tests... let me see if I can find it.
Hmm we are making the assumption in mainly of our tests that a window will be focused when it's opened, or focused when it's first tab is ready, when it's really being focused later, and this may be a OS level operation that we can't affect unless the ready/load events are delayed until the window focuses..  So I can imagine that a platform fix is not possible, and we've written a lot of tests with an assumption that just isn't accurate..
Flags: needinfo?(evold)
Attachment #764452 - Flags: review?(dtownsend+bugmail)
Attachment #764452 - Flags: review?(dtownsend+bugmail) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/15ace24bccb2261670c2dafc19bb48afb423ab13
Bug 882867: fixing tab:testOpenInNewWindowOnOpen test and making various other tab tests more stable

https://github.com/mozilla/addon-sdk/commit/b9d5ce9152997dd40964279daf4fbe615eb8cfd7
Merge pull request #1041 from erikvold/882867

Bug 882867: fixing tab:testOpenInNewWindowOnOpen test r=@Mossop
This got backed out, so will need to be fixed and relanded.
Flags: needinfo?(kwierso)
Priority: -- → P1
Comment on attachment 767011 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1051

I forgot to upload a change to sdk/window/helpers.js in the last commit.. but this time I realized that opening new windows simply isn't necessary for most of these tab tests, I guess they were written to assume there was no window already open, which I don't think we should do.
Attachment #767011 - Flags: review?(dtownsend+bugmail)
Attachment #767011 - Flags: review?(dtownsend+bugmail) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/688780147c91234ea11a04a2f570b888bd1af803
Bug 882867: fixing tab:testOpenInNewWindowOnOpen test and making various other tab tests more stable

https://github.com/mozilla/addon-sdk/commit/d9484cf24cdf14f82f984da275eaf5e68f1a98d3
Merge pull request #1051 from erikvold/882867v2

Bug 882867: fixing tab:testOpenInNewWindowOnOpen and other tab tests r=@Mossop
Commits pushed to australis at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/688780147c91234ea11a04a2f570b888bd1af803
Bug 882867: fixing tab:testOpenInNewWindowOnOpen test and making various other tab tests more stable

https://github.com/mozilla/addon-sdk/commit/d9484cf24cdf14f82f984da275eaf5e68f1a98d3
Merge pull request #1051 from erikvold/882867v2
Attachment #769537 - Flags: review?(dtownsend+bugmail)
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/10012b0f158493f2f9c4bed756509f9ea0230820
Bug 882867 fixing tab:testOpenInNewWindow linux issue

https://github.com/mozilla/addon-sdk/commit/29e0d52ad4997ed1e7d75dc4c7512c70b38466a9
Merge pull request #1069 from erikvold/882867v2

Bug 882867 fixing tab:testOpenInNewWindow linux issue r=mossop
Comment on attachment 769537 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1069

Dave gave an r+ via IRC.
Attachment #769537 - Flags: review?(dtownsend+bugmail) → review+
Commit pushed to firefox24 at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b3ca2d2ac314ace033123574287a4b2a243858ae
Bug 882867 - Disable test-firefox-tabs.js on Linux for too many intermittent failures
Haven't seen this for a few months, closing.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: