[WIP] Monkeypatch tests (#448)

* Parameterize test_internals.py

* Create test_spotify_tools.py

* Monkeypatch pafy.download

* Monkeypatch YouTube search page

* Replace globals with fixtures

* Add missing urllib import, re-ordering and rename test_with_metadata.py

* Avoid creating temp directory in current working directory during test

* Update CHANGES.md

spotdl/convert.py

@@ -3,7 +3,8 @@ import os
 from logzero import logger as log
 
 
-"""What are the differences and similarities between ffmpeg, libav, and avconv?
+"""
+What are the differences and similarities between ffmpeg, libav, and avconv?
 https://stackoverflow.com/questions/9477115
 
 ffmeg encoders high to lower quality
@@ -25,10 +26,10 @@ def song(input_song, output_song, folder, avconv=False, trim_silence=False):
     else:
         return 0
     if avconv:
-        exit_code = convert.with_avconv()
+        exit_code, command = convert.with_avconv()
     else:
-        exit_code = convert.with_ffmpeg()
-    return exit_code
+        exit_code, command = convert.with_ffmpeg()
+    return exit_code, command
 
 
 class Converter:
@@ -59,7 +60,7 @@ class Converter:
             log.warning("--trim-silence not supported with avconv")
 
         log.debug(command)
-        return subprocess.call(command)
+        return subprocess.call(command), command
 
     def with_ffmpeg(self):
         ffmpeg_pre = "ffmpeg -y "
@@ -84,7 +85,7 @@ class Converter:
             if output_ext == ".mp3":
                 ffmpeg_params = "-codec:a libmp3lame -ar 44100 "
             elif output_ext == ".m4a":
-                ffmpeg_params = "-cutoff 20000 -codec:a libfdk_aac -ar 44100 "
+                ffmpeg_params = "-cutoff 20000 -codec:a aac -ar 44100 "
 
         if output_ext == ".flac":
             ffmpeg_params = "-codec:a flac -ar 44100 "
@@ -104,4 +105,4 @@ class Converter:
         )
 
         log.debug(command)
-        return subprocess.call(command)
+        return subprocess.call(command), command

spotdl/downloader.py

@@ -1,3 +1,8 @@
+import spotipy
+import urllib
+import os
+from logzero import logger as log
+
 from spotdl import const
 from spotdl import metadata
 from spotdl import convert
@@ -5,10 +10,6 @@ from spotdl import internals
 from spotdl import spotify_tools
 from spotdl import youtube_tools
 
-import spotipy
-from logzero import logger as log
-import os
-
 
 class CheckExists:
     def __init__(self, music_file, meta_tags=None):

spotdl/handle.py

@@ -1,14 +1,14 @@
-import appdirs
-from spotdl import internals
 from logzero import logger as log
+import appdirs
 
 import logging
 import yaml
 import argparse
 import mimetypes
-
 import os
 
+from spotdl import internals
+
 
 _LOG_LEVELS_STR = ["INFO", "WARNING", "ERROR", "DEBUG"]
 

spotdl/internals.py

@@ -1,6 +1,6 @@
+from logzero import logger as log
 import os
 import sys
-from logzero import logger as log
 
 from spotdl import const
 

spotdl/metadata.py

@@ -2,10 +2,11 @@ from mutagen.easyid3 import EasyID3
 from mutagen.id3 import ID3, TORY, TYER, TPUB, APIC, USLT, COMM
 from mutagen.mp4 import MP4, MP4Cover
 from mutagen.flac import Picture, FLAC
-from logzero import logger as log
-from spotdl.const import TAG_PRESET, M4A_TAG_PRESET
 
 import urllib.request
+from logzero import logger as log
+
+from spotdl.const import TAG_PRESET, M4A_TAG_PRESET
 
 
 def compare(music_file, metadata):

spotdl/spotdl.py

@@ -1,5 +1,11 @@
 #!/usr/bin/env python3
 
+import sys
+import platform
+import pprint
+import logzero
+from logzero import logger as log
+
 from spotdl import __version__
 from spotdl import const
 from spotdl import handle
@@ -7,11 +13,6 @@ from spotdl import internals
 from spotdl import spotify_tools
 from spotdl import youtube_tools
 from spotdl import downloader
-from logzero import logger as log
-import logzero
-import sys
-import platform
-import pprint
 
 
 def debug_sys_info():

spotdl/spotify_tools.py

@@ -1,15 +1,15 @@
 import spotipy
 import spotipy.oauth2 as oauth2
 import lyricwikia
