Merge pull request #183 from tehkillerbee/133-fix-broken-tests

133 fix broken tests

mopidy_tidal/backend.py

@@ -63,10 +63,11 @@ def session(self):
 
     @property
     def logged_in(self):
-        if not self._logged_in:
+        # Auto-login if HACK is enabled
+        if not self._logged_in and self.login_method == "HACK":
             if self._active_session.load_session_from_file(self.session_file_path):
                 logger.info("Loaded TIDAL session from file %s", self.session_file_path)
-                self._logged_in = True
+                self._logged_in = self.session_valid
         return self._logged_in
 
     @property

mopidy_tidal/library.py

@@ -3,10 +3,10 @@
 import logging
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import suppress
-from typing import TYPE_CHECKING, List, Optional, Tuple, Union
+from typing import TYPE_CHECKING, List, Optional, Tuple
 
 from mopidy import backend, models
-from mopidy.models import Album, Artist, Image, Playlist, Ref, SearchResult, Track
+from mopidy.models import Image, Ref, SearchResult, Track
 from requests.exceptions import HTTPError
 from tidalapi.exceptions import ObjectNotFound, TooManyRequests
 
@@ -417,10 +417,11 @@ def _get_mood_items(session, mood_id):
     @staticmethod
     def _get_mix_tracks(session, mix_id):
         try:
-            return session.mix(mix_id).items()
+            mix = session.mix(mix_id)
         except ObjectNotFound:
             logger.debug("No such mix: %s", mix_id)
             return []
+        return mix.items()
 
     def _lookup_playlist(self, session, parts):
         playlist_id = parts[2]

tests/conftest.py

@@ -1,4 +1,6 @@
-from typing import Iterable, Optional, Sized
+import json
+from concurrent.futures import Future
+from typing import Optional
 from unittest.mock import Mock
 
 import pytest
@@ -9,6 +11,7 @@
 from tidalapi.mix import Mix
 from tidalapi.page import Page, PageCategory
 from tidalapi.playlist import UserPlaylist
+from tidalapi.session import LinkLogin
 
 from mopidy_tidal import context
 from mopidy_tidal.backend import TidalBackend
@@ -316,13 +319,42 @@ def get_backend(mocker):
     def _get_backend(config=mocker.MagicMock(), audio=mocker.Mock()):
         backend = TidalBackend(config, audio)
         session_factory = mocker.Mock()
-        session = mocker.Mock()
+        # session = mocker.Mock()
+        session = mocker.Mock(spec=SessionForTest)
         session.token_type = "token_type"
         session.session_id = "session_id"
         session.access_token = "access_token"
         session.refresh_token = "refresh_token"
+        session.is_pkce = False
         session_factory.return_value = session
         mocker.patch("mopidy_tidal.backend.Session", session_factory)
+
+        # Mock web_auth
+        backend.web_auth_server.start_oauth_daemon = mocker.Mock()
+
+        # Mock login_oauth
+        url = mocker.Mock(spec=LinkLogin, verification_uri_complete="link.tidal/URI")
+        future = mocker.Mock(spec=Future)
+        session.login_oauth.return_value = (url, future)
+
+        def save_session_dummy(file_path):
+            data = {
+                "token_type": {"data": session.token_type},
+                "session_id": {"data": session.session_id},
+                "access_token": {"data": session.access_token},
+                "refresh_token": {"data": session.refresh_token},
+                "is_pkce": {"data": session.is_pkce},
+                # "expiry_time": {"data": self.expiry_time},
+            }
+            with file_path.open("w") as outfile:
+                json.dump(data, outfile)
+
+        # Saving a session will create a dummy file containing the expected data
+        session.save_session_to_file.side_effect = save_session_dummy
+
+        # Always start in logged-out state
+        session.check_login.return_value = False
+
         return backend, config, audio, session_factory, session
 
     yield _get_backend

tests/test_backend.py

@@ -28,50 +28,64 @@ def test_backend_begins_in_correct_state(get_backend):
 
 def test_initial_login_caches_credentials(get_backend, config):
     backend, _, _, _, session = get_backend(config=config)
-    session.check_login.return_value = False
 
