863456 - Report failures for tests that leave unexpected tabs or windows behind

Status: RESOLVED FIXED

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

Description

[Dave Townsend [:mossop]]

Some of our tests look likely due to earlier tests not waiting for tabs or windows to close before continuing. This patch adds a check after every test function that verifies there is only one window open containing one tab. If not it logs everything that is open.

I found about 30 tests that fail this so there are fixes for those cases. In addition I made getTabs ignore tabs that are in the process of closing. This is necessary because when the callback for tab.close() is called the tab still exists in the XUL since the tabbrowser TabClose event is dispatched before removal so listeners can inspect the tab element.

Comment 1

[Dave Townsend [:mossop]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/953

Pointer to Github pull-request

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 739285 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/953

As you are used to say: Ship it!
See pull request comment, but I think that we should really tune tabbrowser to avoid other test race conditions in a followup.

Comment 3

[Alexandre Poirot [:ochameau]]

Oh and btw, have you tested this patch on Fennec?

Comment 4

[Dave Townsend [:mossop]]

Ah right, looks like addon-page leaves something open permanently on Fennec, also bug 863502 :(

Comment 7

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/d9d01d18a5492b2bcbfb145236b0bd77d4232db5
Bug 863456: Report failures for tests that leave unexpected tabs or windows behind. r=ochameau

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/a4c75202281d4c716439d15ba38cc78f56f73e24
Revert "Bug 863456: Report failures for tests that leave unexpected tabs or windows behind. r=ochameau"

This reverts commit d9d01d18a5492b2bcbfb145236b0bd77d4232db5.

Comment 9

[Dave Townsend [:mossop]]

This introduced a random orange on linux: https://tbpl.mozilla.org/php/getParsedLog.php?id=25741115&tree=Jetpack

Comment 10

[Erik Vold [:erikvold] (please needinfo? me)]

Can we land this again and try to deal with the orange?

Comment 11

[Dave Townsend [:mossop]]

We need to fix the orange before we can land again, unfortunately I was foolish and didn't save the log so I've no idea what it was :(

I don't know when I'll have time to play with this, anyone else is welcome to pick it up

Comment 12

[Dave Townsend [:mossop]]

Attachment: updated patch

This fixes the problems with the last patch and had some 5 green runs on try.

Comment 13

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8409010 [details] [review]
updated patch

I just made one comment that I think it's worth addressing, but I would rather land this and do it as a separate patch.

Comment 14

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

This is really exciting, sorry to take so long to review this.

Comment 15

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/934c61db28e2a395c90a8dd82ceb3a3530d00e17
Bug 863456: Report failures for tests that leave unexpected tabs or windows behind.

https://github.com/mozilla/addon-sdk/commit/400bd5cbacaa0f79f7b0ef4c8400c0945ee04844
Merge pull request #1466 from Mossop/bug863456

Bug 863456: Report failures for tests that leave unexpected tabs or windows behind. r=gozala

Comment 16

[Dave Townsend [:mossop]]

I filed bug 1004514 for the follow-up