-from logzero import logger as log
-
-from spotdl import internals
 
 from slugify import slugify
 from titlecase import titlecase
+from logzero import logger as log
 import pprint
 import sys
 
+from spotdl import internals
+
 
 def generate_token():
     """ Generate the token. Please respect these credentials :) """
@@ -29,8 +29,8 @@ def refresh_token():
 
 # token is mandatory when using Spotify's API
 # https://developer.spotify.com/news-stories/2017/01/27/removing-unauthenticated-calls-to-the-web-api/
-token = generate_token()
-spotify = spotipy.Spotify(auth=token)
+_token = generate_token()
+spotify = spotipy.Spotify(auth=_token)
 
 
 def generate_metadata(raw_song):
@@ -87,12 +87,6 @@ def generate_metadata(raw_song):
     return meta_tags
 
 
-def write_user_playlist(username, text_file=None):
-    links = get_playlists(username=username)
-    playlist = internals.input_link(links)
-    return write_playlist(playlist, text_file)
-
-
 def get_playlists(username):
     """ Fetch user playlists when using the -u option. """
     playlists = spotify.user_playlists(username)
@@ -121,6 +115,12 @@ def get_playlists(username):
     return links
 
 
+def write_user_playlist(username, text_file=None):
+    links = get_playlists(username=username)
+    playlist = internals.input_link(links)
+    return write_playlist(playlist, text_file)
+
+
 def fetch_playlist(playlist):
     try:
         playlist_id = internals.extract_spotify_id(playlist)
@@ -154,7 +154,7 @@ def fetch_album(album):
     return album
 
 
-def fetch_album_from_artist(artist_url, album_type="album"):
+def fetch_albums_from_artist(artist_url, album_type="album"):
     """
     This funcction returns all the albums from a give artist_url using the US
     market
@@ -191,7 +191,7 @@ def write_all_albums_from_artist(artist_url, text_file=None):
     album_base_url = "https://open.spotify.com/album/"
 
     # fetching all default albums
-    albums = fetch_album_from_artist(artist_url)
+    albums = fetch_albums_from_artist(artist_url)
 
     # if no file if given, the default save file is in the current working
     # directory with the name of the artist
@@ -204,7 +204,7 @@ def write_all_albums_from_artist(artist_url, text_file=None):
         write_album(album_base_url + album["id"], text_file=text_file)
 
     # fetching all single albums
-    singles = fetch_album_from_artist(artist_url, album_type="single")
+    singles = fetch_albums_from_artist(artist_url, album_type="single")
 
     for single in singles:
         log.info("Fetching single: " + single["name"])

spotdl/youtube_tools.py

@@ -1,15 +1,15 @@
 from bs4 import BeautifulSoup
 import urllib
 import pafy
+
 from slugify import slugify
 from logzero import logger as log
+import os
 
 from spotdl import spotify_tools
 from spotdl import internals
 from spotdl import const
 
-import os
-
 # Fix download speed throttle on short duration tracks
 # Read more on mps-youtube/pafy#199
 pafy.g.opener.addheaders.append(("Range", "bytes=0-"))
@@ -75,6 +75,8 @@ def generate_m3u(track_file):
     log.info("Generating {0} from {1} YouTube URLs".format(target_file, total_tracks))
     with open(target_file, "w") as output_file:
         output_file.write("#EXTM3U\n\n")
+
+    videos = []
     for n, track in enumerate(tracks, 1):
         content, _ = match_video_and_metadata(track)
         if content is None:
@@ -94,6 +96,9 @@ def generate_m3u(track_file):
             log.debug(m3u_key)
             with open(target_file, "a") as output_file:
                 output_file.write(m3u_key)
+            videos.append(content.watchv_url)
+
+    return videos
 
 
 def download_song(file_name, content):
@@ -240,7 +245,7 @@ class GenerateYouTubeURL:
         search_url = generate_search_url(self.search_query)
         log.debug("Opening URL: {0}".format(search_url))
 
-        item = urllib.request.urlopen(search_url).read()
+        item = self._fetch_response(search_url).read()
         items_parse = BeautifulSoup(item, "html.parser")
 
         videos = []
@@ -319,3 +324,11 @@ class GenerateYouTubeURL:
             return self._best_match(videos)
 
         return videos
+
+    @staticmethod
+    def _fetch_response(url):
+        # XXX: This method exists only because it helps us indirectly
+        # monkey patch `urllib.request.open`, directly monkey patching
+        # `urllib.request.open` causes us to end up in an infinite recursion
+        # during the test since `urllib.request.open` would monkeypatch itself.
+        return urllib.request.urlopen(url)