-    def login(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
+
+    # Starting the mock web server will trigger login instantly
+    backend.web_auth_server.start_oauth_daemon.side_effect = login_success
 
-    session.login_oauth_simple.side_effect = login
     backend._active_session = session
     authf = Path(config["core"]["data_dir"], "tidal/tidal-oauth.json")
     assert not authf.exists()
 
-    backend._login()
+    # backend._login()
+    backend.on_start()
+
+    # First attempt to (mock) load session from file (fails)
+    session.load_session_from_file.assert_called_once()
+    # Followed by OAuth (mock) daemon starting
+    backend.web_auth_server.start_oauth_daemon.assert_called_once()
+    # After a succesful (mock) oauth, the session file should be created
+    session.save_session_to_file.assert_called_once()
 
+    # Check if dummy file was created with the expected contents
     with authf.open() as f:
         data = load(f)
-    assert data == {
-        "token_type": dict(data="token_type"),
-        "session_id": dict(data="session_id"),
-        "access_token": dict(data="access_token"),
-        "refresh_token": dict(data="refresh_token"),
-    }
-    session.login_oauth_simple.assert_called_once()
+        assert data == {
+            "token_type": dict(data="token_type"),
+            "session_id": dict(data="session_id"),
+            "access_token": dict(data="access_token"),
+            "refresh_token": dict(data="refresh_token"),
+            "is_pkce": dict(data=False),
+        }
 
 
 def test_login_after_failed_cached_credentials_overwrites_cached_credentials(
     get_backend, config
 ):
     backend, _, _, _, session = get_backend(config=config)
-    session.check_login.return_value = False
 
-    def login(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
+
+    # Starting the mock web server will trigger login instantly
+    backend.web_auth_server.start_oauth_daemon.side_effect = login_success
 
-    session.login_oauth_simple.side_effect = login
     backend._active_session = session
     authf = Path(config["core"]["data_dir"], "tidal/tidal-oauth.json")
     cached_data = {
         "token_type": dict(data="token_type2"),
         "session_id": dict(data="session_id2"),
         "access_token": dict(data="access_token2"),
         "refresh_token": dict(data="refresh_token2"),
+        "is_pkce": dict(data=False),
     }
     authf.write_text(dumps(cached_data))
 
-    backend._login()
+    backend.on_start()
 
     with authf.open() as f:
         data = load(f)
@@ -80,15 +94,25 @@ def login(*_, **__):
         "session_id": dict(data="session_id"),
         "access_token": dict(data="access_token"),
         "refresh_token": dict(data="refresh_token"),
+        "is_pkce": dict(data=False),
     }
-    session.login_oauth_simple.assert_called_once()
+
+    # After a succesful (mock) oauth, the session file should be created (overwriting original file)
+    session.save_session_to_file.assert_called_once()
 
 
 def test_failed_login_does_not_overwrite_cached_credentials(
     get_backend, mocker, config, tmp_path
 ):
     backend, _, _, _, session = get_backend(config=config)
-    session.check_login.return_value = False
+
+    # trigger failed login by setting check_login false even though oauth was completed
+    def login_failed(*_, **__):
+        # session.check_login.return_value = False
+        backend._logged_in = True  # Set to true to skip login timeout immediately
+
+    # Starting the mock web server will trigger login instantly (but login will fail)
+    backend.web_auth_server.start_oauth_daemon.side_effect = login_failed
 
     backend._active_session = session
     authf = Path(config["core"]["data_dir"], "tidal/tidal-oauth.json")
@@ -101,16 +125,30 @@ def test_failed_login_does_not_overwrite_cached_credentials(
     authf.write_text(dumps(cached_data))
 
     with pytest.raises(ConnectionError):
-        backend._login()
+        backend.on_start()
 
     data = loads(authf.read_text())
     assert data == cached_data
-    session.login_oauth_simple.assert_called_once()
+
+    # First attempt to (mock) load session from file (fails)
+    session.load_session_from_file.assert_called_once()
+    # Followed by OAuth (mock) daemon starting
+    backend.web_auth_server.start_oauth_daemon.assert_called_once()
+    # Login failed, no session file created
+    session.save_session_to_file.assert_not_called()
 
 
 def test_failed_overall_login_throws_error(get_backend, tmp_path, mocker, config):
     backend, _, _, _, session = get_backend(config=config)
-    session.check_login.return_value = False
+
+    # trigger failed login by setting check_login false even though oauth was completed
+    def login_failed(*_, **__):
+        # session.check_login.return_value = False
+        backend._logged_in = True  # Set to true to skip login timeout immediately
+
+    # Starting the mock web server will trigger login instantly (but login will fail)
+    backend.web_auth_server.start_oauth_daemon.side_effect = login_failed
+
     backend._active_session = session
     authf = tmp_path / "auth.json"
 
@@ -124,11 +162,12 @@ def test_logs_in(get_backend, mocker, config):
     backend, _, _, session_factory, session = get_backend(config=config)
     backend._active_session = session
 
-    def set_logged_in(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
 
-    session.login_oauth_simple.side_effect = set_logged_in
-    session.check_login.return_value = False
+    # Starting the mock web server will trigger login instantly
+    backend.web_auth_server.start_oauth_daemon.side_effect = login_success
 
     backend.on_start()
 
@@ -143,19 +182,21 @@ def test_accessing_session_triggers_lazy_login(get_backend, mocker, config):
     config["tidal"]["lazy"] = True
     backend, _, _, session_factory, session = get_backend(config=config)
 
-    def set_logged_in(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
 
-    session.check_login.return_value = False
-    session.login_oauth_simple.side_effect = set_logged_in
+    # Loading session from file will result in successful login
+    session.load_session_from_file.side_effect = login_success
 
     backend.on_start()
 
-    session.login_oauth_simple.assert_not_called()
+    session.load_session_from_file.assert_not_called()
     assert not backend._active_session.check_login()
-    assert backend.session
+    assert backend.session  # accessing session will trigger login
     assert backend.session.check_login()
     session_factory.assert_called_once()
+    session.load_session_from_file.assert_called_once()
     config_obj = session_factory.mock_calls[0].args[0]
     assert config_obj.quality == config["tidal"]["quality"]
     assert config_obj.client_id == config["tidal"]["client_id"]
@@ -166,15 +207,16 @@ def test_logs_in_only_client_secret(get_backend, mocker, config):
     config["tidal"]["client_id"] = ""
     backend, _, _, session_factory, session = get_backend(config=config)
 
-    def set_logged_in(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
 
-    session.login_oauth_simple.side_effect = set_logged_in
-    session.check_login.return_value = False
+    # Loading session from file will result in successful login
+    session.load_session_from_file.side_effect = login_success
 
     backend.on_start()
 
-    session.login_oauth_simple.assert_called_once()
+    session.load_session_from_file.assert_called_once()
     session_factory.assert_called_once()
     config_obj = session_factory.mock_calls[0].args[0]
     assert config_obj.quality == config["tidal"]["quality"]
@@ -190,15 +232,16 @@ def test_logs_in_default_id_secret(get_backend, mocker, config):
     config["tidal"]["client_secret"] = ""
     backend, _, _, session_factory, session = get_backend(config=config)
 
-    def set_logged_in(*_, **__):
+    def login_success(*_, **__):
         session.check_login.return_value = True
+        backend._logged_in = True  # Set to true to skip login timeout immediately
 
-    session.login_oauth_simple.side_effect = set_logged_in
-    session.check_login.return_value = False
+    # Loading session from file will result in successful login
+    session.load_session_from_file.side_effect = login_success
 
     backend.on_start()
 
-    session.login_oauth_simple.assert_called_once()
+    session.load_session_from_file.assert_called_once()
     session_factory.assert_called_once()
     config_obj = session_factory.mock_calls[0].args[0]
     assert config_obj.quality == config["tidal"]["quality"]
@@ -225,6 +268,7 @@ def test_loads_session(get_backend, mocker, config):
 
     backend.on_start()
 
-    session.login_oauth_simple.assert_not_called()
+    # Loading session successfully should not trigger oauth_daemon
+    backend.web_auth_server.start_oauth_daemon.assert_not_called()
     session.load_session_from_file.assert_called_once()
     session_factory.assert_called_once()

tests/test_image_getter.py

@@ -1,4 +1,5 @@
 import pytest
+from tidalapi import Artist
 
 from mopidy_tidal.library import HTTPError, Image, ImagesGetter
 
@@ -87,12 +88,14 @@ def test_get_artist_image(images_getter, mocker):
 
 
 def test_get_artist_no_image_no_picture(images_getter, mocker):
-    # TODO: This follows exactly the strange logic about .image/.picture.
-    # However I'm not convinced that's correct anyhow.
+    # Handle artists with missing images
     ig, session = images_getter
     uri = "tidal:artist:2-2-2"
-    get_artist = mocker.Mock()
+    get_artist = mocker.Mock(spec=Artist)
+    # All image related entries should be None to trigger missing images
     get_artist.picture = None
+    get_artist.image = None
+    get_artist.square_picture = None
     session.artist.return_value = get_artist
     assert ig(uri) == (uri, [])