Closed Bug 1094144 Opened 5 years ago Closed 5 years ago

Add a test that ensures test_play_*_video has the correct audio channel opened

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

References

()

Details

Attachments

(1 file, 1 obsolete file)

46 bytes, text/x-github-pull-request
Bebe
: review+
gmealer
: review+
Details | Review
See bug 1086662, comment 3:
"
I don't think that testing the sound or video is easily done, however perhaps as an addition to test_play_*_video you could check that the correct audio channel is being opened (but that'd be a separate bug).
"

This could be done with the use of "current_audio_channel":
Example:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/dialer/test_dialer.py#28
QA Whiteboard: [fxosqa-auto-backlog+]
QA Whiteboard: [fxosqa-auto-backlog+] → [fxosqa-auto-s4]
Attachment #8522245 - Flags: review?(zcampbell)
Comment on attachment 8522245 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/26119

mostly r+ but jsut a small change will save a few lines of code and combine the logic better.
Attachment #8522245 - Flags: review?(zcampbell) → review-
Attached file audio_channel2
Thanks, that's much cleaner.
Tested locally on device and desktop.
Attachment #8522245 - Attachment is obsolete: true
Attachment #8526110 - Flags: review?(zcampbell)
Attachment #8526110 - Flags: review?(zcampbell)
Attachment #8526110 - Flags: review?(gmealer)
Attachment #8526110 - Flags: review?(florin.strugariu)
QA Whiteboard: [fxosqa-auto-s4] → fxosqa-auto-s5, fxosqa-auto-from-s4, fxosqa-auto-points=8
Attachment #8526110 - Flags: review?(florin.strugariu) → review+
Comment on attachment 8526110 [details] [review]
audio_channel2

Echoed from PR so it's in Bugzilla.

Per conversation, please file a bug about self.data.layer.current_audio_channel stealing the context. Getters should never do that (or should cache/restore it if they have to).

With a comment added at the first usage including a quick explanation that context gets stolen and a TODO to that bug, looks good and r=me
Attachment #8526110 - Flags: review?(gmealer) → review+
Ok, I filed bug 1109203 for that.
Blocks: 1109203
I updated the pull request with a comment with the bug number, so this pull request can be merged too.
Florin, could you perhaps merge this pull request (I'm still busy trying to get merge access)?
Flags: needinfo?(florin.strugariu)
I have merge capabilities myself now: https://github.com/mozilla-b2g/gaia/commit/41868e156bbb54c83cd88c9174c2caa9725574ef
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(florin.strugariu)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.