If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Intermittent test-attachment-reminder.js | test-attachment-reminder.js::test_manual_automatic_attachment_reminder_interaction

RESOLVED FIXED in Thunderbird 32.0

Status

Thunderbird
Testing Infrastructure
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: RyanVM, Assigned: aceman)

Tracking

({intermittent-failure})

unspecified
Thunderbird 32.0
x86
Linux
intermittent-failure
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

1.14 KB, patch
Magnus Melin
: review+
Details | Diff | Splinter Review
15.51 KB, patch
aceman
: review+
Details | Diff | Splinter Review
(Reporter)

Description

4 years ago
We saw intermittent failures in test-attachment-reminder.js after bug 939700 landed last time, so tentatively calling this a regression from that.

https://tbpl.mozilla.org/php/getParsedLog.php?id=33629621&tree=Thunderbird-Trunk

Ubuntu VM 12.04 comm-central opt test mozmill on 2014-01-27 06:31:15 PST for push eff977c91041
slave: tst-linux32-ec2-070

TEST-START | /builds/slave/test/build/mozmill/composition/test-attachment-reminder.js | test_manual_automatic_attachment_reminder_interaction
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "controller.waitFor()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "controller.click()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.keypress()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Step Pass: {"function": "Controller.type()"}
Test Failure: Expected the notification with value attachmentReminder to be shown
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/mozmill/composition/test-attachment-reminder.js | test-attachment-reminder.js::test_manual_automatic_attachment_reminder_interaction
Comment hidden (Treeherder Robot)
(Assignee)

Comment 2

4 years ago
OK, so far the failures have only been on linux 32 bit and always in the same test. I'll look at it.
Assignee: nobody → acelists
(Assignee)

Comment 3

4 years ago
Created attachment 8366735 [details] [diff] [review]
patch

Considering there this does fail, we could try to wait longer for the notification. Can the machine be overloaded so that the 0.5 s timeout is delayed a bit?
Attachment #8366735 - Flags: review?(mkmelin+mozilla)
Comment hidden (Treeherder Robot)

Comment 5

4 years ago
Comment on attachment 8366735 [details] [diff] [review]
patch

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

Lets give it a shot
Attachment #8366735 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 6

4 years ago
Thanks, let's see.
Status: NEW → ASSIGNED
Keywords: checkin-needed
(Reporter)

Comment 7

4 years ago
https://hg.mozilla.org/comm-central/rev/c3d4df847b66
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 30.0
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 13

3 years ago
reopening since this is happening again

https://tbpl.mozilla.org/php/getParsedLog.php?id=39218463&tree=Thunderbird-Trunk&full=1

SUMMARY-UNEXPECTED-FAIL | test-attachment-reminder.js | test-attachment-reminder.js::test_manual_automatic_attachment_reminder_interaction
  EXCEPTION: Expected the notification with value attachmentReminder to be shown
    at: test-notificationbox-helpers.js line 43
       assert_notification_displayed test-notificationbox-helpers.js:43 1
       assert_automatic_reminder_state test-attachment-reminder.js:46 3
       test_manual_automatic_attachment_reminder_interaction test-attachment-reminder.js:322 3
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestDirectory frame.js:525 7
       runTestDirectory frame.js:707 3
       Bridge.prototype._execFunction server.js:179 3
       Bridge.prototype.execFunction server.js:183 9
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 17

3 years ago
This test is dependent on timing. It waits some seconds for the reminder to appear/reappear. That is needed because the TB code itself shows it on a timer.

If the host running the test is overloaded, the tested TB may not get to run the needed code before the test timer kicks in. I don't know how the test infrastructure works, whether the host is dedicated solely to run the tests or whether there may be other jobs running on the same machine while the tests run.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 21

3 years ago
Do you guys know the answer to comment 17 about the test servers?
Flags: needinfo?(standard8)
Flags: needinfo?(Pidgeot18)
I believe that they are generally AWS box instances. The resources on a given box are only in use by the one set of tests, but of course, I don't know how other boxes on the same platform interact.

