Closed
Bug 1083217
Opened 9 years ago
Closed 9 years ago
Add a test to play a song in artist list view
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: viorela, Assigned: viorela)
Details
Attachments
(2 files, 1 obsolete file)
In order to match the manual smoketest https://moztrap.mozilla.org/manage/case/4031/, we need to update test_music_album_mp3.py to select artist view, instead of album list view. Also rename the test file. Link to the automated test: https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/functional/music/test_music_album_mp3.py
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8509531 -
Flags: review?(robert.chira)
Attachment #8509531 -
Flags: review?(gmealer)
Attachment #8509531 -
Flags: review?(florin.strugariu)
Comment 2•9 years ago
|
||
Comment on attachment 8509531 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25410/files Test runs ok, but you need to fix merge conflicts.
Attachment #8509531 -
Flags: review?(robert.chira) → review+
Before I give a review comment, why didn't we just move the test link to https://moztrap.mozilla.org/manage/case/3755/ (same test in Album view) and add a completely new test (and fields, etc.) for Artist? It seems wrong to me to remove all the album stuff from the page objects just because the view changed. I don't even want to review the code until I understand the reasoning behind the approach.
Assignee | ||
Comment 4•9 years ago
|
||
I was not sure about how people would rather do here. IMO, I had 2 options in this case: 1. Update one of the existing tests (test_music_album_mp3.py or test_music_songs_3gp.py) to play music by artist, as both are marked as smoketests, but none of them follow STR from https://moztrap.mozilla.org/manage/case/4031/ 2. Add a new automated test to play music in artist view, and remove the existing smoketest tags of test_music_album_mp3.py and test_music_songs_3gp.py. The reason why I selected the first option was that there are just a few differences between the 2 test cases and we will have a lot of duplicate code if we will automate both. I will update my PR according to your suggestion, in order to not lose coverage on album view - then ask for review again.
Assignee | ||
Updated•9 years ago
|
Summary: Update test_music_album_mp3.py to play song in artist list view → Add a test to play a song in artist list view
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8509531 -
Attachment is obsolete: true
Attachment #8509531 -
Flags: review?(gmealer)
Attachment #8509531 -
Flags: review?(florin.strugariu)
Attachment #8512594 -
Flags: review?(robert.chira)
Attachment #8512594 -
Flags: review?(gmealer)
Attachment #8512594 -
Flags: review?(florin.strugariu)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → viorela.ioia
Updated•9 years ago
|
Attachment #8512594 -
Flags: review?(florin.strugariu) → review+
Comment on attachment 8512594 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25575 Looks good. Thanks for rethinking the approach. I'm not overly concerned regarding duplicate code when it comes to test case flows. It's typical for test cases to have lots of common flow right up to the specific point of test. Strictly speaking, you could move everything up to line 25 into setup and these two cases could share setup by being in the same file, assuming our harness supports standard xunit grouping. But since we seem to stick to one test case per file strictly, this came out about like I'd expect. I wouldn't share setup between files--too many opportunities to quietly invalidate one case when editing for another.
Attachment #8512594 -
Flags: review?(gmealer) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8512594 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25575 https://github.com/mozilla-b2g/gaia/commit/5d068ae7cc5622c32731b96d6d10e36bc58989b6
Attachment #8512594 -
Flags: review?(robert.chira)
Comment 9•9 years ago
|
||
(In reply to Geo Mealer [:geo] from comment #6) > > But since we seem to stick to one test case per file strictly, this came out > about like I'd expect. I wouldn't share setup between files--too many > opportunities to quietly invalidate one case when editing for another. Yes the one test per file format is dictated by the use of manifest/ini files for disabling/configuring tests.
Assignee | ||
Comment 10•9 years ago
|
||
uplift to v2.1
Flags: needinfo?(viorela.ioia)
Attachment #8514881 -
Flags: review?(robert.chira)
Attachment #8514881 -
Flags: review?(florin.strugariu)
Comment 11•9 years ago
|
||
Comment on attachment 8514881 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25662 Uplifted to v2.1: https://github.com/mozilla-b2g/gaia/commit/50358d2161da6b6da70a511143a79c6d1bffb2fa
Attachment #8514881 -
Flags: review?(robert.chira)
Attachment #8514881 -
Flags: review?(florin.strugariu)
Attachment #8514881 -
Flags: review+
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
QA Whiteboard: [fxosqa-auto-s2]
You need to log in
before you can comment on or make changes to this bug.
Description
•