C22 Sunitha Nela and Izzy Hirad #15
Conversation
|
|
||
| # Act | ||
| updated_data = watch_movie(janes_data, MOVIE_TITLE_1) | ||
|
|
||
| # Assert | ||
| assert len(updated_data["watchlist"]) == 0 | ||
| assert len(updated_data["watched"]) == 1 | ||
|
|
||
| raise Exception("Test needs to be completed.") | ||
| assert updated_data == expected_result |
|
|
||
| # Act | ||
| updated_data = watch_movie(janes_data, movie_to_watch["title"]) | ||
|
|
||
| # Assert | ||
| assert len(updated_data["watchlist"]) == 1 | ||
| assert len(updated_data["watched"]) == 2 | ||
|
|
||
| raise Exception("Test needs to be completed.") | ||
| assert updated_data == expected_result |
There was a problem hiding this comment.
👍 Another way to approach this test could be to write assertions like this:
assert HORROR_1 in updated_data["watched"]
assert FANTASY_2 in updated_data["watched"]
assert FANTASY_1 in updated_data["watchlist"]Then you wouldn't need to create expected_result on lines 188-194
| assert HORROR_1 in friends_unique_movies | ||
| assert INTRIGUE_3 in friends_unique_movies | ||
| assert FANTASY_4 in friends_unique_movies | ||
| assert INTRIGUE_3_count == 1 |
There was a problem hiding this comment.
The assertions on lines 63-65 look good 👍
I have a couple of thoughts about the assertion on line 66. The value of INTRIGUE_3_count is calculated on lines 55-58 in this test file, not in party.py, which means we're not directly testing the method get_friends_unique_watched which is the function under test here.
What is the condition that you want to test with the assertion on line 66?
Why is it that we would only need to test the count of INTRIGUE_3 but not HORROR_1 or FANTASY_4?
When writing tests, we'll want to think about 'What am I testing for? Do my assertions accurately measure or test what I think I'm testing for?"
| assert not recommendations | ||
| assert len(recommendations) == 0 |
There was a problem hiding this comment.
These assertions are testing the same thing, to see if recommendations is an empty list. I think line 60 because it's more idiomatic. To avoid redundancy here we can remove line 61.
| @@ -52,13 +52,21 @@ def test_new_genre_rec_from_empty_friends(): | |||
| } | |||
| ] | |||
| } | |||
| expected_result = [] | |||
There was a problem hiding this comment.
This variable is not accessed so it should be deleted.
| most_watched_genre = get_most_watched_genre(user_data) | ||
| friend_unique_movie = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
🥳 Nice job reusing functions you already wrote.
| for movie in friend_unique_movie: | ||
| if most_watched_genre == movie["genre"]: | ||
| rec_movies.append(movie) |
There was a problem hiding this comment.
How could you use list comprehension here to create rec_movies?
|
|
||
|
|
||
| rec_movies = [movie for movie in unique_watched_movie if movie["title"] in fav_titles] | ||
|
|
||
|
|
There was a problem hiding this comment.
We only need 1 blank line to visually break up our code in functions
|
|
||
| def get_rec_from_favorites(user_data): | ||
| unique_watched_movie = get_unique_watched(user_data) | ||
| fav_titles = {movie["title"] for movie in user_data["favorites"]} |
There was a problem hiding this comment.
😎 set comprehension, nice!
Would prefer fav_titles to be named more descriptively so we know whose favorite titles we're working with. How about user_fav_titles?
| fav_titles = {movie["title"] for movie in user_data["favorites"]} | ||
|
|
||
|
|
||
| rec_movies = [movie for movie in unique_watched_movie if movie["title"] in fav_titles] |
There was a problem hiding this comment.
Nicely done 👍
You could also directly return the list, like:
return [movie for movie in unique_watched_movie if movie["title"] in fav_titles]
No description provided.