Add a test to play a song in artist list view

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: viorela, Assigned: viorela)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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

4 years ago
Created attachment 8509531 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25410/files
Attachment #8509531 - Flags: review?(robert.chira)
Attachment #8509531 - Flags: review?(gmealer)
Attachment #8509531 - Flags: review?(florin.strugariu)
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

4 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

4 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

4 years ago
Created attachment 8512594 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25575
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

4 years ago
Assignee: nobody → viorela.ioia
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+
Can we get this uplifted to v2.1
Flags: needinfo?(viorela.ioia)

Comment 9

4 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

4 years ago
Created attachment 8514881 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25662

uplift to v2.1
Flags: needinfo?(viorela.ioia)
Attachment #8514881 - Flags: review?(robert.chira)
Attachment #8514881 - Flags: review?(florin.strugariu)
Status: NEW → RESOLVED
Last Resolved: 4 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.