Skip to content

Commit

Permalink
player: show warning when user removes the current song in some cases (
Browse files Browse the repository at this point in the history
…#897)

* player: show warning when user removes the current song in some cases
* pytest: less warnings
  • Loading branch information
cosven authored Jan 16, 2025
1 parent f7d210c commit d9f39dc
Show file tree
Hide file tree
Showing 17 changed files with 67 additions and 47 deletions.
13 changes: 12 additions & 1 deletion feeluown/gui/components/player_playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,15 @@ def scroll_to_current_song(self):

def _remove_songs(self, songs):
for song in songs:
self._app.playlist.remove(song)
playlist_songs = self._app.playlist.list()
if (
self._app.playlist.mode is PlaylistMode.fm
# playlist_songs should not be empty, just for robustness
and playlist_songs
and song == self._app.playlist.current_song
and playlist_songs[-1] == song
):
self._app.show_msg("FM 模式下,如果当前歌曲是最后一首歌,则无法移除,请稍后再尝试移除", timeout=3000)
self._app.playlist.next()
else:
self._app.playlist.remove(song)
7 changes: 6 additions & 1 deletion feeluown/player/playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,13 @@ def remove_no_lock(self, song):
self._songs.remove(song)
new_next_song = self._get_next_song_no_lock()
self.set_existing_song_as_current_song(new_next_song)
elif next_song is None and self.mode is PlaylistMode.fm:
# The caller should not remove the current song when it
# is the last song in fm mode.
logger.error("Can't remove the last song in fm mode, will play next")
self._next_no_lock()
return
else:
next_song = self._get_next_song_no_lock()
self._songs.remove(song)
self.set_existing_song_as_current_song(next_song)
else:
Expand Down
4 changes: 2 additions & 2 deletions feeluown/serializers/plain.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ def _get_items(self, model):
# initialize fields that need to be serialized
# if as_line option is set, we always use fields_display
modelcls = type(model)
fields = [field for field in model.__fields__
if field not in BaseModel.__fields__]
fields = [field for field in model.model_fields
if field not in BaseModel.model_fields]
# Include properties.
pydantic_fields = ("__values__", "fields", "__fields_set__",
"model_computed_fields", "model_extra",
Expand Down
Empty file removed feeluown/uimodels/README.rst
Empty file.
Empty file removed feeluown/uimodels/__init__.py
Empty file.
8 changes: 0 additions & 8 deletions feeluown/uimodels/my_music.py

This file was deleted.

8 changes: 0 additions & 8 deletions feeluown/uimodels/playlist.py

This file was deleted.

8 changes: 0 additions & 8 deletions feeluown/uimodels/provider.py

This file was deleted.

6 changes: 6 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ test = pytest
# ignore DeprecationWarning raised by feeluown
filterwarnings =
ignore:use feeluown.*?:DeprecationWarning

# marshmallow may be not installed on some environment
# If you want to ignore the warning in local envinronment,
# you can run pytest like the following::
# pytest tests/ -W ignore::marshmallow.warnings.RemovedInMarshmallow4Warning
addopts = -q
--ignore=docs/
--ignore-glob=*/**/mpv*.py
Expand All @@ -40,6 +45,7 @@ addopts = -q
--cov-report=
--cov=feeluown
--doctest-modules
asyncio_default_fixture_loop_scope = function
[mypy-feeluown.mpv]
ignore_errors = True

Expand Down
11 changes: 9 additions & 2 deletions tests/app/conftest.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import asyncio
from unittest import mock

import pytest
import pytest_asyncio

from feeluown.argparser import create_cli_parser
from feeluown.app import create_config
from feeluown.plugin import PluginsManager
from feeluown.collection import CollectionManager
from feeluown.utils.dispatch import Signal
from feeluown.player import PlayerPositionDelegate


@pytest.fixture
@pytest.mark.asyncio
@pytest_asyncio.fixture
async def signal_aio_support():
Signal.setup_aio_support()
yield
Expand Down Expand Up @@ -41,3 +45,6 @@ def noharm(mocker):
mocker.patch('feeluown.app.app.Player')
mocker.patch.object(PluginsManager, 'enable_plugins')
mocker.patch.object(CollectionManager, 'scan')
# To avoid resource leak::
# RuntimeWarning: coroutine 'xxx' was never awaited
PlayerPositionDelegate.start = mock.MagicMock(return_value=asyncio.Future())
3 changes: 1 addition & 2 deletions tests/app/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@


@skip("No easy way to simulate QEventLoop.")
@pytest.mark.asyncio
async def test_create_gui_app(args):
def test_create_gui_app():
pass


Expand Down
1 change: 1 addition & 0 deletions tests/app/test_server_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ async def test_server_app_initialize(signal_aio_support, args, config, mocker,
mock_connect_1 = mocker.patch.object(app.live_lyric.sentence_changed, 'connect')
mock_connect_2 = mocker.patch.object(app.player.position_changed, 'connect')
mock_connect_3 = mocker.patch.object(app.playlist.song_changed, 'connect')

app.initialize()

# live lyric should be initialized properly.
Expand Down
4 changes: 3 additions & 1 deletion tests/entry_points/test_run_app.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import asyncio
import sys
from unittest.mock import AsyncMock
from unittest.mock import AsyncMock, MagicMock

import pytest

Expand All @@ -10,6 +10,7 @@
from feeluown.app import App, AppMode, get_app
from feeluown.app.cli_app import CliApp
from feeluown.plugin import PluginsManager
from feeluown.version import VersionManager


@pytest.fixture
Expand Down Expand Up @@ -72,6 +73,7 @@ def test_run_app_with_no_window_mode(argsparser, mocker, noqt, noharm):

@pytest.mark.asyncio
async def test_start_app(argsparser, mocker, noharm):
VersionManager.check_release = MagicMock(return_value=asyncio.Future())
mocker.patch('feeluown.entry_points.run_app.fuoexec_load_rcfile')
# fuoexec_init can be only called once, mock it here.
mocker.patch('feeluown.entry_points.run_app.fuoexec_init')
Expand Down
22 changes: 19 additions & 3 deletions tests/gui/pages/test_pages.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import asyncio
from unittest import mock

import pytest
import pytest_asyncio

from feeluown.library import BriefArtistModel, BriefAlbumModel, SongModel, \
PlaylistModel
from feeluown.utils.router import Request
from feeluown.gui.pages.model import render as render_model
from feeluown.gui.pages.song_explore import render as render_song_explore
from feeluown.gui.pages.song_explore import (
render as render_song_explore, SongWikiLabel, CoverLabelV2,
)
from feeluown.gui.page_containers.table import TableContainer
from feeluown.gui.uimain.page_view import RightPanel

Expand All @@ -21,6 +27,14 @@ def guiapp(qtbot, app_mock, library):
return app_mock


@pytest_asyncio.fixture
async def no_warning():
# To avoid such warning::
# RuntimeWarning: coroutine 'CoverLabelV2.show_cover' was never awaited
SongWikiLabel.show_song = mock.MagicMock(return_value=asyncio.Future())
CoverLabelV2.show_cover = mock.MagicMock(return_value=asyncio.Future())


@pytest.mark.asyncio
async def test_render_artist_v2(guiapp, ekaf_provider, ekaf_artist0, ):
artistv2 = BriefArtistModel(source=ekaf_provider.identifier,
Expand All @@ -42,7 +56,7 @@ async def test_render_album_v2(guiapp, ekaf_provider, ekaf_album0, ):


@pytest.mark.asyncio
async def test_render_song_v2(guiapp, ekaf_provider):
async def test_render_song_v2(guiapp, ekaf_provider, no_warning):
song = SongModel(source=ekaf_provider.identifier,
identifier='0',
title='',
Expand All @@ -56,12 +70,14 @@ async def test_render_song_v2(guiapp, ekaf_provider):


@pytest.mark.asyncio
async def test_render_song_v2_with_non_exists_album(guiapp, ekaf_provider):
async def test_render_song_v2_with_non_exists_album(guiapp, ekaf_provider, no_warning):
"""
When the album does not exist, the rendering process should succeed.
This test case tests that every exceptions, which raised by library, should
be correctly catched.
"""
SongWikiLabel.show_song = mock.MagicMock(return_value=asyncio.Future())
CoverLabelV2.show_cover = mock.MagicMock(return_value=asyncio.Future())
song = SongModel(source=ekaf_provider.identifier,
identifier='0',
title='',
Expand Down
2 changes: 1 addition & 1 deletion tests/gui/widgets/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_comment_list_view(qtbot):
content=content,
time=int(time.time()),
parent=brief_comment,)
comment2 = comment.copy()
comment2 = comment.model_copy()
comment2.content = 'hello world'

reader = create_reader([comment, comment2, comment])
Expand Down
8 changes: 0 additions & 8 deletions tests/library/test_model_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
from feeluown.library import SongModel, BriefAlbumModel, BriefArtistModel, BriefSongModel


def test_use_pydantic_from_orm(song):
# Raise a pydantic exception.
# For pydantic v1, pydantic.ConfigError is raised.
# FOr pydantic v2, pydantic.ValidationError is raised.
with pytest.raises(Exception):
BriefSongModel.from_orm(song)


def test_create_song_model_basic():
identifier = '1'
brief_album = BriefAlbumModel(identifier='1', source='x',
Expand Down
9 changes: 7 additions & 2 deletions tests/player/test_playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,13 @@ def feed_playlist():


@pytest.mark.asyncio
async def test_playlist_remove_current_song(app_mock):
pass
async def test_playlist_remove_current_song(pl, song1, mocker):
# Remove the current song, and it is the last song in the playlist.
mock_emit = mocker.patch.object(pl.eof_reached, 'emit')
pl.mode = PlaylistMode.fm
pl._current_song = song1
pl.remove(song1)
assert mock_emit.called


@pytest.mark.asyncio
Expand Down

0 comments on commit d9f39dc

Please sign in to comment.