Permanent Orange: TEST-UNEXPECTED-FAIL | test-recent-menu.js | test_move_message, test_delete_message, test_archive_message

RESOLVED FIXED in Thunderbird 26.0

Status

Thunderbird
Folder and Message Lists
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: standard8, Assigned: aceman)

Tracking

({intermittent-failure})

Trunk
Thunderbird 26.0
All
Linux
intermittent-failure
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird24+ fixed)

Details

Attachments

(4 attachments, 4 obsolete attachments)

Regression from bug 876347:

Test Failure: a != b: '' != 'true'.
TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/mozmill/folder-display/test-recent-menu.js | test-recent-menu.js::test_move_message

(I suspect the other messages may be follow-on).
(Assignee)

Comment 1

4 years ago
Note that there is a patch in bug 876347 to address this failure on other platforms. But it was not enough for Linux.
Assignee: nobody → acelists
Whiteboard: [free to take if you have a patch idea]
(Assignee)

Comment 2

4 years ago
Created attachment 771018 [details] [diff] [review]
WIP patch

Aryx, could I have a try run with this patch, mozmill only? Thanks
Flags: needinfo?(archaeopteryx)
Pushed to Thunderbird-Try: https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=9416b0f056f4
Flags: needinfo?(archaeopteryx)
(Assignee)

Comment 4

