Closed Bug 1258020 Opened 4 years ago Closed 4 years ago

Timeouts for bandwidth limiting tests should be set higher

Categories

(Testing Graveyard :: external-media-tests, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: sydpolk, Assigned: sydpolk)

References

Details

Attachments

(1 file)

The first load of a video under conditions take a long time, and we see a failure. The screen shows that youtube is buffering when this happens.

2016-03-18 16:10:13     INFO -  TEST-START | test_playback_limiting_bandwidth.py TestPlaybackLimitingBandwidth.test_playback_limiting_bandwidth_1000
2016-03-18 16:11:56    ERROR -  TEST-UNEXPECTED-ERROR | test_playback_limiting_bandwidth.py TestPlaybackLimitingBandwidth.test_playback_limiting_bandwidth_1000 | TimeoutException: TimeoutException: Timed out after 59.8 seconds with message: Check if video current_time > 0
2016-03-18 16:11:56     INFO -  condition: <lambda>
2016-03-18 16:11:56     INFO -  VideoPuppeteer - test url: https://youtu.be/AbAACm1IQE0: {
2016-03-18 16:11:56     INFO -  	(video)
2016-03-18 16:11:56     INFO -  	current_time: 0,
2016-03-18 16:11:56     INFO -  	duration: 12.581,
2016-03-18 16:11:56     INFO -  	expected_duration: 0,
2016-03-18 16:11:56     INFO -  	lag: 2.7936511484e-06,
2016-03-18 16:11:56     INFO -  	url: https://www.youtube.com/watch?v=AbAACm1IQE0&feature=youtu.be
2016-03-18 16:11:56     INFO -  	src: mediasource:https://www.youtube.com/b16ad065-c6ed-4995-b443-102ad0055e11
2016-03-18 16:11:56     INFO -  	frames total: 15
2016-03-18 16:11:56     INFO -  	 - dropped: 0
2016-03-18 16:11:56     INFO -  	 - corrupted: 0
2016-03-18 16:11:56     INFO -  }
2016-03-18 16:11:56     INFO -  Traceback (most recent call last):
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\venv\lib\site-packages\marionette\marionette_test.py", line 351, in run
2016-03-18 16:11:56     INFO -      testMethod()
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\tests\external-media-tests\external_media_tests\playback\test_playback_limiting_bandwidth.py", line 23, in test_playback_limiting_bandwidth_1000
2016-03-18 16:11:56     INFO -      self.run_videos()
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\venv\lib\site-packages\external_media_harness\testcase.py", line 99, in run_videos
2016-03-18 16:11:56     INFO -      set_duration=60)
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\venv\lib\site-packages\external_media_tests\media_utils\video_puppeteer.py", line 93, in __init__
2016-03-18 16:11:56     INFO -      "Check if video current_time > 0")
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\venv\lib\site-packages\external_media_tests\utils.py", line 39, in verbose_until
2016-03-18 16:11:56     INFO -      return wait.until(condition, message=err_message)
2016-03-18 16:11:56     INFO -    File "c:\jenkins\workspace\mn-mse-bw-limits-youtube-default-nightly-win_32_64\build\venv\lib\site-packages\marionette_driver\wait.py", line 143, in until
2016-03-18 16:11:56     INFO -      cause=last_exc)
2016-03-18 16:11:56     INFO -  TEST-INFO took 102819ms
Duplicate of this bug: 1255615
Comment on attachment 8734414 [details]
MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix takes a LONG time to load when bandwidth is low. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42225/diff/1-2/
Attachment #8734414 - Attachment description: MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix → MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix takes a LONG time to load when bandwidth is low. r?SingingTree, r?maja_zf
Attachment #8734414 - Flags: review?(mjzffr)
Attachment #8734414 - Flags: review?(bvandyk)
Comment on attachment 8734414 [details]
MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix takes a LONG time to load when bandwidth is low. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42225/#review39451

Looks good to me. Is there a resonon why we use 'content' and 'chrome' strings rather than the synbolic constants CONTEXT_CONTENT and CONTEXT_CHROME?
Attachment #8734414 - Flags: review?(bvandyk) → review+
https://reviewboard.mozilla.org/r/42225/#review39451

That idoim is all over marionette in the code base. If we decided to use constants, the right thing to do would be to add them to marionette, release new versions of marionette to pypi, convert all of the code to use them (including new imports), submit all of that code, and release those new packages to pypi... Defining them locally makes no sense.

There are three reasons to make constants out of hard-coded strings and values.
1. Improve readability. However, it is obvious that you are passing a string type to marionette.using_context this way. I am not sure that constants would improve that.
2. Guard against the values changing - That's not going to happen.
3. Save space typing (PI = 3.1415926....). Those constants won't do that.

Let's just be consistent with what marinoette already does.
Comment on attachment 8734414 [details]
MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix takes a LONG time to load when bandwidth is low. r?SingingTree, r?maja_zf

https://reviewboard.mozilla.org/r/42225/#review39523
Attachment #8734414 - Flags: review?(mjzffr) → review+
(In reply to Syd Polk :sydpolk from comment #5)
> https://reviewboard.mozilla.org/r/42225/#review39451
> 
> That idoim is all over marionette in the code base. 
> ...
> Let's just be consistent with what marinoette already does.

I believe :SingingTree was referring to constants that are already provided in the Marionette Python client (example of usage https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/runner/base.py#602). However, I suggest you file a separate media-tests bug to convert all the hard-coded strings to Marionette constants; it could be a good first bug for new contributors. 

(The constants are already used in many areas of the marionette codebase; if we find any hard-coded strings in marionette-client, they should be replaced.)
https://reviewboard.mozilla.org/r/42225/#review39451

Unless I'm mistaken, such constants already exist with the names noted above. Internal to marionette both the constants and literals look to used. The docs discuss usage in terms of the constants, which is what made me ask the initial question.
Whoops, didn't see the comment here before replying on review board. The above sounds good. For clarity, I wasn't suggesting we alter the literal/constant usage as part of this tick, was looking to understand the usage.
https://reviewboard.mozilla.org/r/42225/#review39451

Yes, I see that now. There are also many many references to 'chrome' in marionette, firefox-ui, and puppeteer. Maja suggests a separate bug for this as a good first bug, and I agree.
Assignee: nobody → spolk
Comment on attachment 8734414 [details]
MozReview Request: Bug 1258020 - Increase timeouts for network bandwidth limiting tests. Netflix takes a LONG time to load when bandwidth is low. r?SingingTree, r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42225/diff/2-3/
https://hg.mozilla.org/mozilla-central/rev/b2d35c87d6cd
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.