Closed Bug 1113953 Opened 5 years ago Closed 5 years ago

Bug 1097754 Follow up - Centralizing the default Plivo timeout value

Categories

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: whsu, Assigned: jlorenzo)

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
gmealer
: review+
whsu
: review+
Bebe
: review+
Details | Review
Plivo timeout value is recorded in different test case.
We need to evaluate it to see if it worth to centralize the default timeout value.
1. Pros
2. Cons
3. How to...
No longer depends on: 1097754
Depends on: 1097754
Accident...
No longer depends on: 1097754
Assignee: nobody → jlorenzo
QA Whiteboard: [fxosqa-auto-s7][fxosqa-auto-points=1]
Bug 1097754 hasn't landed yet. Let's drop this current issue until the dependency is resolved.
QA Whiteboard: [fxosqa-auto-s7][fxosqa-auto-points=1] → [fxosqa-auto-dropped-s7][fxosqa-auto-points=1]
Attached file Gaia PR
Kicked off DSDS job: http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/558
Non-DSDS job: http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/557

The non DSDS job has 3 errors, which are likely due to bug 1097754 and bug 1118728
Attachment #8545272 - Flags: review?(whsu)
Attachment #8545272 - Flags: review?(gmealer)
Bug 1097754 is not about timeouts anymore.
QA Whiteboard: [fxosqa-auto-dropped-s7][fxosqa-auto-points=1] → [fxosqa-auto-s7][fxosqa-auto-points=1]
Comments in github. TL;DF: should use timeout= syntax instead of reverting to positional, and I optionally suggest wait_for_call_completed/connected() wrappers in PlivoUtil since it's obviously a common construct.
I agree with both of your points. I updated the PR. I left 2 new commits to make the second review easier (I'll squash them before the merge). The tests look way better that way.
Comment on attachment 8545272 [details] [review]
Gaia PR

Looks great!
Attachment #8545272 - Flags: review?(gmealer) → review+
Attachment #8545272 - Flags: review?(florin.strugariu)
Comment on attachment 8545272 [details] [review]
Gaia PR

Great!

The patch looks good to me!
Diff this change, PLIVO_TIMEOUT was used 25 times in original tests, Johan successfully centralizes PLIVO_TIMEOUT value and reduce many function calls.


The results also looks good to me.
@ DSDS: http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/570/
  - Result: 4 passed: 4 and 0 failed

@ Non-DSDS: http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/571/
  - 13 passed and 2 failed (The 2 fail case doesn't relate to the patch)

Thanks Johan! r+
Have a nice weekend! :)
Attachment #8545272 - Flags: review?(whsu) → review+
Attachment #8545272 - Flags: review?(florin.strugariu) → review+
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.