4 years ago
Great, what gets the test to reliably pass locally does nothing for the try server :(
(Assignee)

Comment 5

4 years ago
Created attachment 771450 [details] [diff] [review]
patch v2 - keep menus open

While the patch does not seem to fix the problem, I'd still like to get some of the changes reviewed. Conceptually I do not like that we open and close the menu and only then look at its (generated) elements. I think that is not safe universally. Is there a guarantee that after closing of a menu the elements are still there? I am not sure. So I'd like to keep the menu open while we inspect its elements and close it after that. This patch allows that, while not changing the current behaviour for tests that do not want that. But I think there are more tests that would like this feature.
Attachment #771018 - Attachment is obsolete: true
Attachment #771450 - Flags: review?(mconley)
Attachment #771450 - Flags: review?(mbanner)
(Assignee)

Updated

4 years ago
Attachment #771450 - Attachment description: patch v2 → patch v2 - keep menus open
(Assignee)

Comment 6

4 years ago
Created attachment 771456 [details] [diff] [review]
patch - focus menus

This patch makes the test pass on my machine locally. I don't know why it does not help on try servers. It explicitly focuses the menuitems being clicked on. Is this good for anything? (To be applied on top of the first patch.)
Attachment #771456 - Flags: feedback?(mconley)
Attachment #771456 - Flags: feedback?(mbanner)
Comment hidden (Treeherder Robot)
Though this failure is permanent on build servers but I can not reproduce this failure on local.

aceman, cloud you please take a log with "NSPR_LOG_MODULES=Focus:5,WidgetFocus:5" on failure case if you can reproduce this failure? The log will be very helpful to solve the failure.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 46

4 years ago
Created attachment 776595 [details]
focus log

This is the log you requested with a clean tree. The test fails like this:

SUMMARY-UNEXPECTED-FAIL | test-recent-menu.js | test-recent-menu.js::test_move_message
  EXCEPTION: Popup never opened at action depth 0; id=mailContext-fileHereMenu, state=showing
    at: utils.js line 447
       TimeoutError utils.js 447
       waitFor utils.js 485
       _click_menus test-window-helpers.js 1004
       test_move_message test-recent-menu.js 40
       Runner.prototype.wrapper frame.js 585
       Runner.prototype._runTestModule frame.js 655
       Runner.prototype.runTestModule frame.js 701
       Runner.prototype.runTestFile frame.js 534
       runTestFile frame.js 713
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183
SUMMARY-UNEXPECTED-FAIL | test-recent-menu.js | test-recent-menu.js::test_delete_message
  EXCEPTION: Popup never opened! id=mailContext, state=showing
    at: utils.js line 447
       TimeoutError utils.js 447
       waitFor utils.js 485
       _click_menus test-window-helpers.js 965
       test_delete_message test-recent-menu.js 83
       Runner.prototype.wrapper frame.js 585
       Runner.prototype._runTestModule frame.js 655
       Runner.prototype.runTestModule frame.js 701
       Runner.prototype.runTestFile frame.js 534
       runTestFile frame.js 713
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183
SUMMARY-UNEXPECTED-FAIL | test-recent-menu.js | test-recent-menu.js::test_archive_message
  EXCEPTION: Popup never opened! id=mailContext, state=showing
    at: utils.js line 447
       TimeoutError utils.js 447
       waitFor utils.js 485
       _click_menus test-window-helpers.js 965
       test_archive_message test-recent-menu.js 97
       Runner.prototype.wrapper frame.js 585
       Runner.prototype._runTestModule frame.js 655
       Runner.prototype.runTestModule frame.js 701
       Runner.prototype.runTestFile frame.js 534
       runTestFile frame.js 713
       Bridge.prototype._execFunction server.js 179
       Bridge.prototype.execFunction server.js 183
SUMMARY-PASS | test-recent-menu.js::teardownModule
make: *** [mozmill-one] Error 1
Attachment #776595 - Flags: feedback?(hiikezoe)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
aceman, thanks for the log. But this log is slightly different what I was expecting...
What I was expecting a log that including mozmill's log like this:

TEST-PASS | /home/zoe/hg/comm-central/mail/test/mozmill/folder-display/test-recent-menu.js | test-recent-menu.js::setupModule
TEST-START | /home/zoe/hg/comm-central/mail/test/mozmill/folder-display/test-recent-menu.js | test_move_message
1354350464[19544b0]: Window 2a020a0 Hidden [Currently: 2769010 2769010]
1354350464[19544b0]:   Hide Window: chrome://messenger/content/msgAccountCentral.xul
1354350464[19544b0]:   Focused Window: chrome://messenger/content/messenger.xul
1354350464[19544b0]:   Active Window: chrome://messenger/content/messenger.xul
1354350464[19544b0]: Window 29e3380 Hidden [Currently: 2769010 2769010]

Anyway what I can tell from the log you post is some focus events did no seem to be handled before starting test_move_message.

From your log:

-1219864832[b722b0c0]: Window b72906b0 Shown [Currently: b2632e30 b2632e30]
-1219864832[b722b0c0]: Shown Window: mailbox:///var/SSD/TB-hg/tbird-bin/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/aaafolder1?number=0
-1219864832[b722b0c0]:  Focused Window: chrome://messenger/content/messenger.xul
-1219864832[b722b0c0]: <<SetFocus begin>>
-1219864832[b722b0c0]: <<SetFocus end>>
-1219864832[b722b0c0]: <<SetFocus begin>>
-1219864832[b722b0c0]: <<SetFocus end>>
-1219864832[b722b0c0]: <<SetFocus begin>>
-1219864832[b722b0c0]: <<SetFocus end>>
-1219864832[b722b0c0]: OnContainerFocusOutEvent [af97f7b0]
-1219864832[b722b0c0]: Window b2632e30 Lowered [Currently: b2632e30 b2632e30]
-1219864832[b722b0c0]:   Lowered Window: chrome://messenger/content/messenger.xul
-1219864832[b722b0c0]:   Active Window: chrome://messenger/content/messenger.xul
-1219864832[b722b0c0]: <<Blur begin>>
-1219864832[b722b0c0]: Element tree has been blurred
-1219864832[b722b0c0]: Done with container focus out [af97f7b0]
-1219864832[b722b0c0]: OnContainerFocusInEvent [af97f7b0]
-1219864832[b722b0c0]: Window b2632e30 Raised [Currently: 0 0]
-1219864832[b722b0c0]:   Raised Window: b2632e30 chrome://messenger/content/messenger.xul
-1219864832[b722b0c0]: <<Focus begin>>
-1219864832[b722b0c0]: Element tree has been focused
-1219864832[b722b0c0]:  from window
-1219864832[b722b0c0]:  [Newdoc: 1 FocusChanged: 0 Raised: 1 Flags: 0]
-1219864832[b722b0c0]:   SetFocus 0 [af97f7b0]
-1219864832[b722b0c0]:   widget now has focus in SetFocus() [af97f7b0]
-1219864832[b722b0c0]: Update Caret: 0 1
-1219864832[b722b0c0]: Events sent from focus in event [af97f7b0]
-1219864832[b722b0c0]: Window b72906b0 Hidden [Currently: b2632e30 b2632e30]
-1219864832[b722b0c0]:   Hide Window: mailbox:///var/SSD/TB-hg/tbird-bin/mozilla/_tests/mozmill/mozmillprofile/Mail/Local%20Folders/aaafolder1?number=0
-1219864832[b722b0c0]:   Focused Window: chrome://messenger/content/messenger.xul
-1219864832[b722b0c0]:   Active Window: chrome://messenger/content/messenger.xul

I suppose this is the part of test_move_message. "OnContainerFocusOutEvent" and "OnContainerFocusInEvent" should have been handled before the test. Those events prevents popup menu open.
aceman, I would like to ask you one more thing. Your log is *timeout* case. I would like to see *Test Failure: a != b: '' != 'true'* case.
Flags: needinfo?(acelists)
(Assignee)

Comment 55

4 years ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #53)
> aceman, thanks for the log. But this log is slightly different what I was
> expecting...
> What I was expecting a log that including mozmill's log like this:
Sorry, I was redirecting mozmill log to another file. I can produce it if needed.

> Anyway what I can tell from the log you post is some focus events did no >seem to be handled before starting test_move_message.
Maybe that is why my "patch - focus menus" helps me.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #54)
> aceman, I would like to ask you one more thing. Your log is *timeout* case.
> I would like to see *Test Failure: a != b: '' != 'true'* case.

I understand you see that on thunderbird-trunk. But I think I do not get it locally. For me it fails sooner. The menu is not properly opened and it fails there. On Tb-trunk it somehow does not notice the menu was not opened and then fails accessing the menu items. But it is the same menu so the cause may 
be the same.
Flags: needinfo?(acelists)
Created attachment 776967 [details] [diff] [review]
Possible fix

Somebody please send this patch to try server.
I suppose this patch will solve the failure case of "Test Failure: a != b: '' != 'true'".
(Assignee)

Comment 57

4 years ago
Comment on attachment 776967 [details] [diff] [review]
Possible fix

Aryx, could you please push this to try server, mozmill tests only.
Attachment #776967 - Flags: feedback?(archaeopteryx)
Pushed to Thunderbird-Try as https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=e9b7510a3bca
Comment hidden (Treeherder Robot)
(Assignee)

Comment 60

4 years ago
There seems to be no change with the patch in the try run.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
My assumption was wrong. I supposed _build method in folderWidgets.xml is not processed at the failure time.

The only other possibility I can imagine is :

the following .noSelect in folderWidgets.xml check does not properly work

 if (this._parentFolder.noSelect)
   menuitem.setAttribute("disabled", "true");

Is this really possible?
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #73)
> The only other possibility I can imagine is :
> 
> the following .noSelect in folderWidgets.xml check does not properly work
> 
>  if (this._parentFolder.noSelect)
>    menuitem.setAttribute("disabled", "true");

I meant
  if (!recentFolders.length)
    menu.setAttribute("disabled", "true");
(Assignee)

Comment 80

4 years ago
Bug 876347 triggered these problems, where the lines you say have been added. But as the failures only happen on Linux, it should be some focus/timing problem again. And in my local tests, the whole menu does not popup even before execution comes to the code you mention.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 85

4 years ago
Comment on attachment 771456 [details] [diff] [review]
patch - focus menus

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

This has bitrotted, but after fixing the bitrot it does seem to help for me!
Attachment #771456 - Flags: feedback+
Comment on attachment 771456 [details] [diff] [review]
patch - focus menus

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

This is fine, though it won't help if the OS is doing focus-stealing prevention, and something other than TB takes focus.

I recall that being an issue one time - during a test run, some OS dialog would pop up during the test run and take focus. The OS's focus-stealing code prevented TB from taking focus again programmatically (it had to be triggered via a user action). Maybe that's what we're experiencing here - focus-stealing protection.

If so, I wonder if there's a way to turn that off on our tinderbox machines?
Attachment #771456 - Flags: feedback?(mconley) → feedback+
Comment on attachment 771450 [details] [diff] [review]
patch v2 - keep menus open

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

::: mail/test/mozmill/shared-modules/test-window-helpers.js
@@ +919,5 @@
>       *     node has an attribute with that name and value.  We click whatever we
>       *     find.  We throw if we don't find what you were asking for.
> +     * @param aKeepOpen  If set to true the popups are not closed after last click.
> +     *
> +     * @return  An array of popup elements that still were left open.

We should add that this returns an empty array if aKeepOpen is not true.
Attachment #771450 - Flags: review?(mconley) → review+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 111

4 years ago
Created attachment 779950 [details] [diff] [review]
[checked in] patch - keep menus open v3
Attachment #771450 - Attachment is obsolete: true
Attachment #771450 - Flags: review?(mbanner)
Attachment #779950 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Whiteboard: [free to take if you have a patch idea] → [free to take if you have a patch idea] [check in only the marked patch]
(Assignee)

Updated

4 years ago
Blocks: 871266
(Assignee)

Comment 112

4 years ago
Comment on attachment 776967 [details] [diff] [review]
Possible fix

Looks like this didn't work.
Attachment #776967 - Attachment is obsolete: true
Attachment #776967 - Flags: feedback?(archaeopteryx)
Comment on attachment 779950 [details] [diff] [review]
[checked in] patch - keep menus open v3

https://hg.mozilla.org/comm-central/rev/6b2de324d78f
Attachment #779950 - Attachment description: patch - keep menus open v3 [ready for checkin] → [checked in] patch - keep menus open v3 [ready for checkin]
Keywords: checkin-needed
Whiteboard: [free to take if you have a patch idea] [check in only the marked patch] → [free to take if you have a patch idea]
Comment on attachment 771456 [details] [diff] [review]
patch - focus menus

I'm guessing this one is obsolete.
Attachment #771456 - Attachment is obsolete: true
Attachment #771456 - Flags: feedback?(mbanner)
(Assignee)

Comment 115

4 years ago
Comment on attachment 771456 [details] [diff] [review]
patch - focus menus

It is not obsolete it just does work 100%. It worked for me and mkmelin, but not on the try server. So either I scrape it or refine it.
Attachment #771456 - Attachment is obsolete: false
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
tracking-thunderbird24: --- → +
(Assignee)

Comment 131

4 years ago
Created attachment 781877 [details] [diff] [review]
[checked in] patch - focus menus v2

This still seems to help me (and Magnus) locally and it didn't seem to cause any problems on TB-try so it may be worth to include.
To be applied on top of bug 871266.
Attachment #771456 - Attachment is obsolete: true
Attachment #781877 - Flags: review?(mbanner)
Comment on attachment 781877 [details] [diff] [review]
[checked in] patch - focus menus v2

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

Ok, lets go for it. r=Standard8
Attachment #781877 - Flags: review?(mbanner) → review+
Comment on attachment 781877 [details] [diff] [review]
[checked in] patch - focus menus v2

https://hg.mozilla.org/comm-central/rev/9b3eb173c798
Attachment #781877 - Attachment description: patch - focus menus v2 → [checked in] patch - focus menus v2
Created attachment 784804 [details] [diff] [review]
Avoid another suspicion

I am now suspecting some other tests updated MRMTime property.
This patch resets MRMTime before running test-recent-menu.js.

Someone please push this patch to try server.
Thank you.
(Assignee)

Comment 135

4 years ago
Comment on attachment 784804 [details] [diff] [review]
Avoid another suspicion

But the patch
https://bug876347.bugzilla.mozilla.org/attachment.cgi?id=765535 did not change the logic of the number of recent menu items so if this would be a problem, it should have cropped up even before my change. But your change seems sound anyway.
Attachment #784804 - Attachment description: Avoid another suspicionss → Avoid another suspicion
Attachment #784804 - Flags: feedback+
(Assignee)

Comment 136

4 years ago
Aryx, we'd need a trybuild with the patch from comment 134. All systems but Mozmill only. Thanks.
Flags: needinfo?(archaeopteryx)
Opps, forgot to comment:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=2a1b8350ea9a
Flags: needinfo?(archaeopteryx)
Looking good so far for Linux 32 Mozmill. I've just triggered some respins of just the mozmill tests for that.

Linux 64 failed due to tree bustage, that should be fixed now, so I pushed afresh for that:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=64b81cec5d12
Yo ho! Thanks Mark! test-recent-menu.js passed on all respins now!
Comment on attachment 784804 [details] [diff] [review]
Avoid another suspicion

Linux 64 has passed with flying green as well :-)

Therefore I'm going to say r+ to this and get it landed. The only thing I'd say, is that it would be nice to have a comment in the file as to why we're doing this (just like the check-in comment, but quicker to find) - but I'll do that on checkin.
Attachment #784804 - Flags: review+
https://hg.mozilla.org/comm-central/rev/259c50bae39e

Thanks to both aceman & Hiro for working on the fix to this.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Attachment #779950 - Attachment description: [checked in] patch - keep menus open v3 [ready for checkin] → [checked in] patch - keep menus open v3
Yay! We did it!
http://hg.mozilla.org/releases/comm-aurora/rev/259c50bae39e
http://hg.mozilla.org/releases/comm-beta/rev/bb5e4362e4bf
status-thunderbird24: --- → fixed
Target Milestone: --- → Thunderbird 26.0
(Assignee)

Updated

4 years ago
Whiteboard: [free to take if you have a patch idea]
Attachment #776595 - Flags: feedback?(hiikezoe)
You need to log in before you can comment on or make changes to this bug.