If you want more info, try asking in #releng or irc.
Flags: needinfo?(standard8)
Flags: needinfo?(Pidgeot18)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 25

3 years ago
Created attachment 8435245 [details] [diff] [review]
new rework

So this does extend the time we wait for the notification. But not a hardcoded sleep time as it was, but an incremental sleep for short intervals. If the notification changes state as we need sooner, we can proceed in the test.
So it should not prolong the test execution even when the new total intervals are up to 30 seconds.

Aryx, please push this to try. Mozmill tests are enough, all systems.
Attachment #8435245 - Flags: feedback?(archaeopteryx)
Pushed to Thunderbird-Try as https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=c970703e9278
(Assignee)

Comment 27

3 years ago
Comment on attachment 8435245 [details] [diff] [review]
new rework

Thanks. At least it doesn't regress :)
Attachment #8435245 - Flags: feedback?(archaeopteryx) → review?(mkmelin+mozilla)

Comment 28

3 years ago
Comment on attachment 8435245 [details] [diff] [review]
new rework

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

::: mail/test/mozmill/composition/test-attachment-reminder.js
@@ +58,5 @@
> +  } else if (check_notification_displayed(aCwc, kBoxId, kNotificationId)) {
> +    // This waits up to 30 seconds for the notification to disappear.
> +    wait_for_notification_to_stop(aCwc, kBoxId, kNotificationId);
> +  } else {
> +    // This waits up to 10 seconds for which the notification must not appear.

s/for which/during which/

@@ +63,5 @@
> +    let sleepStep = 300;
> +    for (let i = 0; i <= 10000; i = i + sleepStep) {
> +      aCwc.sleep(sleepStep);
> +      assert_automatic_reminder_state(aCwc, false);
> +    }

10 s is a long time to always be waiting. how about half - 5s is plenty too.
and is it worth checking in the middle. fast failure yes, but failures should be rare

::: mail/test/mozmill/shared-modules/test-notificationbox-helpers.js
@@ +22,5 @@
> + * @param aController    the controller of the window to check
> + * @param aBoxId         the id of the notification box
> + * @param aValue         the value of the notification to look for
> + * @param aNotification  an optional out parameter: object that will pass the
> + *                       notification element out of this function in it's

its
Attachment #8435245 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 29

3 years ago
Created attachment 8437131 [details] [diff] [review]
new rework v1.1

Ok, thanks.
Attachment #8435245 - Attachment is obsolete: true
Attachment #8437131 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Reporter)

Comment 30

3 years ago
https://hg.mozilla.org/comm-central/rev/1ac4de6dc081
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: Thunderbird 30.0 → Thunderbird 32.0
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to TBPL Robot from comment #33)
> Standard8
> https://tbpl.mozilla.org/php/getParsedLog.php?id=43603466&tree=Thunderbird-
> Trunk
> TB Rev5 MacOSX Mountain Lion 10.8 comm-central debug test mozmill on
> 2014-07-11 04:14:11
> revision: 9e8809412458
> slave: talos-mtnlion-r5-091
> 
> TEST-UNEXPECTED-FAIL |
> /builds/slave/talos-slave/test/build/mozmill/composition/test-attachment-
> reminder.js |
> test-attachment-reminder.js::
> test_manual_automatic_attachment_reminder_interaction
> TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during
> directory run

Looks like this is still happening, or has moved. Aceman, can you take a look and either move to a new bug if appropriate, or just comment here.

Thanks.
Status: RESOLVED → REOPENED
Flags: needinfo?(acelists)
Resolution: FIXED → ---
I was wrong, the changes weren't this bug, they were bug 938829 causing a failure in the tests.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Flags: needinfo?(acelists)
You need to log in before you can comment on or make changes to this bug.