Closed Bug 1032757 Opened 6 years ago Closed 6 years ago

Re-enable Music player tests - Playback tests

Categories

(Firefox OS Graveyard :: Gaia::Music, defect)

x86
macOS
defect
Not set

Tracking

(b2g-v2.1 fixed)

RESOLVED FIXED
Tracking Status
b2g-v2.1 --- fixed

People

(Reporter: rudyl, Unassigned)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1032037 +++

The test has been disabled on Travis due to Bug #1032037, so file this to track re-enabling work.
Attached file Patch V1
This patch attempts to address the player test issue by removing the delay between switching music app to fake control, and only verify that the progress would be updated.

I've done several runs on Travis to make sure this will be a stable test,
https://travis-ci.org/mozilla-b2g/gaia/builds/29299041

Jim,

Could you please help review this modification?
Thank you.
Attachment #8451571 - Flags: review?(squibblyflabbetydoo)
I don't think this is actually testing anything anymore. Even if this broke again, wouldn't the test pass, since the progress would have moved between setting t0 and running controls.launch()?
Comment on attachment 8451571 [details] [review]
Patch V1

I understand your point and will try another way to get a meaningful test.
Thanks for the feedback.
Attachment #8451571 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8451571 [details] [review]
Patch V1

Patch updated so that it will try to get the song progress of music app (in the background) after the fakeControl has stopped the music, and then it would check the progress would be updated after the music app is brought to foreground.

Jim, would you please review this again?
Thanks.

--
The stability check for this test could be found here,
https://travis-ci.org/mozilla-b2g/gaia/builds/29392117
Attachment #8451571 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8451571 [details] [review]
Patch V1

This seems better, thanks. r=me

If you can, please verify that the tests fail if you comment out this line:

https://github.com/mozilla-b2g/gaia/blob/1e85890f5b52b734308c0cedda2dfc3b98266941/apps/music/js/Player.js#L1044
Attachment #8451571 - Flags: review?(squibblyflabbetydoo) → review+
(In reply to Jim Porter (:squib) from comment #5)
> Comment on attachment 8451571 [details] [review]
> Patch V1
> 
> This seems better, thanks. r=me
> 
> If you can, please verify that the tests fail if you comment out this line:
> 
> https://github.com/mozilla-b2g/gaia/blob/
> 1e85890f5b52b734308c0cedda2dfc3b98266941/apps/music/js/Player.js#L1044

Yes, I've done that verification before asking for review.
Thanks for the review and the reminding.
Status: NEW → ASSIGNED
Landed to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/ff6ddf1949d23155afed3d14c060cf7eddeb7c28

--
Thanks for the review.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.