Closed Bug 964025 Opened 10 years ago Closed 8 years ago

TEST-UNEXPECTED-FAIL | test-message-header.js | test-message-header.js::test_toolbar_collapse_and_expand

Categories

(Thunderbird :: General, defect)

Product:
Thunderbird
Component:
General
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 31.0

People

(Reporter: mkmelin, Assigned: Fallen)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(2 files, 1 obsolete file)

[checked in] Intermediate Fix - v2
5.69 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
[checked in] (Un)collapse panes in setup/teardownModule instead
1.62 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review 
SUMMARY-UNEXPECTED-FAIL | test-message-header.js | test-message-header.js::test_toolbar_collapse_and_expand
  EXCEPTION: The header box should have been made smaller!
    at: test-message-header.js line 737
       test_toolbar_collapse_and_expand test-message-header.js 737
       Runner.prototype.wrapper frame.js 585
       Runner.prototype._runTestModule frame.js 655
       Runner.prototype.runTestModule frame.js 701
       Runner.prototype.runTestDirectory frame.js 525
       runTestDirectory frame.js 707
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183

https://tbpl.mozilla.org/php/getParsedLog.php?id=33540757&tree=Thunderbird-Trunk&full=1
All occurrences on linux.
This is on Windows & Linux (Mac has accessibility disabled due to the same failure but that was present from the start of accessibility being enabled on Mac - see bug 862238).

The regression here is from bug 963105 - I've verified it using try server fixed to before & after revisions.

Potentially useful links:

http://hg.mozilla.org/comm-central/annotate/2f2fa58f41a7/mail/test/mozmill/message-header/test-message-header.js#l981

http://hg.mozilla.org/comm-central/annotate/2f2fa58f41a7/mail/test/mozmill/message-header/test-message-header.js#l892

http://hg.mozilla.org/comm-central/annotate/2f2fa58f41a7/mail/base/content/msgHdrViewOverlay.xul#l411
Blocks: 963105
Keywords: regression
OS: Linux → All
Target Milestone: --- → Thunderbird 29.0
Please ignore comment 10 through 12 - I was looking at the wrong bug #. I've split comment 10 out to bug 967432.
No longer blocks: 963105
OS: All → Linux
I get this also with Lightning enabled. A little analysis shows that the way the test is written it depends on the window having the exact width needed so that the header wraps as expected. It also depends on various other things, like the length of some of the header fields.

It becomes even more obvious when Lightning is enabled in tests, the today pane messes with the widths even more. I have an intermediate patch that disables the folder pane and any extension panes in the tabmail-container, but I'll see if I can fix this while I am here.

It would probably be worth adding a screenshot the moment the test fails to see how wrapping works. Are such screenshots automatically saved, or do I have to add a dump statment with a data url of a screenshot and grab it from the log?
Blocks: 680203
Status: NEW → ASSIGNED
QA Contact: philipp
Some tests do require a certain size of window - this is why the unit test builders also have a set screen resolution.

You can get a detailed view of failures from tbpl by using asuth's processor:

http://clicky.visophyte.org/tools/arbpl-mozmill-standalone/?log=

just append the link to the gzipped log file and it'll process it and give you screenshots and other info.
Assignee: nobody → philipp
QA Contact: philipp
Attached patch Intermediate Fix - v1 (obsolete) — — Splinter Review 
The TBPL links have expired, so I suggest we push this intermediate fix first and find out if it still happens afterwards. It collapses all panes, with the result that the message header takes the full width of the window. This might make the calculations more accurate.
Attachment #8395725 - Flags: review?(standard8)
Comment on attachment 8395725 [details] [diff] [review]
Intermediate Fix - v1

Magnus, I'll also take your review on this if its ok.
Attachment #8395725 - Flags: review?(standard8) → review?(mkmelin+mozilla)
Comment on attachment 8395725 [details] [diff] [review]
Intermediate Fix - v1

Review of attachment 8395725 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know if it will help, but lets give it a shot.

::: mail/test/mozmill/quick-filter-bar/test-display-issues.js
@@ +26,5 @@
>    wh.installInto(module);
>    let qfb = collector.getModule('quick-filter-bar-helper');
>    qfb.installInto(module);
> +  let dh = collector.getModule('dom-helpers');
> +  dh.installInto(module);

we tend to just do it like this nowadays:

collector.getModule("dom-helpers").installInto(module);

::: mail/test/mozmill/shared-modules/test-dom-helpers.js
@@ +182,5 @@
> + * @param aElement              The splitter or container
> + * @param aShouldBeCollapsed    If true, collapse the pane
> + */
> +function collapse_panes(aElement, aShouldBeCollapsed) {
> +    let state = aShouldBeCollapsed ? "collapsed" : "open";

2 space indention please (whole function)
Attachment #8395725 - Flags: review?(mkmelin+mozilla) → review+
(In reply to Magnus Melin from comment #19)
> 
> we tend to just do it like this nowadays:
> 
> collector.getModule("dom-helpers").installInto(module);
Done, also changed it for the other modules to ensure consistency.

> 2 space indention please (whole function)
Oops, sorry about that. Fixed that too.
Attachment #8395725 - Attachment is obsolete: true
Attachment #8402277 - Flags: review+
Not sure what the standard procedure is here, either leave this one open until we are certain it doesn't occur anymore, or close it and wait for a new failure?
Keywords: checkin-needed
I'd leave it open for a while and see how it goes, especially as this was bisected data that is was a regression from bug 963105.
Blocks: 963105
Whiteboard: [leave open]
Target Milestone: Thunderbird 29.0 → Thunderbird 31.0
I saw a few more intermittent failures that might be vaguely related with Lightning enabled in
https://tbpl.mozilla.org/?tree=Thunderbird-Trunk&rev=f79b49f9149c


The failing tests are test_show_all_header_mode and test_more_widget_with_maxlines_of_3, which depend on the window width implicitly since they create a number of attendees and expect a certain line length. Simply rebuilding made things work so its also intermittent.

The failing build was on talos-r4-snow-007, the succeeding build was on talos-r4-snow-037. Maybe there is a difference in machine configuration too? I'm doing a few more rebuilds to try to figure this out.

I have a patch that moves hiding and showing the panes to setupModule/teardownModule, I don't yet know if this will improve the situation though. A try run succeeds, but I might need to do a few more to see if its really fixed.
Attachment #8402277 - Attachment description: Intermediate Fix - v2 → [checked in] Intermediate Fix - v2
I think we should hide the panes in all tests in this file, just to be sure. I've tested this on try multiple times by retriggering a job that had failed in another run. It succeeded 9 times without issues, the only orange is a different test.

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=9e4df7cf626e
Attachment #8413166 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8413166 [details] [diff] [review]
[checked in] (Un)collapse panes in setup/teardownModule instead

Review of attachment 8413166 [details] [diff] [review]:
-----------------------------------------------------------------

Lets try it!
Attachment #8413166 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed, leave-open
Whiteboard: [leave open]
https://hg.mozilla.org/comm-central/rev/c64774094c45

Please make sure you patch includes commit information when requesting checkin.
Keywords: checkin-needed
Attachment #8413166 - Attachment description: (Un)collapse panes in setup/teardownModule instead → [checked in] (Un)collapse panes in setup/teardownModule instead
Target Milestone: Thunderbird 31.0 → ---
I believe that the recent failures are a different issue, see bug 1039798.
Let's close this one.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 31.0
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: