Merge lp:~nskaggs/music-app/fix-shuffle-test into lp:music-app/trusty

Proposed by Nicholas Skaggs
Status: Merged
Approved by: Nicholas Skaggs
Approved revision: 286
Merged at revision: 273
Proposed branch: lp:~nskaggs/music-app/fix-shuffle-test
Merge into: lp:music-app/trusty
Diff against target: 293 lines (+156/-71)
1 file modified
tests/autopilot/music_app/tests/test_music.py (+156/-71)
To merge this branch: bzr merge lp:~nskaggs/music-app/fix-shuffle-test
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Andrew Hayzen Approve
Review via email: mp+198485@code.launchpad.net

Commit message

Simplify shuffle test and make it consistent, fix next and previous as well -- don't force order

Description of the change

Change the shuffle test to be much simpler. Test now checks to see that we cycle at least one song and that song was not the 'next' song in the queue order to prove shuffle is random.

fix next and previous as well -- don't force order or use specific song names

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Shuffle is failing on the dashboard, and this should make it more consistent. I'm not sure if there is indeed more you wish to test, but the current iteration isn't useful

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Not sure how useful this is, but I ran autopilot with this branch on a Nexus 4 five times and got the following results.

First run:
Ran 8 tests in 200.609s
OK

Second run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 146, in test_previous_and_mp3
self.assertThat(title, Eventually(Equals("TestMP3Title")))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test failed: 'TestMP3Title' != u'Swansong'

Ran 8 tests in 214.126s
FAILED (failures=1)

Third run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 168, in test_shuffle
self.assertThat(self.main_view.random, Eventually(Equals(True)))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test on MainView.random failed: True != dbus.Boolean(False, variant_level=1)

Ran 8 tests in 199.545s
FAILED (failures=1)

Fourth run:
Ran 8 tests in 199.184s
OK

Fifth run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 168, in test_shuffle
self.assertThat(self.main_view.random, Eventually(Equals(True)))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test on MainView.random failed: True != dbus.Boolean(False, variant_level=1)

Ran 8 tests in 201.106s
FAILED (failures=1)

If you need any more information please let me know.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

Can you explain the shuffle test changes? It looks like you're checking just to see if the song has changed to confirm that your "shuffled" variable is true. This is incorrect--hitting next when shuffle is off also changes the currently playing song. We need to confirm that the next track is NOT what would have otherwise came next. Then it looks like you keep hitting forward until you get to the original track. What does that prove?

maybe a decent strategy would be to hit forward, turn off shuffle, hit back, if the current track is the original track then shuffle hasn't been verified so we try again (currently up to 100 times).

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Victor, thanks for commenting. I knew you would probably find an issue with the implementation which is why I asked. I must admit shuffle has been a source of confusion for me with the music app :-)

"We need to confirm that the next track is NOT what would have otherwise came next. "

^^ This is a good point. Your view of what I'm doing is correct. It's possible the original implementation would work now with the tweaks made, but I like your idea of hitting next and previous while toggling shuffle. I'll implement that and see what it looks like.

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

As a note, I haven't had the latest code fail, *yet*.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Victor, having Andrew review. Made the changes you requested, and it seems to verify fine. I ran it enough times that it sometimes had to cycle a bit to "prove" the randomness. I think all is well.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Looks good, ran 5 times on Nexus 4 without fail :)

'Ran 9 tests in 258.209s
OK'

review: Approve
Revision history for this message
Victor Thompson (vthompson) wrote :

I don't know if I agree with the next and previous changes. It looks like you attempted to harden then against failure, but at the same time it looks like you've made it almost impossible for the tests to fail. IMO it should not loop through 10 times and hit the forward/back button until it hits it's target. These are not randomized like shuffle so if it doesn't work the first try the test should fail. Or am I missing something?

review: Needs Fixing
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Victor I was just looking at them again. I think I could tweak them a bit further. The trouble is I didn't like enforcing which track was the first -- it was gimicky. Instead, we should do the same as shuffle. Hit next, check song, hit previous and make sure it's the original song (and of course that the "next" song != original). Repeat once and I'd say we've confirmed it eh? The only thing is you wanted to ensure you tested an mp3 playback as well, so that might be trickier. Can you hop on IRC?

280. By Nicholas Skaggs

revamped next, previous tests and tweaked mp3 test

281. By Nicholas Skaggs

pep8 and pyflakes :-)

282. By Nicholas Skaggs

tweak comments

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

The test_next_previous looks good. I'm not so sure with the test_mp3 1) maybe ensure shuffle is off outside the loop, 2) it's possible to break out of the loop because of the count--so I think you need to assert that the file actually ends with .mp3 just as you assert that it is playing.

review: Needs Fixing
283. By Nicholas Skaggs

tweaks for mp3 test, endswith check fails

284. By Nicholas Skaggs

mp3 test fixed

285. By Nicholas Skaggs

fix mp3 test and add debug line

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
286. By Nicholas Skaggs

make queue size dynamic (read size)

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

Did a few runs on the device. LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/music_app/tests/test_music.py'
2--- tests/autopilot/music_app/tests/test_music.py 2013-12-06 17:04:38 +0000
3+++ tests/autopilot/music_app/tests/test_music.py 2013-12-12 19:26:51 +0000
4@@ -9,6 +9,8 @@
5
6 from __future__ import absolute_import
7
8+import time
9+import logging
10 from autopilot.matchers import Eventually
11 from testtools.matchers import Equals, Is, Not, LessThan, NotEquals
12 from testtools.matchers import GreaterThan
13@@ -16,6 +18,8 @@
14
15 from music_app.tests import MusicTestCase
16
17+logger = logging.getLogger(__name__)
18+
19
20 class TestMainWindow(MusicTestCase):
21
22@@ -86,63 +90,117 @@
23 self.pointing_device.click_object(playbutton)
24 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
25
26- def test_next(self):
27+ def test_next_previous(self):
28 """ Test going to next track (Music Library must exist) """
29
30 # populate queue
31 first_genre_item = self.main_view.get_first_genre_item()
32 self.pointing_device.click_object(first_genre_item)
33
34- forwardbutton = self.main_view.get_forward_button()
35+ playbutton = self.main_view.get_now_playing_play_button()
36+ shufflebutton = self.main_view.get_shuffle_button()
37
38 title = lambda: self.main_view.currentTracktitle
39 artist = lambda: self.main_view.currentArtist
40- self.assertThat(title,
41- Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
42- self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
43-
44- """ Track is playing"""
45- self.assertThat(self.main_view.isPlaying, Equals(True))
46+
47+ orgTitle = self.main_view.currentTracktitle
48+ orgArtist = self.main_view.currentArtist
49+
50+ #check original track
51+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
52+ logger.debug("Original Song %s, %s" % (orgTitle, orgArtist))
53+
54+ """ Pause track """
55+ self.pointing_device.click_object(playbutton)
56+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
57+
58+ #ensure shuffle is off
59+ if self.main_view.random:
60+ logger.debug("Turning off shuffle")
61+ self.pointing_device.click_object(shufflebutton)
62+ else:
63+ logger.debug("Shuffle already off")
64+ self.assertThat(self.main_view.random, Eventually(Equals(False)))
65+
66+ """ Select next """
67+ #goal is to go back and forth and ensure 2 different songs
68+ forwardbutton = self.main_view.get_forward_button()
69 self.pointing_device.click_object(forwardbutton)
70-
71- """ Track is playing"""
72 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
73- self.assertThat(title, Eventually(Equals("Swansong")))
74- self.assertThat(artist, Eventually(Equals("Josh Woodward")))
75-
76- def test_previous_and_mp3(self):
77- """ Test going to previous track, last item must be an MP3
78- (Music Library must exist) """
79-
80- # populate queue
81- first_genre_item = self.main_view.get_first_genre_item()
82- self.pointing_device.click_object(first_genre_item)
83-
84- playbutton = self.main_view.get_now_playing_play_button()
85+
86+ #ensure different song
87+ self.assertThat(title, Eventually(NotEquals(orgTitle)))
88+ self.assertThat(artist, Eventually(NotEquals(orgArtist)))
89+ nextTitle = self.main_view.currentTracktitle
90+ nextArtist = self.main_view.currentArtist
91+ logger.debug("Next Song %s, %s" % (nextTitle, nextArtist))
92
93 """ Pause track """
94 self.pointing_device.click_object(playbutton)
95 self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
96
97- """ Repeat is off """
98- repeatbutton = self.main_view.get_repeat_button()
99- self.pointing_device.click_object(repeatbutton)
100-
101- previousbutton = self.main_view.get_previous_button()
102-
103- title = lambda: self.main_view.currentTracktitle
104- artist = lambda: self.main_view.currentArtist
105- self.assertThat(title,
106- Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
107- self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
108-
109 """ Select previous """
110+ previousbutton = self.main_view.get_previous_button()
111 self.pointing_device.click_object(previousbutton)
112+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
113+
114+ #ensure we're back to original song
115+ self.assertThat(title, Eventually(Equals(orgTitle)))
116+ self.assertThat(artist, Eventually(Equals(orgArtist)))
117+
118+ def test_mp3(self):
119+ """ Test that mp3 "plays" or at least doesn't crash on load """
120+
121+ # populate queue
122+ first_genre_item = self.main_view.get_first_genre_item()
123+ self.pointing_device.click_object(first_genre_item)
124+
125+ playbutton = self.main_view.get_now_playing_play_button()
126+ shufflebutton = self.main_view.get_shuffle_button()
127+
128+ title = self.main_view.currentTracktitle
129+ artist = self.main_view.currentArtist
130+
131+ #ensure track is playing
132+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
133+
134+ #ensure shuffle is off
135+ if self.main_view.random:
136+ logger.debug("Turning off shuffle")
137+ self.pointing_device.click_object(shufflebutton)
138+ else:
139+ logger.debug("Shuffle already off")
140+ self.assertThat(self.main_view.random, Eventually(Equals(False)))
141
142 """ Track is playing """
143+ count = 1
144+ #ensure track appears before looping through queue more than once
145+ #needs to contain test mp3 metadata and end in *.mp3
146+ queue = self.main_view.get_queue_track_count()
147+ while title != "TestMP3Title" and artist != "TestMP3Artist":
148+ self.assertThat(count, LessThan(queue))
149+
150+ """ Pause track """
151+ self.pointing_device.click_object(playbutton)
152+ self.assertThat(self.main_view.isPlaying,
153+ Eventually(Equals(False)))
154+
155+ """ Select next """
156+ forwardbutton = self.main_view.get_forward_button()
157+ self.pointing_device.click_object(forwardbutton)
158+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
159+
160+ title = self.main_view.currentTracktitle
161+ artist = self.main_view.currentArtist
162+ logger.debug("Current Song %s, %s" % (title, artist))
163+ logger.debug("File found %s" % self.main_view.currentFile)
164+
165+ count = count + 1
166+
167+ #make sure mp3 plays
168+ self.assertThat(self.main_view.currentFile.endswith("mp3"),
169+ Equals(True))
170 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
171- self.assertThat(title, Eventually(Equals("TestMP3Title")))
172- self.assertThat(artist, Eventually(Equals("TestMP3Artist")))
173
174 def test_shuffle(self):
175 """ Test shuffle (Music Library must exist) """
176@@ -151,55 +209,82 @@
177 first_genre_item = self.main_view.get_first_genre_item()
178 self.pointing_device.click_object(first_genre_item)
179
180- shufflebutton = self.main_view.get_shuffle_button()
181-
182- forwardbutton = self.main_view.get_forward_button()
183-
184- previousbutton = self.main_view.get_previous_button()
185-
186- playbutton = self.main_view.get_now_playing_play_button()
187-
188- title = lambda: self.main_view.currentTracktitle
189- artist = lambda: self.main_view.currentArtist
190- self.assertThat(title,
191- Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
192- self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
193-
194 """ Track is playing, shuffle is turned on"""
195- self.assertThat(self.main_view.isPlaying, Equals(True))
196- self.pointing_device.click_object(shufflebutton)
197- self.assertThat(self.main_view.random, Eventually(Equals(True)))
198-
199- forward = True
200+ shufflebutton = self.main_view.get_shuffle_button()
201+ forwardbutton = self.main_view.get_forward_button()
202+ playbutton = self.main_view.get_now_playing_play_button()
203+ previousbutton = self.main_view.get_previous_button()
204+
205+ #play for a second, then pause
206+ if not self.main_view.isPlaying:
207+ logger.debug("Play not selected")
208+ self.pointing_device.click_object(playbutton)
209+ else:
210+ logger.debug("Already playing")
211+
212+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
213+ time.sleep(1)
214+ self.pointing_device.click_object(playbutton)
215+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
216+
217 count = 0
218 while True:
219 self.assertThat(count, LessThan(100))
220
221+ #goal is to hit next under shuffle mode
222+ #then verify original track is not the previous track
223+ #this means a true shuffle happened
224+ #if it doesn't try again, up to count times
225+
226+ orgTitle = self.main_view.currentTracktitle
227+ orgArtist = self.main_view.currentArtist
228+ logger.debug("Original Song %s, %s" % (orgTitle, orgArtist))
229+
230 if (not self.main_view.toolbarShown):
231 self.main_view.show_toolbar()
232
233- if forward:
234- self.pointing_device.click_object(forwardbutton)
235+ #ensure shuffle is on
236+ if not self.main_view.random:
237+ logger.debug("Turning on shuffle")
238+ self.pointing_device.click_object(shufflebutton)
239 else:
240- self.pointing_device.click_object(previousbutton)
241+ logger.debug("Shuffle already on")
242+ self.assertThat(self.main_view.random, Eventually(Equals(True)))
243
244- """ Track is playing"""
245+ self.pointing_device.click_object(forwardbutton)
246 self.assertThat(self.main_view.isPlaying,
247 Eventually(Equals(True)))
248- if (self.main_view.currentTracktitle == "TestMP3Title" and
249- self.main_view.currentArtist == "TestMP3Artist"):
250+ title = self.main_view.currentTracktitle
251+ artist = self.main_view.currentArtist
252+ logger.debug("Current Song %s, %s" % (title, artist))
253+
254+ #go back to previous and check against original
255+ #play song, then pause before switching
256+ time.sleep(1)
257+ self.pointing_device.click_object(playbutton)
258+ self.assertThat(self.main_view.isPlaying,
259+ Eventually(Equals(False)))
260+
261+ #ensure shuffle is off
262+ if self.main_view.random:
263+ logger.debug("Turning off shuffle")
264+ self.pointing_device.click_object(shufflebutton)
265+ else:
266+ logger.debug("Shuffle already off")
267+ self.assertThat(self.main_view.random, Eventually(Equals(False)))
268+
269+ self.pointing_device.click_object(previousbutton)
270+
271+ title = self.main_view.currentTracktitle
272+ artist = self.main_view.currentArtist
273+
274+ if title != orgTitle and artist != orgArtist:
275+ #we shuffled properly
276+ logger.debug("Yay, shuffled %s, %s" % (title, artist))
277 break
278 else:
279- """ Show toolbar if hidden """
280- if (not self.main_view.toolbarShown):
281- self.main_view.show_toolbar()
282-
283- """ Pause track """
284- self.pointing_device.click_object(playbutton)
285- self.assertThat(self.main_view.isPlaying,
286- Eventually(Equals(False)))
287- forward = not forward
288- count += 1
289+ logger.debug("Same track, no shuffle %s, %s" % (title, artist))
290+ count += 1
291
292 def test_show_albums_sheet(self):
293 """tests navigating to the Albums tab and displaying the album sheet"""

Subscribers

People subscribed via source and target branches

to status/vote changes: