Closed Bug 1127580 Opened 5 years ago Closed 4 years ago

TEST-UNEXPECTED-FAIL | /builds/slave/test/build/mozmill/content-policy/test-general-content-policy.js | test-general-content-policy.js::test_generalContentPolicy

Categories

(Thunderbird :: Testing Infrastructure, defect)

x86_64
Linux
defect
Not set

Tracking

(thunderbird43 fixed, thunderbird44 fixed)

RESOLVED FIXED
Thunderbird 44.0
Tracking Status
thunderbird43 --- fixed
thunderbird44 --- fixed

People

(Reporter: hiro, Assigned: alta88)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

SUMMARY-UNEXPECTED-FAIL | test-general-content-policy.js | test-general-content-policy.js::test_generalContentPolicy
EXCEPTION: Popup never opened! id=remoteContentOptions, state=showing
at: utils.js line 447
TimeoutError utils.js:447 13
waitFor utils.js:485 1
_click_menus test-window-helpers.js:961 1
allowRemoteContentAndCheck test-general-content-policy.js:213 1
test_generalContentPolicy test-general-content-policy.js:365 7
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 10
Bridge.prototype.execFunction server.js:183 16
Session.prototype.receive server.js:283 3
AsyncRead.prototype.onDataAvailable server.js:88 3
May this also be caused by bug 1193200, similar to bug 1197597?
Attached patch testfail.patchSplinter Review
I think this is really nothing more than timing on the notification box long transition css. This defaults the waitFor to 100ms for the popup, and passes locally; magnus could you 'try' it?

There will be failure later in checkAllowForSenderWithPerms and checkAllowForHostsWithPerms, for the permissions issue.
Attachment #8657479 - Flags: review?(mkmelin+mozilla)
See Also: → 1193200
What is the status on this one? Will this fix that specific testcase? I'd love to see green mozmill tests ;-) Magnus can you review?
Comment on attachment 8657479 [details] [diff] [review]
testfail.patch

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

This just changes the "are we done yet" interval from 50 to 100, not the timeout, so I kind of doubt it would change things.
Do you reproduce it locally without the patch applied? Please try it  a few times if so.
Well, what I observed was that 50 is not enough to get the 'open' state on the first try, whereas 100 always was.  So it's as though waitFor isn't working right, which doesn't make sense as it logs the message.  The timeout doesn't reproduce locally.  In the absence of a better theory, it can't hurt.

Anyway, the issue is that this bug is being used to star the permissions error, which should be a different bug (and pretty much expected).
Comment on attachment 8657479 [details] [diff] [review]
testfail.patch

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

I guess we could try it.
Attachment #8657479 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
I have pushed it to try to see if it helps: https://hg.mozilla.org/try-comm-central/rev/6cd9631b52ba
it is certainly not going to stop the error in nsIPermissions.add, but rather (potentially) the one in comment 0 from long ago.  this bug should be closed and a new one opened, with a log indicating the permission failure, and starred correctly from now on if it's actually that failure.  starring everything with content-policy to this bug is not correct.
(In reply to alta88 from comment #230)
> it is certainly not going to stop the error in nsIPermissions.add, but
> rather (potentially) the one in comment 0 from long ago.  this bug should be
> closed and a new one opened, with a log indicating the permission failure,
> and starred correctly from now on if it's actually that failure.  starring
> everything with content-policy to this bug is not correct.

Yes, the current failure doesn't match the description, so this bug shouldn't be used for the new failure.  

So, is this patch still checkin-needed? Or can this bug be closed WFM?
i think it should be checked in; it increases the initial timeout and uses that brand new beyond cool fat arrow style to boot.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/5f1a420df1af2a2261af4c0a83eb777445e8e054
Bug 1127580 - mozmill/content-policy/test-general-content-policy.js transition timing improvement. r=mkmelin a=aleth
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 44.0
Note: for bugs with titles like this (TEST_UNEXPECTED_FAIL...) it's better to use a different commit message which is 1) shorter and 2) says what the patch actually does.
Comment on attachment 8657479 [details] [diff] [review]
testfail.patch

[Triage Comment]

Let's try to get rid of these failures in aurora and beta as well.
Attachment #8657479 - Flags: approval-comm-beta+
Attachment #8657479 - Flags: approval-comm-aurora+
Assignee: nobody → alta88
You need to log in before you can comment on or make changes to this bug.