Closed Bug 1306630 Opened 3 years ago Closed 3 years ago

Port bug 1206133 to Thunderbird - TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/message-header/test-message-header.js


(Thunderbird :: General, defect)

Not set


(Not tracked)

Thunderbird 52.0


(Reporter: jorgk, Assigned: jorgk)


(Keywords: intermittent-failure)


(1 file, 2 obsolete files)

Looks like bug 1206133 changed popup states here:

This causes Mozmill failures here:

INFO -  SUMMARY-UNEXPECTED-FAIL | test-message-header.js | test-message-header.js::test_address_book_switch_disabled_on_contact_in_mailing_list
INFO -    EXCEPTION: a != b: 'showing' != 'open'.
INFO -      at: test-folder-display-helpers.js line 2858
INFO -         assert_true test-folder-display-helpers.js:2858 11
INFO -         assert_equals test-folder-display-helpers.js:2845 3
INFO -         test_address_book_switch_disabled_on_contact_in_mailing_list test-message-header.js:464 3
Summary: Port bug 1206133 to Thunderbird → Port bug 1206133 to Thunderbird - TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/message-header/test-message-header.js
Attached patch 1306630.patch (obsolete) — Splinter Review
OK, what happens is that instead of "open" we now get "showing".

In total there are four states: "closed", "open", "showing" and "hidden".

Let's be a little more flexible and cater for "open" and "showing" as being open.

With this patch the test passes on current trunk.
Attachment #8796608 - Flags: review?(acelists)
Where is the "positioning" state? Or is that just an event?

Also, why should we consider "showing" as being open, in this particular test only?
We often get popups stuck in "showing" state on Linux for infinite length of time and the test fails. But those problems are intermittent so we attribute them to either mozmill, or some Linux bug in Gtk/XUL backend or something.

So why is this test special?
Kirk, bug 1206133 broke one of our test. We now get the panel state returned as "showing" and not as "open" as it used to be. Most likely this is due to:
Is this expected?
Flags: needinfo?(ksteuber)
Bug 1206133 changed panel behavior. Panels used to open synchronously; now they open asynchronously. This means that tests that do things like this: |; assert(panel.state == "open");| now fail. The fix for this is typically to wait for the popupshown event.
Flags: needinfo?(ksteuber)
Attached patch 1306630-take2.patch (obsolete) — Splinter Review
Attachment #8796608 - Attachment is obsolete: true
Attachment #8796608 - Flags: review?(acelists)
Attachment #8796679 - Flags: review?(acelists)
Comment on attachment 8796679 [details] [diff] [review]

Review of attachment 8796679 [details] [diff] [review]:

Yes, this seems more correct. Please remove the now redundant assert_equals()s after the waitFor()s (3 occurrences in the patch).
Attachment #8796679 - Flags: review?(acelists) → review+
Addressed review comments and removed those three lines.
Attachment #8796679 - Attachment is obsolete: true
Attachment #8796751 - Flags: review+
(pushed with DONTBUILD since we're waiting for bug 456053 to fix its test failures.)
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
You need to log in before you can comment on or make changes to this bug.