1230832 - TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/session-store/test-session-store.js | test-session-store.js::test_message_pane_height_persistence

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[aleth [:aleth]]

SUMMARY-UNEXPECTED-FAIL | test-session-store.js | test-session-store.js::test_message_pane_height_persistence
  EXCEPTION: The message pane height should be 232, but is actually 364. The oldHeight was: 364: '232' != '364'.
    at: test-folder-display-helpers.js line 2849
       assert_true test-folder-display-helpers.js:2849 11
       assert_equals test-folder-display-helpers.js:2836 3
       test_message_pane_height_persistence test-session-store.js:228 1
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestFile frame.js:534 3
       runTestFile frame.js:713 3
       Bridge.prototype._execFunction server.js:179 10
       Bridge.prototype.execFunction server.js:183 16
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3

Previously this was fixed in bug 1154747. Looks like it didn't do it robustly.

Comment 1

[aleth [:aleth]]

Hmm, the mismatch in bug 1154747 was 7px:
EXCEPTION: The message pane height should be 226, but is actually 233. The oldHeight was: 352: '226' != '233'.

Here it's 132.

Comment 2

[aleth [:aleth]]

The calendar notification bar is clearly visible when running the test locally, so it seems the patch from bug 1154747 doesn't work very well on OSX.

Comment 3

[aleth [:aleth]]

Paenglab, this is a regression from bug 509359 (backing it out fixes the test). Could you take care of updating the test accordingly?

Comment 4

[aleth [:aleth]]

(In reply to aleth [:aleth] from comment #3)
> Paenglab, this is a regression from bug 509359 (backing it out fixes the
> test). Could you take care of updating the test accordingly?

(The failing test moves a splitter around.)

Comment 5

[Richard Marti (:Paenglab)]

I can try with partial backouts of my code and run this on try server to see what of my code breaks the test.

Comment 6

[aleth [:aleth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c05db52a727e

Comment 7

[aleth [:aleth]]

Attachment: Be smarter about moving splitters in test-sessionstore.js

Comment 8

[Richard Marti (:Paenglab)]

Aleth, thank you for finding the solution.

I tried when this failure is triggered and this looks strange:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1d54a6c6e4f1 changes only the #folderpane_splitter which should not affect the test (the test tests the #threadpane-splitter).

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f5b238e39337 changes only the #threadpane-splitter in vertical view and then the test is working.

Comment 9

[aleth [:aleth]]

(In reply to Richard Marti (:Paenglab) from comment #8)
> Aleth, thank you for finding the solution.
> 
> I tried when this failure is triggered and this looks strange:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=1d54a6c6e4f1 changes only the #folderpane_splitter which
> should not affect the test (the test tests the #threadpane-splitter).
> 
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=f5b238e39337 changes only the #threadpane-splitter in
> vertical view and then the test is working.

I think due to the extra splitter width on OSX, _move_splitter was hitting the wrong splitter because it wasn't actually grabbing it in the middle as claimed.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 8696383 [details] [diff] [review]
Be smarter about moving splitters in test-sessionstore.js

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

LGTM, r=mkmelin

Comment 11

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/72da0473732f208c5a775fa8d298b4ab50313a5f
Bug 1230832 - Be smarter about moving splitters in test-sessionstore.js. r=mkmelin