[b2g][emulator] explicitly handle the timeout issue for permission request test cases

RESOLVED WONTFIX

Status

Firefox OS
General
RESOLVED WONTFIX
4 years ago
4 years ago

People

(Reporter: schien, Assigned: schien)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Explicitly catch timeout issue in permission request test cases for investigating Bug 984274 and Bug 1019572.
Created attachment 8440398 [details] [diff] [review]
handle-timeout-permission-request.patch

catch the timeout issue in test case instead of waiting for test harness timeout.

https://tbpl.mozilla.org/?tree=Try&rev=7bd8d3ef191f
Attachment #8440398 - Flags: review?(amarchesini)
Comment on attachment 8440398 [details] [diff] [review]
handle-timeout-permission-request.patch

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

I'm a bit worried about b2g and android servers. Sometimes they are slow and using timeouts can be a problem.
Can you put the '5000' in a test preference?
Attachment #8440398 - Flags: review?(amarchesini) → review+
Created attachment 8441758 [details] [diff] [review]
pref-timeout-value.patch

@baku, is this what you expected for moving 5000 to test preference?
Attachment #8441758 - Flags: feedback?(amarchesini)
Attachment #8441758 - Flags: feedback?(amarchesini) → feedback+
Created attachment 8442131 [details] [diff] [review]
handle-timeout-permission-request.patch

update patch according to review comment, carry r+.

try result: https://tbpl.mozilla.org/?tree=Try&rev=3b1305537c38
Attachment #8440398 - Attachment is obsolete: true
Attachment #8441758 - Attachment is obsolete: true
Attachment #8442131 - Flags: review+
Keywords: checkin-needed
schien, on try it seems we have a couple of orange issue. Can you take a look?
Flags: needinfo?(schien)
Keywords: checkin-needed
@baku, this patch is for providing more debug information on TBPL and for making test case fail faster than 330s timeout. In bug 984274 comment #437 @ayang found webapp updater resets the permission table during the test. If we can land bug 984274 soon then there is no need to land the patch in this bug. @baku, How do you think?
Flags: needinfo?(schien) → needinfo?(amarchesini)
Hmm...looks like we are not landing bug 984274 that soon. I'd like to land this patch for providing more debug information on these test cases. @baku and @Ryan, how do you think?
Flags: needinfo?(ryanvm)
My recollection from the Try push was that it didn't make much difference in how the results were being reported. Anyway, I'm on PTO today, so I'll defer to Andrea.
Flags: needinfo?(ryanvm)
This patch is mainly for Bug 984274. I'm closing this bug as WONTFIX because we already land the fix for Bug 984274.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(amarchesini)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.