diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 5441940a4..975c884d6 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -51,3 +51,5 @@ c490ac5810b70f3cf5fd8649669838e8fdb19f4d 9147577b2b19f43ca827e9650261a86fb0450cef # Copy paste query, types from library to dbcore 1a045c91668c771686f4c871c84f1680af2e944b +# Library restructure (split library.py into multiple modules) +0ad4e19d4f870db757373f44d12ff3be2441363a diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ac3263bcd..390878372 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -52,7 +52,7 @@ jobs: - if: ${{ env.IS_MAIN_PYTHON != 'true' }} name: Test without coverage run: | - poetry install --extras=autobpm --extras=lyrics + poetry install --extras=autobpm --extras=lyrics --extras=embedart poe test - if: ${{ env.IS_MAIN_PYTHON == 'true' }} diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 7d9f0cee7..ae8e0ddf6 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -411,39 +411,6 @@ class BooleanQuery(MatchQuery[int]): super().__init__(field_name, pattern_int, fast) -class BytesQuery(FieldQuery[bytes]): - """Match a raw bytes field (i.e., a path). This is a necessary hack - to work around the `sqlite3` module's desire to treat `bytes` and - `unicode` equivalently in Python 2. Always use this query instead of - `MatchQuery` when matching on BLOB values. - """ - - def __init__(self, field_name: str, pattern: bytes | str | memoryview): - # Use a buffer/memoryview representation of the pattern for SQLite - # matching. This instructs SQLite to treat the blob as binary - # rather than encoded Unicode. - if isinstance(pattern, (str, bytes)): - if isinstance(pattern, str): - bytes_pattern = pattern.encode("utf-8") - else: - bytes_pattern = pattern - self.buf_pattern = memoryview(bytes_pattern) - elif isinstance(pattern, memoryview): - self.buf_pattern = pattern - bytes_pattern = bytes(pattern) - else: - raise ValueError("pattern must be bytes, str, or memoryview") - - super().__init__(field_name, bytes_pattern) - - def col_clause(self) -> tuple[str, Sequence[SQLiteType]]: - return self.field + " = ?", [self.buf_pattern] - - @classmethod - def value_match(cls, pattern: bytes, value: Any) -> bool: - return pattern == value - - class NumericQuery(FieldQuery[str]): """Matches numeric fields. A syntax using Ruby-style range ellipses (``..``) lets users specify one- or two-sided ranges. For example, diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 75f04cf5a..441224b6b 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -26,7 +26,8 @@ from typing import TYPE_CHECKING, Callable, Iterable, Sequence import mediafile -from beets import autotag, config, dbcore, library, plugins, util +from beets import autotag, config, library, plugins, util +from beets.dbcore.query import PathQuery from .state import ImportState @@ -520,9 +521,7 @@ class ImportTask(BaseImportTask): ) replaced_album_ids = set() for item in self.imported_items(): - dup_items = list( - lib.items(query=dbcore.query.BytesQuery("path", item.path)) - ) + dup_items = list(lib.items(query=PathQuery("path", item.path))) self.replaced_items[item] = dup_items for dup_item in dup_items: if ( diff --git a/beets/library/__init__.py b/beets/library/__init__.py new file mode 100644 index 000000000..286b84189 --- /dev/null +++ b/beets/library/__init__.py @@ -0,0 +1,16 @@ +from .exceptions import FileOperationError, ReadError, WriteError +from .library import Library +from .models import Album, Item, LibModel +from .queries import parse_query_parts, parse_query_string + +__all__ = [ + "Library", + "LibModel", + "Album", + "Item", + "parse_query_parts", + "parse_query_string", + "FileOperationError", + "ReadError", + "WriteError", +] diff --git a/beets/library/exceptions.py b/beets/library/exceptions.py new file mode 100644 index 000000000..7f117a2fe --- /dev/null +++ b/beets/library/exceptions.py @@ -0,0 +1,38 @@ +from beets import util + + +class FileOperationError(Exception): + """Indicate an error when interacting with a file on disk. + + Possibilities include an unsupported media type, a permissions + error, and an unhandled Mutagen exception. + """ + + def __init__(self, path, reason): + """Create an exception describing an operation on the file at + `path` with the underlying (chained) exception `reason`. + """ + super().__init__(path, reason) + self.path = path + self.reason = reason + + def __str__(self): + """Get a string representing the error. + + Describe both the underlying reason and the file path in question. + """ + return f"{util.displayable_path(self.path)}: {self.reason}" + + +class ReadError(FileOperationError): + """An error while reading a file (i.e. in `Item.read`).""" + + def __str__(self): + return "error reading " + str(super()) + + +class WriteError(FileOperationError): + """An error while writing a file (i.e. in `Item.write`).""" + + def __str__(self): + return "error writing " + str(super()) diff --git a/beets/library/library.py b/beets/library/library.py new file mode 100644 index 000000000..7370f7ecd --- /dev/null +++ b/beets/library/library.py @@ -0,0 +1,148 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import platformdirs + +import beets +from beets import dbcore +from beets.util import normpath + +from .models import Album, Item +from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string + +if TYPE_CHECKING: + from beets.dbcore import Results + + +class Library(dbcore.Database): + """A database of music containing songs and albums.""" + + _models = (Item, Album) + + def __init__( + self, + path="library.blb", + directory: str | None = None, + path_formats=((PF_KEY_DEFAULT, "$artist/$album/$track $title"),), + replacements=None, + ): + timeout = beets.config["timeout"].as_number() + super().__init__(path, timeout=timeout) + + self.directory = normpath(directory or platformdirs.user_music_path()) + + self.path_formats = path_formats + self.replacements = replacements + + # Used for template substitution performance. + self._memotable: dict[tuple[str, ...], str] = {} + + # Adding objects to the database. + + def add(self, obj): + """Add the :class:`Item` or :class:`Album` object to the library + database. + + Return the object's new id. + """ + obj.add(self) + self._memotable = {} + return obj.id + + def add_album(self, items): + """Create a new album consisting of a list of items. + + The items are added to the database if they don't yet have an + ID. Return a new :class:`Album` object. The list items must not + be empty. + """ + if not items: + raise ValueError("need at least one item") + + # Create the album structure using metadata from the first item. + values = {key: items[0][key] for key in Album.item_keys} + album = Album(self, **values) + + # Add the album structure and set the items' album_id fields. + # Store or add the items. + with self.transaction(): + album.add(self) + for item in items: + item.album_id = album.id + if item.id is None: + item.add(self) + else: + item.store() + + return album + + # Querying. + + def _fetch(self, model_cls, query, sort=None): + """Parse a query and fetch. + + If an order specification is present in the query string + the `sort` argument is ignored. + """ + # Parse the query, if necessary. + try: + parsed_sort = None + if isinstance(query, str): + query, parsed_sort = parse_query_string(query, model_cls) + elif isinstance(query, (list, tuple)): + query, parsed_sort = parse_query_parts(query, model_cls) + except dbcore.query.InvalidQueryArgumentValueError as exc: + raise dbcore.InvalidQueryError(query, exc) + + # Any non-null sort specified by the parsed query overrides the + # provided sort. + if parsed_sort and not isinstance(parsed_sort, dbcore.query.NullSort): + sort = parsed_sort + + return super()._fetch(model_cls, query, sort) + + @staticmethod + def get_default_album_sort(): + """Get a :class:`Sort` object for albums from the config option.""" + return dbcore.sort_from_strings( + Album, beets.config["sort_album"].as_str_seq() + ) + + @staticmethod + def get_default_item_sort(): + """Get a :class:`Sort` object for items from the config option.""" + return dbcore.sort_from_strings( + Item, beets.config["sort_item"].as_str_seq() + ) + + def albums(self, query=None, sort=None) -> Results[Album]: + """Get :class:`Album` objects matching the query.""" + return self._fetch(Album, query, sort or self.get_default_album_sort()) + + def items(self, query=None, sort=None) -> Results[Item]: + """Get :class:`Item` objects matching the query.""" + return self._fetch(Item, query, sort or self.get_default_item_sort()) + + # Convenience accessors. + + def get_item(self, id): + """Fetch a :class:`Item` by its ID. + + Return `None` if no match is found. + """ + return self._get(Item, id) + + def get_album(self, item_or_id): + """Given an album ID or an item associated with an album, return + a :class:`Album` object for the album. + + If no such album exists, return `None`. + """ + if isinstance(item_or_id, int): + album_id = item_or_id + else: + album_id = item_or_id.album_id + if album_id is None: + return None + return self._get(Album, album_id) diff --git a/beets/library.py b/beets/library/models.py similarity index 86% rename from beets/library.py rename to beets/library/models.py index 9223b3209..68c80b934 100644 --- a/beets/library.py +++ b/beets/library/models.py @@ -1,23 +1,6 @@ -# This file is part of beets. -# Copyright 2016, Adrian Sampson. -# -# Permission is hereby granted, free of charge, to any person obtaining -# a copy of this software and associated documentation files (the -# "Software"), to deal in the Software without restriction, including -# without limitation the rights to use, copy, modify, merge, publish, -# distribute, sublicense, and/or sell copies of the Software, and to -# permit persons to whom the Software is furnished to do so, subject to -# the following conditions: -# -# The above copyright notice and this permission notice shall be -# included in all copies or substantial portions of the Software. - -"""The core data store and collection logic for beets.""" - from __future__ import annotations import os -import shlex import string import sys import time @@ -26,12 +9,11 @@ from functools import cached_property from pathlib import Path from typing import TYPE_CHECKING -import platformdirs from mediafile import MediaFile, UnreadableFileError import beets from beets import dbcore, logging, plugins, util -from beets.dbcore import Results, types +from beets.dbcore import types from beets.util import ( MoveOperation, bytestring_path, @@ -42,57 +24,16 @@ from beets.util import ( ) from beets.util.functemplate import Template, template +from .exceptions import FileOperationError, ReadError, WriteError +from .queries import PF_KEY_DEFAULT, parse_query_string + if TYPE_CHECKING: - from .dbcore.query import FieldQuery, FieldQueryType + from ..dbcore.query import FieldQuery, FieldQueryType + from .library import Library # noqa: F401 log = logging.getLogger("beets") -# Special path format key. -PF_KEY_DEFAULT = "default" - - -# Exceptions. -class FileOperationError(Exception): - """Indicate an error when interacting with a file on disk. - - Possibilities include an unsupported media type, a permissions - error, and an unhandled Mutagen exception. - """ - - def __init__(self, path, reason): - """Create an exception describing an operation on the file at - `path` with the underlying (chained) exception `reason`. - """ - super().__init__(path, reason) - self.path = path - self.reason = reason - - def __str__(self): - """Get a string representing the error. - - Describe both the underlying reason and the file path in question. - """ - return f"{util.displayable_path(self.path)}: {self.reason}" - - -class ReadError(FileOperationError): - """An error while reading a file (i.e. in `Item.read`).""" - - def __str__(self): - return "error reading " + str(super()) - - -class WriteError(FileOperationError): - """An error while writing a file (i.e. in `Item.write`).""" - - def __str__(self): - return "error writing " + str(super()) - - -# Item and Album model classes. - - class LibModel(dbcore.Model["Library"]): """Shared concrete functionality for Items and Albums.""" @@ -104,6 +45,11 @@ class LibModel(dbcore.Model["Library"]): def writable_media_fields(cls) -> set[str]: return set(MediaFile.fields()) & cls._fields.keys() + @property + def filepath(self) -> Path: + """The path to the entity as pathlib.Path.""" + return Path(os.fsdecode(self.path)) + def _template_funcs(self): funcs = DefaultTemplateFunctions(self, self._db).functions() funcs.update(plugins.template_funcs()) @@ -259,6 +205,407 @@ class FormattedItemMapping(dbcore.db.FormattedMapping): return len(self.all_keys) +class Album(LibModel): + """Provide access to information about albums stored in a + library. + + Reflects the library's "albums" table, including album art. + """ + + artpath: bytes + + _table = "albums" + _flex_table = "album_attributes" + _always_dirty = True + _fields = { + "id": types.PRIMARY_ID, + "artpath": types.NullPathType(), + "added": types.DATE, + "albumartist": types.STRING, + "albumartist_sort": types.STRING, + "albumartist_credit": types.STRING, + "albumartists": types.MULTI_VALUE_DSV, + "albumartists_sort": types.MULTI_VALUE_DSV, + "albumartists_credit": types.MULTI_VALUE_DSV, + "album": types.STRING, + "genre": types.STRING, + "style": types.STRING, + "discogs_albumid": types.INTEGER, + "discogs_artistid": types.INTEGER, + "discogs_labelid": types.INTEGER, + "year": types.PaddedInt(4), + "month": types.PaddedInt(2), + "day": types.PaddedInt(2), + "disctotal": types.PaddedInt(2), + "comp": types.BOOLEAN, + "mb_albumid": types.STRING, + "mb_albumartistid": types.STRING, + "mb_albumartistids": types.MULTI_VALUE_DSV, + "albumtype": types.STRING, + "albumtypes": types.SEMICOLON_SPACE_DSV, + "label": types.STRING, + "barcode": types.STRING, + "mb_releasegroupid": types.STRING, + "release_group_title": types.STRING, + "asin": types.STRING, + "catalognum": types.STRING, + "script": types.STRING, + "language": types.STRING, + "country": types.STRING, + "albumstatus": types.STRING, + "albumdisambig": types.STRING, + "releasegroupdisambig": types.STRING, + "rg_album_gain": types.NULL_FLOAT, + "rg_album_peak": types.NULL_FLOAT, + "r128_album_gain": types.NULL_FLOAT, + "original_year": types.PaddedInt(4), + "original_month": types.PaddedInt(2), + "original_day": types.PaddedInt(2), + } + + _search_fields = ("album", "albumartist", "genre") + + _types = { + "path": types.PathType(), + "data_source": types.STRING, + } + + _sorts = { + "albumartist": dbcore.query.SmartArtistSort, + "artist": dbcore.query.SmartArtistSort, + } + + # List of keys that are set on an album's items. + item_keys = [ + "added", + "albumartist", + "albumartists", + "albumartist_sort", + "albumartists_sort", + "albumartist_credit", + "albumartists_credit", + "album", + "genre", + "style", + "discogs_albumid", + "discogs_artistid", + "discogs_labelid", + "year", + "month", + "day", + "disctotal", + "comp", + "mb_albumid", + "mb_albumartistid", + "mb_albumartistids", + "albumtype", + "albumtypes", + "label", + "barcode", + "mb_releasegroupid", + "asin", + "catalognum", + "script", + "language", + "country", + "albumstatus", + "albumdisambig", + "releasegroupdisambig", + "release_group_title", + "rg_album_gain", + "rg_album_peak", + "r128_album_gain", + "original_year", + "original_month", + "original_day", + ] + + _format_config_key = "format_album" + + @cached_classproperty + def _relation(cls) -> type[Item]: + return Item + + @cached_classproperty + def relation_join(cls) -> str: + """Return FROM clause which joins on related album items. + + Use LEFT join to select all albums, including those that do not have + any items. + """ + return ( + f"LEFT JOIN {cls._relation._table} " + f"ON {cls._table}.id = {cls._relation._table}.album_id" + ) + + @property + def art_filepath(self) -> Path | None: + """The path to album's cover picture as pathlib.Path.""" + return Path(os.fsdecode(self.artpath)) if self.artpath else None + + @classmethod + def _getters(cls): + # In addition to plugin-provided computed fields, also expose + # the album's directory as `path`. + getters = plugins.album_field_getters() + getters["path"] = Album.item_dir + getters["albumtotal"] = Album._albumtotal + return getters + + def items(self): + """Return an iterable over the items associated with this + album. + + This method conflicts with :meth:`LibModel.items`, which is + inherited from :meth:`beets.dbcore.Model.items`. + Since :meth:`Album.items` predates these methods, and is + likely to be used by plugins, we keep this interface as-is. + """ + return self._db.items(dbcore.MatchQuery("album_id", self.id)) + + def remove(self, delete=False, with_items=True): + """Remove this album and all its associated items from the + library. + + If delete, then the items' files are also deleted from disk, + along with any album art. The directories containing the album are + also removed (recursively) if empty. + + Set with_items to False to avoid removing the album's items. + """ + super().remove() + + # Send a 'album_removed' signal to plugins + plugins.send("album_removed", album=self) + + # Delete art file. + if delete: + artpath = self.artpath + if artpath: + util.remove(artpath) + + # Remove (and possibly delete) the constituent items. + if with_items: + for item in self.items(): + item.remove(delete, False) + + def move_art(self, operation=MoveOperation.MOVE): + """Move, copy, link or hardlink (depending on `operation`) any + existing album art so that it remains in the same directory as + the items. + + `operation` should be an instance of `util.MoveOperation`. + """ + old_art = self.artpath + if not old_art: + return + + if not os.path.exists(syspath(old_art)): + log.error( + "removing reference to missing album art file {}", + util.displayable_path(old_art), + ) + self.artpath = None + return + + new_art = self.art_destination(old_art) + if new_art == old_art: + return + + new_art = util.unique_path(new_art) + log.debug( + "moving album art {0} to {1}", + util.displayable_path(old_art), + util.displayable_path(new_art), + ) + if operation == MoveOperation.MOVE: + util.move(old_art, new_art) + util.prune_dirs(os.path.dirname(old_art), self._db.directory) + elif operation == MoveOperation.COPY: + util.copy(old_art, new_art) + elif operation == MoveOperation.LINK: + util.link(old_art, new_art) + elif operation == MoveOperation.HARDLINK: + util.hardlink(old_art, new_art) + elif operation == MoveOperation.REFLINK: + util.reflink(old_art, new_art, fallback=False) + elif operation == MoveOperation.REFLINK_AUTO: + util.reflink(old_art, new_art, fallback=True) + else: + assert False, "unknown MoveOperation" + self.artpath = new_art + + def move(self, operation=MoveOperation.MOVE, basedir=None, store=True): + """Move, copy, link or hardlink (depending on `operation`) + all items to their destination. Any album art moves along with them. + + `basedir` overrides the library base directory for the destination. + + `operation` should be an instance of `util.MoveOperation`. + + By default, the album is stored to the database, persisting any + modifications to its metadata. If `store` is `False` however, + the album is not stored automatically, and it will have to be manually + stored after invoking this method. + """ + basedir = basedir or self._db.directory + + # Ensure new metadata is available to items for destination + # computation. + if store: + self.store() + + # Move items. + items = list(self.items()) + for item in items: + item.move(operation, basedir=basedir, with_album=False, store=store) + + # Move art. + self.move_art(operation) + if store: + self.store() + + def item_dir(self): + """Return the directory containing the album's first item, + provided that such an item exists. + """ + item = self.items().get() + if not item: + raise ValueError("empty album for album id %d" % self.id) + return os.path.dirname(item.path) + + def _albumtotal(self): + """Return the total number of tracks on all discs on the album.""" + if self.disctotal == 1 or not beets.config["per_disc_numbering"]: + return self.items()[0].tracktotal + + counted = [] + total = 0 + + for item in self.items(): + if item.disc in counted: + continue + + total += item.tracktotal + counted.append(item.disc) + + if len(counted) == self.disctotal: + break + + return total + + def art_destination(self, image, item_dir=None): + """Return a path to the destination for the album art image + for the album. + + `image` is the path of the image that will be + moved there (used for its extension). + + The path construction uses the existing path of the album's + items, so the album must contain at least one item or + item_dir must be provided. + """ + image = bytestring_path(image) + item_dir = item_dir or self.item_dir() + + filename_tmpl = template(beets.config["art_filename"].as_str()) + subpath = self.evaluate_template(filename_tmpl, True) + if beets.config["asciify_paths"]: + subpath = util.asciify_path( + subpath, beets.config["path_sep_replace"].as_str() + ) + subpath = util.sanitize_path( + subpath, replacements=self._db.replacements + ) + subpath = bytestring_path(subpath) + + _, ext = os.path.splitext(image) + dest = os.path.join(item_dir, subpath + ext) + + return bytestring_path(dest) + + def set_art(self, path, copy=True): + """Set the album's cover art to the image at the given path. + + The image is copied (or moved) into place, replacing any + existing art. + + Send an 'art_set' event with `self` as the sole argument. + """ + path = bytestring_path(path) + oldart = self.artpath + artdest = self.art_destination(path) + + if oldart and samefile(path, oldart): + # Art already set. + return + elif samefile(path, artdest): + # Art already in place. + self.artpath = path + return + + # Normal operation. + if oldart == artdest: + util.remove(oldart) + artdest = util.unique_path(artdest) + if copy: + util.copy(path, artdest) + else: + util.move(path, artdest) + self.artpath = artdest + + plugins.send("art_set", album=self) + + def store(self, fields=None, inherit=True): + """Update the database with the album information. + + `fields` represents the fields to be stored. If not specified, + all fields will be. + + The album's tracks are also updated when the `inherit` flag is enabled. + This applies to fixed attributes as well as flexible ones. The `id` + attribute of the album will never be inherited. + """ + # Get modified track fields. + track_updates = {} + track_deletes = set() + for key in self._dirty: + if inherit: + if key in self.item_keys: # is a fixed attribute + track_updates[key] = self[key] + elif key not in self: # is a fixed or a flexible attribute + track_deletes.add(key) + elif key != "id": # is a flexible attribute + track_updates[key] = self[key] + + with self._db.transaction(): + super().store(fields) + if track_updates: + for item in self.items(): + for key, value in track_updates.items(): + item[key] = value + item.store() + if track_deletes: + for item in self.items(): + for key in track_deletes: + if key in item: + del item[key] + item.store() + + def try_sync(self, write, move, inherit=True): + """Synchronize the album and its items with the database. + Optionally, also write any new tags into the files and update + their paths. + + `write` indicates whether to write tags to the item files, and + `move` controls whether files (both audio and album art) are + moved. + """ + self.store(inherit=inherit) + for item in self.items(): + item.try_sync(write, move) + + class Item(LibModel): """Represent a song or track.""" @@ -413,11 +760,6 @@ class Item(LibModel): f"ON {cls._table}.album_id = {cls._relation._table}.id" ) - @property - def filepath(self) -> Path: - """The path to the item's file as pathlib.Path.""" - return Path(os.fsdecode(self.path)) - @property def _cached_album(self): """The Album object that this item belongs to, if any, or @@ -898,589 +1240,6 @@ class Item(LibModel): return normpath(os.path.join(basedir, lib_path_bytes)) -class Album(LibModel): - """Provide access to information about albums stored in a - library. - - Reflects the library's "albums" table, including album art. - """ - - _table = "albums" - _flex_table = "album_attributes" - _always_dirty = True - _fields = { - "id": types.PRIMARY_ID, - "artpath": types.NullPathType(), - "added": types.DATE, - "albumartist": types.STRING, - "albumartist_sort": types.STRING, - "albumartist_credit": types.STRING, - "albumartists": types.MULTI_VALUE_DSV, - "albumartists_sort": types.MULTI_VALUE_DSV, - "albumartists_credit": types.MULTI_VALUE_DSV, - "album": types.STRING, - "genre": types.STRING, - "style": types.STRING, - "discogs_albumid": types.INTEGER, - "discogs_artistid": types.INTEGER, - "discogs_labelid": types.INTEGER, - "year": types.PaddedInt(4), - "month": types.PaddedInt(2), - "day": types.PaddedInt(2), - "disctotal": types.PaddedInt(2), - "comp": types.BOOLEAN, - "mb_albumid": types.STRING, - "mb_albumartistid": types.STRING, - "mb_albumartistids": types.MULTI_VALUE_DSV, - "albumtype": types.STRING, - "albumtypes": types.SEMICOLON_SPACE_DSV, - "label": types.STRING, - "barcode": types.STRING, - "mb_releasegroupid": types.STRING, - "release_group_title": types.STRING, - "asin": types.STRING, - "catalognum": types.STRING, - "script": types.STRING, - "language": types.STRING, - "country": types.STRING, - "albumstatus": types.STRING, - "albumdisambig": types.STRING, - "releasegroupdisambig": types.STRING, - "rg_album_gain": types.NULL_FLOAT, - "rg_album_peak": types.NULL_FLOAT, - "r128_album_gain": types.NULL_FLOAT, - "original_year": types.PaddedInt(4), - "original_month": types.PaddedInt(2), - "original_day": types.PaddedInt(2), - } - - _search_fields = ("album", "albumartist", "genre") - - _types = { - "path": types.PathType(), - "data_source": types.STRING, - } - - _sorts = { - "albumartist": dbcore.query.SmartArtistSort, - "artist": dbcore.query.SmartArtistSort, - } - - # List of keys that are set on an album's items. - item_keys = [ - "added", - "albumartist", - "albumartists", - "albumartist_sort", - "albumartists_sort", - "albumartist_credit", - "albumartists_credit", - "album", - "genre", - "style", - "discogs_albumid", - "discogs_artistid", - "discogs_labelid", - "year", - "month", - "day", - "disctotal", - "comp", - "mb_albumid", - "mb_albumartistid", - "mb_albumartistids", - "albumtype", - "albumtypes", - "label", - "barcode", - "mb_releasegroupid", - "asin", - "catalognum", - "script", - "language", - "country", - "albumstatus", - "albumdisambig", - "releasegroupdisambig", - "release_group_title", - "rg_album_gain", - "rg_album_peak", - "r128_album_gain", - "original_year", - "original_month", - "original_day", - ] - - _format_config_key = "format_album" - - @cached_classproperty - def _relation(cls) -> type[Item]: - return Item - - @cached_classproperty - def relation_join(cls) -> str: - """Return FROM clause which joins on related album items. - - Use LEFT join to select all albums, including those that do not have - any items. - """ - return ( - f"LEFT JOIN {cls._relation._table} " - f"ON {cls._table}.id = {cls._relation._table}.album_id" - ) - - @classmethod - def _getters(cls): - # In addition to plugin-provided computed fields, also expose - # the album's directory as `path`. - getters = plugins.album_field_getters() - getters["path"] = Album.item_dir - getters["albumtotal"] = Album._albumtotal - return getters - - def items(self): - """Return an iterable over the items associated with this - album. - - This method conflicts with :meth:`LibModel.items`, which is - inherited from :meth:`beets.dbcore.Model.items`. - Since :meth:`Album.items` predates these methods, and is - likely to be used by plugins, we keep this interface as-is. - """ - return self._db.items(dbcore.MatchQuery("album_id", self.id)) - - def remove(self, delete=False, with_items=True): - """Remove this album and all its associated items from the - library. - - If delete, then the items' files are also deleted from disk, - along with any album art. The directories containing the album are - also removed (recursively) if empty. - - Set with_items to False to avoid removing the album's items. - """ - super().remove() - - # Send a 'album_removed' signal to plugins - plugins.send("album_removed", album=self) - - # Delete art file. - if delete: - artpath = self.artpath - if artpath: - util.remove(artpath) - - # Remove (and possibly delete) the constituent items. - if with_items: - for item in self.items(): - item.remove(delete, False) - - def move_art(self, operation=MoveOperation.MOVE): - """Move, copy, link or hardlink (depending on `operation`) any - existing album art so that it remains in the same directory as - the items. - - `operation` should be an instance of `util.MoveOperation`. - """ - old_art = self.artpath - if not old_art: - return - - if not os.path.exists(syspath(old_art)): - log.error( - "removing reference to missing album art file {}", - util.displayable_path(old_art), - ) - self.artpath = None - return - - new_art = self.art_destination(old_art) - if new_art == old_art: - return - - new_art = util.unique_path(new_art) - log.debug( - "moving album art {0} to {1}", - util.displayable_path(old_art), - util.displayable_path(new_art), - ) - if operation == MoveOperation.MOVE: - util.move(old_art, new_art) - util.prune_dirs(os.path.dirname(old_art), self._db.directory) - elif operation == MoveOperation.COPY: - util.copy(old_art, new_art) - elif operation == MoveOperation.LINK: - util.link(old_art, new_art) - elif operation == MoveOperation.HARDLINK: - util.hardlink(old_art, new_art) - elif operation == MoveOperation.REFLINK: - util.reflink(old_art, new_art, fallback=False) - elif operation == MoveOperation.REFLINK_AUTO: - util.reflink(old_art, new_art, fallback=True) - else: - assert False, "unknown MoveOperation" - self.artpath = new_art - - def move(self, operation=MoveOperation.MOVE, basedir=None, store=True): - """Move, copy, link or hardlink (depending on `operation`) - all items to their destination. Any album art moves along with them. - - `basedir` overrides the library base directory for the destination. - - `operation` should be an instance of `util.MoveOperation`. - - By default, the album is stored to the database, persisting any - modifications to its metadata. If `store` is `False` however, - the album is not stored automatically, and it will have to be manually - stored after invoking this method. - """ - basedir = basedir or self._db.directory - - # Ensure new metadata is available to items for destination - # computation. - if store: - self.store() - - # Move items. - items = list(self.items()) - for item in items: - item.move(operation, basedir=basedir, with_album=False, store=store) - - # Move art. - self.move_art(operation) - if store: - self.store() - - def item_dir(self): - """Return the directory containing the album's first item, - provided that such an item exists. - """ - item = self.items().get() - if not item: - raise ValueError("empty album for album id %d" % self.id) - return os.path.dirname(item.path) - - def _albumtotal(self): - """Return the total number of tracks on all discs on the album.""" - if self.disctotal == 1 or not beets.config["per_disc_numbering"]: - return self.items()[0].tracktotal - - counted = [] - total = 0 - - for item in self.items(): - if item.disc in counted: - continue - - total += item.tracktotal - counted.append(item.disc) - - if len(counted) == self.disctotal: - break - - return total - - def art_destination(self, image, item_dir=None): - """Return a path to the destination for the album art image - for the album. - - `image` is the path of the image that will be - moved there (used for its extension). - - The path construction uses the existing path of the album's - items, so the album must contain at least one item or - item_dir must be provided. - """ - image = bytestring_path(image) - item_dir = item_dir or self.item_dir() - - filename_tmpl = template(beets.config["art_filename"].as_str()) - subpath = self.evaluate_template(filename_tmpl, True) - if beets.config["asciify_paths"]: - subpath = util.asciify_path( - subpath, beets.config["path_sep_replace"].as_str() - ) - subpath = util.sanitize_path( - subpath, replacements=self._db.replacements - ) - subpath = bytestring_path(subpath) - - _, ext = os.path.splitext(image) - dest = os.path.join(item_dir, subpath + ext) - - return bytestring_path(dest) - - def set_art(self, path, copy=True): - """Set the album's cover art to the image at the given path. - - The image is copied (or moved) into place, replacing any - existing art. - - Send an 'art_set' event with `self` as the sole argument. - """ - path = bytestring_path(path) - oldart = self.artpath - artdest = self.art_destination(path) - - if oldart and samefile(path, oldart): - # Art already set. - return - elif samefile(path, artdest): - # Art already in place. - self.artpath = path - return - - # Normal operation. - if oldart == artdest: - util.remove(oldart) - artdest = util.unique_path(artdest) - if copy: - util.copy(path, artdest) - else: - util.move(path, artdest) - self.artpath = artdest - - plugins.send("art_set", album=self) - - def store(self, fields=None, inherit=True): - """Update the database with the album information. - - `fields` represents the fields to be stored. If not specified, - all fields will be. - - The album's tracks are also updated when the `inherit` flag is enabled. - This applies to fixed attributes as well as flexible ones. The `id` - attribute of the album will never be inherited. - """ - # Get modified track fields. - track_updates = {} - track_deletes = set() - for key in self._dirty: - if inherit: - if key in self.item_keys: # is a fixed attribute - track_updates[key] = self[key] - elif key not in self: # is a fixed or a flexible attribute - track_deletes.add(key) - elif key != "id": # is a flexible attribute - track_updates[key] = self[key] - - with self._db.transaction(): - super().store(fields) - if track_updates: - for item in self.items(): - for key, value in track_updates.items(): - item[key] = value - item.store() - if track_deletes: - for item in self.items(): - for key in track_deletes: - if key in item: - del item[key] - item.store() - - def try_sync(self, write, move, inherit=True): - """Synchronize the album and its items with the database. - Optionally, also write any new tags into the files and update - their paths. - - `write` indicates whether to write tags to the item files, and - `move` controls whether files (both audio and album art) are - moved. - """ - self.store(inherit=inherit) - for item in self.items(): - item.try_sync(write, move) - - -# Query construction helpers. - - -def parse_query_parts(parts, model_cls): - """Given a beets query string as a list of components, return the - `Query` and `Sort` they represent. - - Like `dbcore.parse_sorted_query`, with beets query prefixes and - ensuring that implicit path queries are made explicit with 'path::' - """ - # Get query types and their prefix characters. - prefixes = { - ":": dbcore.query.RegexpQuery, - "=~": dbcore.query.StringQuery, - "=": dbcore.query.MatchQuery, - } - prefixes.update(plugins.queries()) - - # Special-case path-like queries, which are non-field queries - # containing path separators (/). - parts = [ - f"path:{s}" if dbcore.query.PathQuery.is_path_query(s) else s - for s in parts - ] - - case_insensitive = beets.config["sort_case_insensitive"].get(bool) - - query, sort = dbcore.parse_sorted_query( - model_cls, parts, prefixes, case_insensitive - ) - log.debug("Parsed query: {!r}", query) - log.debug("Parsed sort: {!r}", sort) - return query, sort - - -def parse_query_string(s, model_cls): - """Given a beets query string, return the `Query` and `Sort` they - represent. - - The string is split into components using shell-like syntax. - """ - message = f"Query is not unicode: {s!r}" - assert isinstance(s, str), message - try: - parts = shlex.split(s) - except ValueError as exc: - raise dbcore.InvalidQueryError(s, exc) - return parse_query_parts(parts, model_cls) - - -# The Library: interface to the database. - - -class Library(dbcore.Database): - """A database of music containing songs and albums.""" - - _models = (Item, Album) - - def __init__( - self, - path="library.blb", - directory: str | None = None, - path_formats=((PF_KEY_DEFAULT, "$artist/$album/$track $title"),), - replacements=None, - ): - timeout = beets.config["timeout"].as_number() - super().__init__(path, timeout=timeout) - - self.directory = normpath(directory or platformdirs.user_music_path()) - - self.path_formats = path_formats - self.replacements = replacements - - # Used for template substitution performance. - self._memotable: dict[tuple[str, ...], str] = {} - - # Adding objects to the database. - - def add(self, obj): - """Add the :class:`Item` or :class:`Album` object to the library - database. - - Return the object's new id. - """ - obj.add(self) - self._memotable = {} - return obj.id - - def add_album(self, items): - """Create a new album consisting of a list of items. - - The items are added to the database if they don't yet have an - ID. Return a new :class:`Album` object. The list items must not - be empty. - """ - if not items: - raise ValueError("need at least one item") - - # Create the album structure using metadata from the first item. - values = {key: items[0][key] for key in Album.item_keys} - album = Album(self, **values) - - # Add the album structure and set the items' album_id fields. - # Store or add the items. - with self.transaction(): - album.add(self) - for item in items: - item.album_id = album.id - if item.id is None: - item.add(self) - else: - item.store() - - return album - - # Querying. - - def _fetch(self, model_cls, query, sort=None): - """Parse a query and fetch. - - If an order specification is present in the query string - the `sort` argument is ignored. - """ - # Parse the query, if necessary. - try: - parsed_sort = None - if isinstance(query, str): - query, parsed_sort = parse_query_string(query, model_cls) - elif isinstance(query, (list, tuple)): - query, parsed_sort = parse_query_parts(query, model_cls) - except dbcore.query.InvalidQueryArgumentValueError as exc: - raise dbcore.InvalidQueryError(query, exc) - - # Any non-null sort specified by the parsed query overrides the - # provided sort. - if parsed_sort and not isinstance(parsed_sort, dbcore.query.NullSort): - sort = parsed_sort - - return super()._fetch(model_cls, query, sort) - - @staticmethod - def get_default_album_sort(): - """Get a :class:`Sort` object for albums from the config option.""" - return dbcore.sort_from_strings( - Album, beets.config["sort_album"].as_str_seq() - ) - - @staticmethod - def get_default_item_sort(): - """Get a :class:`Sort` object for items from the config option.""" - return dbcore.sort_from_strings( - Item, beets.config["sort_item"].as_str_seq() - ) - - def albums(self, query=None, sort=None) -> Results[Album]: - """Get :class:`Album` objects matching the query.""" - return self._fetch(Album, query, sort or self.get_default_album_sort()) - - def items(self, query=None, sort=None) -> Results[Item]: - """Get :class:`Item` objects matching the query.""" - return self._fetch(Item, query, sort or self.get_default_item_sort()) - - # Convenience accessors. - - def get_item(self, id): - """Fetch a :class:`Item` by its ID. - - Return `None` if no match is found. - """ - return self._get(Item, id) - - def get_album(self, item_or_id): - """Given an album ID or an item associated with an album, return - a :class:`Album` object for the album. - - If no such album exists, return `None`. - """ - if isinstance(item_or_id, int): - album_id = item_or_id - else: - album_id = item_or_id.album_id - if album_id is None: - return None - return self._get(Album, album_id) - - -# Default path template resources. - - def _int_arg(s): """Convert a string argument to an integer for use in a template function. diff --git a/beets/library/queries.py b/beets/library/queries.py new file mode 100644 index 000000000..7c9d688cd --- /dev/null +++ b/beets/library/queries.py @@ -0,0 +1,61 @@ +from __future__ import annotations + +import shlex + +import beets +from beets import dbcore, logging, plugins + +log = logging.getLogger("beets") + + +# Special path format key. +PF_KEY_DEFAULT = "default" + +# Query construction helpers. + + +def parse_query_parts(parts, model_cls): + """Given a beets query string as a list of components, return the + `Query` and `Sort` they represent. + + Like `dbcore.parse_sorted_query`, with beets query prefixes and + ensuring that implicit path queries are made explicit with 'path::' + """ + # Get query types and their prefix characters. + prefixes = { + ":": dbcore.query.RegexpQuery, + "=~": dbcore.query.StringQuery, + "=": dbcore.query.MatchQuery, + } + prefixes.update(plugins.queries()) + + # Special-case path-like queries, which are non-field queries + # containing path separators (/). + parts = [ + f"path:{s}" if dbcore.query.PathQuery.is_path_query(s) else s + for s in parts + ] + + case_insensitive = beets.config["sort_case_insensitive"].get(bool) + + query, sort = dbcore.parse_sorted_query( + model_cls, parts, prefixes, case_insensitive + ) + log.debug("Parsed query: {!r}", query) + log.debug("Parsed sort: {!r}", sort) + return query, sort + + +def parse_query_string(s, model_cls): + """Given a beets query string, return the `Query` and `Sort` they + represent. + + The string is split into components using shell-like syntax. + """ + message = f"Query is not unicode: {s!r}" + assert isinstance(s, str), message + try: + parts = shlex.split(s) + except ValueError as exc: + raise dbcore.InvalidQueryError(s, exc) + return parse_query_parts(parts, model_cls) diff --git a/beets/plugins.py b/beets/plugins.py index 1ae672e20..983d15402 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -28,6 +28,7 @@ from typing import ( Any, Callable, Generic, + Literal, Sequence, TypedDict, TypeVar, @@ -737,8 +738,8 @@ class MetadataSourcePlugin(Generic[R], BeetsPlugin, metaclass=abc.ABCMeta): @abc.abstractmethod def _search_api( self, - query_type: str, - filters: dict[str, str] | None, + query_type: Literal["album", "track"], + filters: dict[str, str], keywords: str = "", ) -> Sequence[R]: raise NotImplementedError diff --git a/beets/test/_common.py b/beets/test/_common.py index da81a587c..d70f9ec80 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -111,34 +111,6 @@ def import_session(lib=None, loghandler=None, paths=[], query=[], cli=False): return cls(lib, loghandler, paths, query) -class Assertions: - """A mixin with additional unit test assertions.""" - - def assertExists(self, path): - assert os.path.exists(syspath(path)), f"file does not exist: {path!r}" - - def assertNotExists(self, path): - assert not os.path.exists(syspath(path)), f"file exists: {path!r}" - - def assertIsFile(self, path): - self.assertExists(path) - assert os.path.isfile(syspath(path)), ( - "path exists, but is not a regular file: {!r}".format(path) - ) - - def assertIsDir(self, path): - self.assertExists(path) - assert os.path.isdir(syspath(path)), ( - "path exists, but is not a directory: {!r}".format(path) - ) - - def assert_equal_path(self, a, b): - """Check that two paths are equal.""" - a_bytes, b_bytes = util.normpath(a), util.normpath(b) - - assert a_bytes == b_bytes, f"{a_bytes=} != {b_bytes=}" - - # Mock I/O. diff --git a/beets/test/helper.py b/beets/test/helper.py index b86db5b23..db753a760 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -163,15 +163,49 @@ NEEDS_REFLINK = unittest.skipUnless( ) -class TestHelper(_common.Assertions, ConfigMixin): +class IOMixin: + @cached_property + def io(self) -> _common.DummyIO: + return _common.DummyIO() + + def setUp(self): + super().setUp() + self.io.install() + + def tearDown(self): + super().tearDown() + self.io.restore() + + +class TestHelper(ConfigMixin): """Helper mixin for high-level cli and plugin tests. This mixin provides methods to isolate beets' global state provide fixtures. """ + resource_path = Path(os.fsdecode(_common.RSRC)) / "full.mp3" + db_on_disk: ClassVar[bool] = False + @cached_property + def temp_dir_path(self) -> Path: + return Path(self.create_temp_dir()) + + @cached_property + def temp_dir(self) -> bytes: + return util.bytestring_path(self.temp_dir_path) + + @cached_property + def lib_path(self) -> Path: + lib_path = self.temp_dir_path / "libdir" + lib_path.mkdir(exist_ok=True) + return lib_path + + @cached_property + def libdir(self) -> bytes: + return bytestring_path(self.lib_path) + # TODO automate teardown through hook registration def setup_beets(self): @@ -194,8 +228,7 @@ class TestHelper(_common.Assertions, ConfigMixin): Make sure you call ``teardown_beets()`` afterwards. """ - self.create_temp_dir() - temp_dir_str = os.fsdecode(self.temp_dir) + temp_dir_str = str(self.temp_dir_path) self.env_patcher = patch.dict( "os.environ", { @@ -205,9 +238,7 @@ class TestHelper(_common.Assertions, ConfigMixin): ) self.env_patcher.start() - self.libdir = os.path.join(self.temp_dir, b"libdir") - os.mkdir(syspath(self.libdir)) - self.config["directory"] = os.fsdecode(self.libdir) + self.config["directory"] = str(self.lib_path) if self.db_on_disk: dbpath = util.bytestring_path(self.config["library"].as_filename()) @@ -215,12 +246,8 @@ class TestHelper(_common.Assertions, ConfigMixin): dbpath = ":memory:" self.lib = Library(dbpath, self.libdir) - # Initialize, but don't install, a DummyIO. - self.io = _common.DummyIO() - def teardown_beets(self): self.env_patcher.stop() - self.io.restore() self.lib._close() self.remove_temp_dir() @@ -384,16 +411,12 @@ class TestHelper(_common.Assertions, ConfigMixin): # Safe file operations - def create_temp_dir(self, **kwargs): - """Create a temporary directory and assign it into - `self.temp_dir`. Call `remove_temp_dir` later to delete it. - """ - temp_dir = mkdtemp(**kwargs) - self.temp_dir = util.bytestring_path(temp_dir) + def create_temp_dir(self, **kwargs) -> str: + return mkdtemp(**kwargs) def remove_temp_dir(self): """Delete the temporary directory created by `create_temp_dir`.""" - shutil.rmtree(syspath(self.temp_dir)) + shutil.rmtree(self.temp_dir_path) def touch(self, path, dir=None, content=""): """Create a file at `path` with given content. @@ -514,7 +537,6 @@ class ImportHelper(TestHelper): autotagging library and several assertions for the library. """ - resource_path = syspath(os.path.join(_common.RSRC, b"full.mp3")) default_import_config = { "autotag": True, "copy": True, @@ -531,7 +553,7 @@ class ImportHelper(TestHelper): @cached_property def import_path(self) -> Path: - import_path = Path(os.fsdecode(self.temp_dir)) / "import" + import_path = self.temp_dir_path / "import" import_path.mkdir(exist_ok=True) return import_path @@ -599,7 +621,7 @@ class ImportHelper(TestHelper): ] def prepare_albums_for_import(self, count: int = 1) -> None: - album_dirs = Path(os.fsdecode(self.import_dir)).glob("album_*") + album_dirs = self.import_path.glob("album_*") base_idx = int(str(max(album_dirs, default="0")).split("_")[-1]) + 1 for album_id in range(base_idx, count + base_idx): @@ -623,21 +645,6 @@ class ImportHelper(TestHelper): def setup_singleton_importer(self, **kwargs) -> ImportSession: return self.setup_importer(singletons=True, **kwargs) - def assert_file_in_lib(self, *segments): - """Join the ``segments`` and assert that this path exists in the - library directory. - """ - self.assertExists(os.path.join(self.libdir, *segments)) - - def assert_file_not_in_lib(self, *segments): - """Join the ``segments`` and assert that this path does not - exist in the library directory. - """ - self.assertNotExists(os.path.join(self.libdir, *segments)) - - def assert_lib_dir_empty(self): - assert not os.listdir(syspath(self.libdir)) - class AsIsImporterMixin: def setUp(self): @@ -759,7 +766,7 @@ class TerminalImportSessionFixture(TerminalImportSession): self._add_choice_input() -class TerminalImportMixin(ImportHelper): +class TerminalImportMixin(IOMixin, ImportHelper): """Provides_a terminal importer for the import session.""" io: _common.DummyIO diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index b7033e41b..74dee550c 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -104,30 +104,15 @@ def _stream_encoding(stream, default="utf-8"): return stream.encoding or default -def decargs(arglist): - """Given a list of command-line argument bytestrings, attempts to - decode them to Unicode strings when running under Python 2. - """ - return arglist - - -def print_(*strings, **kwargs): +def print_(*strings: str, end: str = "\n") -> None: """Like print, but rather than raising an error when a character is not in the terminal's encoding's character set, just silently replaces it. - The arguments must be Unicode strings: `unicode` on Python 2; `str` on - Python 3. - The `end` keyword argument behaves similarly to the built-in `print` (it defaults to a newline). """ - if not strings: - strings = [""] - assert isinstance(strings[0], str) - - txt = " ".join(strings) - txt += kwargs.get("end", "\n") + txt = " ".join(strings or ("",)) + end # Encode the string and write it to stdout. # On Python 3, sys.stdout expects text strings and uses the @@ -1308,14 +1293,9 @@ class CommonOptionsParser(optparse.OptionParser): setattr(parser.values, option.dest, True) # Use the explicitly specified format, or the string from the option. - if fmt: - value = fmt - elif value: - (value,) = decargs([value]) - else: - value = "" - + value = fmt or value or "" parser.values.format = value + if target: config[target._format_config_key].set(value) else: diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 3117262f1..7b22c2462 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -28,7 +28,6 @@ import beets from beets import autotag, config, importer, library, logging, plugins, ui, util from beets.autotag import Recommendation, hooks from beets.ui import ( - decargs, input_, print_, print_column_layout, @@ -1303,7 +1302,7 @@ class TerminalImportSession(importer.ImportSession): # The import command. -def import_files(lib, paths, query): +def import_files(lib, paths: list[bytes], query): """Import the files in the given list of paths or matching the query. """ @@ -1334,7 +1333,7 @@ def import_files(lib, paths, query): plugins.send("import", lib=lib, paths=paths) -def import_func(lib, opts, args): +def import_func(lib, opts, args: list[str]): config["import"].set_args(opts) # Special case: --copy flag suppresses import_move (which would @@ -1343,7 +1342,7 @@ def import_func(lib, opts, args): config["import"]["move"] = False if opts.library: - query = decargs(args) + query = args paths = [] else: query = None @@ -1356,15 +1355,11 @@ def import_func(lib, opts, args): if not paths and not paths_from_logfiles: raise ui.UserError("no path specified") - # On Python 2, we used to get filenames as raw bytes, which is - # what we need. On Python 3, we need to undo the "helpful" - # conversion to Unicode strings to get the real bytestring - # filename. - paths = [os.fsencode(p) for p in paths] + byte_paths = [os.fsencode(p) for p in paths] paths_from_logfiles = [os.fsencode(p) for p in paths_from_logfiles] # Check the user-specified directories. - for path in paths: + for path in byte_paths: if not os.path.exists(syspath(normpath(path))): raise ui.UserError( "no such file or directory: {}".format( @@ -1385,14 +1380,14 @@ def import_func(lib, opts, args): ) continue - paths.append(path) + byte_paths.append(path) # If all paths were read from a logfile, and none of them exist, throw # an error if not paths: raise ui.UserError("none of the paths are importable") - import_files(lib, paths, query) + import_files(lib, byte_paths, query) import_cmd = ui.Subcommand( @@ -1596,7 +1591,7 @@ def list_items(lib, query, album, fmt=""): def list_func(lib, opts, args): - list_items(lib, decargs(args), opts.album) + list_items(lib, args, opts.album) list_cmd = ui.Subcommand("list", help="query the library", aliases=("ls",)) @@ -1739,7 +1734,7 @@ def update_func(lib, opts, args): return update_items( lib, - decargs(args), + args, opts.album, ui.should_move(opts.move), opts.pretend, @@ -1861,7 +1856,7 @@ def remove_items(lib, query, album, delete, force): def remove_func(lib, opts, args): - remove_items(lib, decargs(args), opts.album, opts.delete, opts.force) + remove_items(lib, args, opts.album, opts.delete, opts.force) remove_cmd = ui.Subcommand( @@ -1931,7 +1926,7 @@ Album artists: {}""".format( def stats_func(lib, opts, args): - show_stats(lib, decargs(args), opts.exact) + show_stats(lib, args, opts.exact) stats_cmd = ui.Subcommand( @@ -2059,7 +2054,7 @@ def modify_parse_args(args): def modify_func(lib, opts, args): - query, mods, dels = modify_parse_args(decargs(args)) + query, mods, dels = modify_parse_args(args) if not mods and not dels: raise ui.UserError("no modifications specified") modify_items( @@ -2127,12 +2122,20 @@ default_commands.append(modify_cmd) def move_items( - lib, dest, query, copy, album, pretend, confirm=False, export=False + lib, + dest_path: util.PathLike, + query, + copy, + album, + pretend, + confirm=False, + export=False, ): """Moves or copies items to a new base directory, given by dest. If dest is None, then the library's base directory is used, making the command "consolidate" files. """ + dest = os.fsencode(dest_path) if dest_path else dest_path items, albums = _do_query(lib, query, album, False) objs = albums if album else items num_objs = len(objs) @@ -2217,7 +2220,7 @@ def move_func(lib, opts, args): move_items( lib, dest, - decargs(args), + args, opts.copy, opts.album, opts.pretend, @@ -2298,7 +2301,7 @@ def write_items(lib, query, pretend, force): def write_func(lib, opts, args): - write_items(lib, decargs(args), opts.pretend, opts.force) + write_items(lib, args, opts.pretend, opts.force) write_cmd = ui.Subcommand("write", help="write tag information to files") diff --git a/beets/util/__init__.py b/beets/util/__init__.py index c1c76c860..00c9ce05d 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -28,6 +28,7 @@ import sys import tempfile import traceback from collections import Counter +from collections.abc import Sequence from contextlib import suppress from enum import Enum from functools import cache @@ -41,7 +42,6 @@ from typing import ( AnyStr, Callable, Generic, - Iterable, NamedTuple, TypeVar, Union, @@ -53,23 +53,17 @@ import beets from beets.util import hidden if TYPE_CHECKING: - from collections.abc import Iterator, Sequence + from collections.abc import Iterable, Iterator from logging import Logger from beets.library import Item -if sys.version_info >= (3, 10): - from typing import TypeAlias -else: - from typing_extensions import TypeAlias - MAX_FILENAME_LENGTH = 200 WINDOWS_MAGIC_PREFIX = "\\\\?\\" T = TypeVar("T") -BytesOrStr = Union[str, bytes] -PathLike = Union[BytesOrStr, Path] -Replacements: TypeAlias = "Sequence[tuple[Pattern[str], str]]" +PathLike = Union[str, bytes, Path] +Replacements = Sequence[tuple[Pattern[str], str]] # Here for now to allow for a easy replace later on # once we can move to a PathLike (mainly used in importer) @@ -860,7 +854,9 @@ class CommandOutput(NamedTuple): stderr: bytes -def command_output(cmd: list[BytesOrStr], shell: bool = False) -> CommandOutput: +def command_output( + cmd: list[str] | list[bytes], shell: bool = False +) -> CommandOutput: """Runs the command and returns its output after it has exited. Returns a CommandOutput. The attributes ``stdout`` and ``stderr`` contain @@ -878,8 +874,6 @@ def command_output(cmd: list[BytesOrStr], shell: bool = False) -> CommandOutput: This replaces `subprocess.check_output` which can have problems if lots of output is sent to stderr. """ - converted_cmd = [os.fsdecode(a) for a in cmd] - devnull = subprocess.DEVNULL proc = subprocess.Popen( @@ -894,7 +888,7 @@ def command_output(cmd: list[BytesOrStr], shell: bool = False) -> CommandOutput: if proc.returncode: raise subprocess.CalledProcessError( returncode=proc.returncode, - cmd=" ".join(converted_cmd), + cmd=" ".join(map(os.fsdecode, cmd)), output=stdout + stderr, ) return CommandOutput(stdout, stderr) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 33b98c413..fe67c506e 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -214,9 +214,9 @@ class IMBackend(LocalBackend): else: return cls._version - convert_cmd: list[str | bytes] - identify_cmd: list[str | bytes] - compare_cmd: list[str | bytes] + convert_cmd: list[str] + identify_cmd: list[str] + compare_cmd: list[str] def __init__(self) -> None: """Initialize a wrapper around ImageMagick for local image operations. @@ -265,7 +265,7 @@ class IMBackend(LocalBackend): # with regards to the height. # ImageMagick already seems to default to no interlace, but we include # it here for the sake of explicitness. - cmd: list[str | bytes] = self.convert_cmd + [ + cmd: list[str] = self.convert_cmd + [ syspath(path_in, prefix=False), "-resize", f"{maxwidth}x>", @@ -295,7 +295,7 @@ class IMBackend(LocalBackend): return path_out def get_size(self, path_in: bytes) -> tuple[int, int] | None: - cmd: list[str | bytes] = self.identify_cmd + [ + cmd: list[str] = self.identify_cmd + [ "-format", "%w %h", syspath(path_in, prefix=False), @@ -480,10 +480,11 @@ class IMBackend(LocalBackend): return True def write_metadata(self, file: bytes, metadata: Mapping[str, str]) -> None: - assignments = list( - chain.from_iterable(("-set", k, v) for k, v in metadata.items()) + assignments = chain.from_iterable( + ("-set", k, v) for k, v in metadata.items() ) - command = self.convert_cmd + [file, *assignments, file] + str_file = os.fsdecode(file) + command = self.convert_cmd + [str_file, *assignments, str_file] util.command_output(command) diff --git a/beetsplug/absubmit.py b/beetsplug/absubmit.py index 3c48f8897..3d3227ed2 100644 --- a/beetsplug/absubmit.py +++ b/beetsplug/absubmit.py @@ -137,7 +137,7 @@ only files which would be processed", ) else: # Get items from arguments - items = lib.items(ui.decargs(args)) + items = lib.items(args) self.opts = opts util.par_map(self.analyze_submit, items) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 714751ac9..56ac0f6c5 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -116,7 +116,7 @@ class AcousticPlugin(plugins.BeetsPlugin): ) def func(lib, opts, args): - items = lib.items(ui.decargs(args)) + items = lib.items(args) self._fetch_info( items, ui.should_write(), diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index 0903ebabf..0511d960d 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -204,7 +204,7 @@ class BadFiles(BeetsPlugin): def command(self, lib, opts, args): # Get items from arguments - items = lib.items(ui.decargs(args)) + items = lib.items(args) self.verbose = opts.verbose def check_and_print(item): diff --git a/beetsplug/bareasc.py b/beetsplug/bareasc.py index 3a52c41dd..d2852bb1d 100644 --- a/beetsplug/bareasc.py +++ b/beetsplug/bareasc.py @@ -23,7 +23,7 @@ from unidecode import unidecode from beets import ui from beets.dbcore.query import StringFieldQuery from beets.plugins import BeetsPlugin -from beets.ui import decargs, print_ +from beets.ui import print_ class BareascQuery(StringFieldQuery[str]): @@ -83,14 +83,13 @@ class BareascPlugin(BeetsPlugin): def unidecode_list(self, lib, opts, args): """Emulate normal 'list' command but with unidecode output.""" - query = decargs(args) album = opts.album # Copied from commands.py - list_items if album: - for album in lib.albums(query): + for album in lib.albums(args): bare = unidecode(str(album)) print_(bare) else: - for item in lib.items(query): + for item in lib.items(args): bare = unidecode(str(item)) print_(bare) diff --git a/beetsplug/bench.py b/beetsplug/bench.py index 62d512ce7..cf72527e8 100644 --- a/beetsplug/bench.py +++ b/beetsplug/bench.py @@ -125,7 +125,7 @@ class BenchmarkPlugin(BeetsPlugin): "-i", "--id", default=None, help="album ID to match against" ) match_bench_cmd.func = lambda lib, opts, args: match_benchmark( - lib, opts.profile, ui.decargs(args), opts.id + lib, opts.profile, args, opts.id ) return [aunique_bench_cmd, match_bench_cmd] diff --git a/beetsplug/bpm.py b/beetsplug/bpm.py index 946769cdc..145986a95 100644 --- a/beetsplug/bpm.py +++ b/beetsplug/bpm.py @@ -63,9 +63,8 @@ class BPMPlugin(BeetsPlugin): return [cmd] def command(self, lib, opts, args): - items = lib.items(ui.decargs(args)) write = ui.should_write() - self.get_bpm(items, write) + self.get_bpm(lib.items(args), write) def get_bpm(self, items, write=False): overwrite = self.config["overwrite"].get(bool) diff --git a/beetsplug/bpsync.py b/beetsplug/bpsync.py index 05be94c99..ccd781b28 100644 --- a/beetsplug/bpsync.py +++ b/beetsplug/bpsync.py @@ -65,10 +65,9 @@ class BPSyncPlugin(BeetsPlugin): move = ui.should_move(opts.move) pretend = opts.pretend write = ui.should_write(opts.write) - query = ui.decargs(args) - self.singletons(lib, query, move, pretend, write) - self.albums(lib, query, move, pretend, write) + self.singletons(lib, args, move, pretend, write) + self.albums(lib, args, move, pretend, write) def singletons(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for items matched by diff --git a/beetsplug/chroma.py b/beetsplug/chroma.py index 5c718154b..de3ac525a 100644 --- a/beetsplug/chroma.py +++ b/beetsplug/chroma.py @@ -233,7 +233,7 @@ class AcoustidPlugin(plugins.BeetsPlugin): apikey = config["acoustid"]["apikey"].as_str() except confuse.NotFoundError: raise ui.UserError("no Acoustid user API key provided") - submit_items(self._log, apikey, lib.items(ui.decargs(args))) + submit_items(self._log, apikey, lib.items(args)) submit_cmd.func = submit_cmd_func @@ -242,7 +242,7 @@ class AcoustidPlugin(plugins.BeetsPlugin): ) def fingerprint_cmd_func(lib, opts, args): - for item in lib.items(ui.decargs(args)): + for item in lib.items(args): fingerprint_item(self._log, item, write=ui.should_write()) fingerprint_cmd.func = fingerprint_cmd_func diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 7586c2a1b..c4df9ab57 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -301,7 +301,7 @@ class ConvertPlugin(BeetsPlugin): encode_cmd.append(os.fsdecode(args[i])) if pretend: - self._log.info("{0}", " ".join(ui.decargs(args))) + self._log.info("{0}", " ".join(args)) return try: @@ -323,9 +323,7 @@ class ConvertPlugin(BeetsPlugin): raise except OSError as exc: raise ui.UserError( - "convert: couldn't invoke '{}': {}".format( - " ".join(ui.decargs(args)), exc - ) + "convert: couldn't invoke '{}': {}".format(" ".join(args), exc) ) if not quiet and not pretend: @@ -579,13 +577,13 @@ class ConvertPlugin(BeetsPlugin): ) = self._get_opts_and_config(opts) if opts.album: - albums = lib.albums(ui.decargs(args)) + albums = lib.albums(args) items = [i for a in albums for i in a.items()] if not pretend: for a in albums: ui.print_(format(a, "")) else: - items = list(lib.items(ui.decargs(args))) + items = list(lib.items(args)) if not pretend: for i in items: ui.print_(format(i, "")) diff --git a/beetsplug/deezer.py b/beetsplug/deezer.py index 89f7436f8..7e4896437 100644 --- a/beetsplug/deezer.py +++ b/beetsplug/deezer.py @@ -18,6 +18,7 @@ from __future__ import annotations import collections import time +from typing import TYPE_CHECKING, Literal, Sequence import requests import unidecode @@ -25,10 +26,14 @@ import unidecode from beets import ui from beets.autotag import AlbumInfo, TrackInfo from beets.dbcore import types -from beets.plugins import BeetsPlugin, MetadataSourcePlugin +from beets.plugins import BeetsPlugin, MetadataSourcePlugin, Response + +if TYPE_CHECKING: + from beets.library import Item, Library + from beetsplug._typing import JSONDict -class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): +class DeezerPlugin(MetadataSourcePlugin[Response], BeetsPlugin): data_source = "Deezer" item_types = { @@ -36,43 +41,26 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): "deezer_track_id": types.INTEGER, "deezer_updated": types.DATE, } - # Base URLs for the Deezer API # Documentation: https://developers.deezer.com/api/ search_url = "https://api.deezer.com/search/" album_url = "https://api.deezer.com/album/" track_url = "https://api.deezer.com/track/" - def __init__(self): - super().__init__() - def commands(self): """Add beet UI commands to interact with Deezer.""" deezer_update_cmd = ui.Subcommand( "deezerupdate", help=f"Update {self.data_source} rank" ) - def func(lib, opts, args): - items = lib.items(ui.decargs(args)) - self.deezerupdate(items, ui.should_write()) + def func(lib: Library, opts, args): + items = lib.items(args) + self.deezerupdate(list(items), ui.should_write()) deezer_update_cmd.func = func return [deezer_update_cmd] - def fetch_data(self, url): - try: - response = requests.get(url, timeout=10) - response.raise_for_status() - data = response.json() - except requests.exceptions.RequestException as e: - self._log.error("Error fetching data from {}\n Error: {}", url, e) - return None - if "error" in data: - self._log.debug("Deezer API error: {}", data["error"]["message"]) - return None - return data - def album_for_id(self, album_id: str) -> AlbumInfo | None: """Fetch an album by its Deezer ID or URL.""" if not (deezer_id := self._get_id(album_id)): @@ -156,52 +144,18 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): cover_art_url=album_data.get("cover_xl"), ) - def _get_track(self, track_data): - """Convert a Deezer track object dict to a TrackInfo object. + def track_for_id(self, track_id: str) -> None | TrackInfo: + """Fetch a track by its Deezer ID or URL. - :param track_data: Deezer Track object dict - :type track_data: dict - :return: TrackInfo object for track - :rtype: beets.autotag.hooks.TrackInfo + Returns a TrackInfo object or None if the track is not found. """ - artist, artist_id = self.get_artist( - track_data.get("contributors", [track_data["artist"]]) - ) - return TrackInfo( - title=track_data["title"], - track_id=track_data["id"], - deezer_track_id=track_data["id"], - isrc=track_data.get("isrc"), - artist=artist, - artist_id=artist_id, - length=track_data["duration"], - index=track_data.get("track_position"), - medium=track_data.get("disk_number"), - deezer_track_rank=track_data.get("rank"), - medium_index=track_data.get("track_position"), - data_source=self.data_source, - data_url=track_data["link"], - deezer_updated=time.time(), - ) + if not (deezer_id := self._get_id(track_id)): + self._log.debug("Invalid Deezer track_id: {}", track_id) + return None - def track_for_id(self, track_id=None, track_data=None): - """Fetch a track by its Deezer ID or URL and return a - TrackInfo object or None if the track is not found. - - :param track_id: (Optional) Deezer ID or URL for the track. Either - ``track_id`` or ``track_data`` must be provided. - :type track_id: str - :param track_data: (Optional) Simplified track object dict. May be - provided instead of ``track_id`` to avoid unnecessary API calls. - :type track_data: dict - :return: TrackInfo object for track - :rtype: beets.autotag.hooks.TrackInfo or None - """ - if track_data is None: - if not (deezer_id := self._get_id(track_id)) or not ( - track_data := self.fetch_data(f"{self.track_url}{deezer_id}") - ): - return None + if not (track_data := self.fetch_data(f"{self.track_url}{deezer_id}")): + self._log.debug("Track not found: {}", track_id) + return None track = self._get_track(track_data) @@ -229,18 +183,43 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): track.medium_total = medium_total return track + def _get_track(self, track_data: JSONDict) -> TrackInfo: + """Convert a Deezer track object dict to a TrackInfo object. + + :param track_data: Deezer Track object dict + :return: TrackInfo object for track + """ + artist, artist_id = self.get_artist( + track_data.get("contributors", [track_data["artist"]]) + ) + return TrackInfo( + title=track_data["title"], + track_id=track_data["id"], + deezer_track_id=track_data["id"], + isrc=track_data.get("isrc"), + artist=artist, + artist_id=artist_id, + length=track_data["duration"], + index=track_data.get("track_position"), + medium=track_data.get("disk_number"), + deezer_track_rank=track_data.get("rank"), + medium_index=track_data.get("track_position"), + data_source=self.data_source, + data_url=track_data["link"], + deezer_updated=time.time(), + ) + @staticmethod - def _construct_search_query(filters=None, keywords=""): + def _construct_search_query( + filters: dict[str, str], keywords: str = "" + ) -> str: """Construct a query string with the specified filters and keywords to be provided to the Deezer Search API (https://developers.deezer.com/api/search). - :param filters: (Optional) Field filters to apply. - :type filters: dict + :param filters: Field filters to apply. :param keywords: (Optional) Query keywords to use. - :type keywords: str :return: Query string to be provided to the Search API. - :rtype: str """ query_components = [ keywords, @@ -251,25 +230,30 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): query = query.decode("utf8") return unidecode.unidecode(query) - def _search_api(self, query_type, filters=None, keywords=""): + def _search_api( + self, + query_type: Literal[ + "album", + "track", + "artist", + "history", + "playlist", + "podcast", + "radio", + "user", + ], + filters: dict[str, str], + keywords="", + ) -> Sequence[Response]: """Query the Deezer Search API for the specified ``keywords``, applying the provided ``filters``. - :param query_type: The Deezer Search API method to use. Valid types - are: 'album', 'artist', 'history', 'playlist', 'podcast', - 'radio', 'track', 'user', and 'track'. - :type query_type: str - :param filters: (Optional) Field filters to apply. - :type filters: dict + :param query_type: The Deezer Search API method to use. :param keywords: (Optional) Query keywords to use. - :type keywords: str :return: JSON data for the class:`Response ` object or None if no search results are returned. - :rtype: dict or None """ query = self._construct_search_query(keywords=keywords, filters=filters) - if not query: - return None self._log.debug(f"Searching {self.data_source} for '{query}'") try: response = requests.get( @@ -284,7 +268,7 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): self.data_source, e, ) - return None + return () response_data = response.json().get("data", []) self._log.debug( "Found {} result(s) from {} for '{}'", @@ -294,7 +278,7 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): ) return response_data - def deezerupdate(self, items, write): + def deezerupdate(self, items: Sequence[Item], write: bool): """Obtain rank information from Deezer.""" for index, item in enumerate(items, start=1): self._log.info( @@ -320,3 +304,16 @@ class DeezerPlugin(MetadataSourcePlugin, BeetsPlugin): item.deezer_updated = time.time() if write: item.try_write() + + def fetch_data(self, url: str): + try: + response = requests.get(url, timeout=10) + response.raise_for_status() + data = response.json() + except requests.exceptions.RequestException as e: + self._log.error("Error fetching data from {}\n Error: {}", url, e) + return None + if "error" in data: + self._log.debug("Deezer API error: {}", data["error"]["message"]) + return None + return data diff --git a/beetsplug/duplicates.py b/beetsplug/duplicates.py index 5a2be0cd2..ea7abaaff 100644 --- a/beetsplug/duplicates.py +++ b/beetsplug/duplicates.py @@ -19,7 +19,7 @@ import shlex from beets.library import Album, Item from beets.plugins import BeetsPlugin -from beets.ui import Subcommand, UserError, decargs, print_ +from beets.ui import Subcommand, UserError, print_ from beets.util import ( MoveOperation, bytestring_path, @@ -163,11 +163,11 @@ class DuplicatesPlugin(BeetsPlugin): if album: if not keys: keys = ["mb_albumid"] - items = lib.albums(decargs(args)) + items = lib.albums(args) else: if not keys: keys = ["mb_trackid", "mb_albumid"] - items = lib.items(decargs(args)) + items = lib.items(args) # If there's nothing to do, return early. The code below assumes # `items` to be non-empty. diff --git a/beetsplug/edit.py b/beetsplug/edit.py index b92c48839..52387c314 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -180,8 +180,7 @@ class EditPlugin(plugins.BeetsPlugin): def _edit_command(self, lib, opts, args): """The CLI command function for the `beet edit` command.""" # Get the objects to edit. - query = ui.decargs(args) - items, albums = _do_query(lib, query, opts.album, False) + items, albums = _do_query(lib, args, opts.album, False) objs = albums if opts.album else items if not objs: ui.print_("Nothing to edit.") diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 2a4e06a93..8df3c3c05 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -22,7 +22,7 @@ import requests from beets import art, config, ui from beets.plugins import BeetsPlugin -from beets.ui import decargs, print_ +from beets.ui import print_ from beets.util import bytestring_path, displayable_path, normpath, syspath from beets.util.artresizer import ArtResizer @@ -115,7 +115,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): ) ) - items = lib.items(decargs(args)) + items = lib.items(args) # Confirm with user. if not opts.yes and not _confirm(items, not opts.file): @@ -151,7 +151,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): except Exception as e: self._log.error("Unable to save image: {}".format(e)) return - items = lib.items(decargs(args)) + items = lib.items(args) # Confirm with user. if not opts.yes and not _confirm(items, not opts.url): os.remove(tempimg) @@ -169,7 +169,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): ) os.remove(tempimg) else: - albums = lib.albums(decargs(args)) + albums = lib.albums(args) # Confirm with user. if not opts.yes and not _confirm(albums, not opts.file): return @@ -212,7 +212,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): def extract_func(lib, opts, args): if opts.outpath: art.extract_first( - self._log, normpath(opts.outpath), lib.items(decargs(args)) + self._log, normpath(opts.outpath), lib.items(args) ) else: filename = bytestring_path( @@ -223,7 +223,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): "Only specify a name rather than a path for -n" ) return - for album in lib.albums(decargs(args)): + for album in lib.albums(args): artpath = normpath(os.path.join(album.path, filename)) artpath = art.extract_first( self._log, artpath, album.items() @@ -244,11 +244,11 @@ class EmbedCoverArtPlugin(BeetsPlugin): ) def clear_func(lib, opts, args): - items = lib.items(decargs(args)) + items = lib.items(args) # Confirm with user. if not opts.yes and not _confirm(items, False): return - art.clear(self._log, lib, decargs(args)) + art.clear(self._log, lib, args) clear_cmd.func = clear_func diff --git a/beetsplug/export.py b/beetsplug/export.py index 9b8ad3580..05ca3f24a 100644 --- a/beetsplug/export.py +++ b/beetsplug/export.py @@ -144,7 +144,7 @@ class ExportPlugin(BeetsPlugin): items = [] for data_emitter in data_collector( lib, - ui.decargs(args), + args, album=opts.album, ): try: diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b442633da..e1ec5aa09 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -1503,9 +1503,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): ) def func(lib: Library, opts, args) -> None: - self.batch_fetch_art( - lib, lib.albums(ui.decargs(args)), opts.force, opts.quiet - ) + self.batch_fetch_art(lib, lib.albums(args), opts.force, opts.quiet) cmd.func = func return [cmd] diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index a85aa9719..150f230aa 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -118,7 +118,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): keep_in_artist_field = self.config["keep_in_artist"].get(bool) write = ui.should_write() - for item in lib.items(ui.decargs(args)): + for item in lib.items(args): if self.ft_in_title(item, drop_feat, keep_in_artist_field): item.store() if write: diff --git a/beetsplug/hook.py b/beetsplug/hook.py index 5ce5ef828..90d66553a 100644 --- a/beetsplug/hook.py +++ b/beetsplug/hook.py @@ -14,27 +14,21 @@ """Allows custom commands to be run when an event is emitted by beets""" +from __future__ import annotations + +import os import shlex import string import subprocess -import sys +from typing import Any from beets.plugins import BeetsPlugin -class CodingFormatter(string.Formatter): - """A variant of `string.Formatter` that converts everything to `unicode` - strings. +class BytesToStrFormatter(string.Formatter): + """A variant of `string.Formatter` that converts `bytes` to `str`.""" - This was necessary on Python 2, in needs to be kept for backwards - compatibility. - """ - - def __init__(self, coding): - """Creates a new coding formatter with the provided coding.""" - self._coding = coding - - def convert_field(self, value, conversion): + def convert_field(self, value: Any, conversion: str | None) -> Any: """Converts the provided value given a conversion type. This method decodes the converted value using the formatter's coding. @@ -42,7 +36,7 @@ class CodingFormatter(string.Formatter): converted = super().convert_field(value, conversion) if isinstance(converted, bytes): - return converted.decode(self._coding) + return os.fsdecode(converted) return converted @@ -72,8 +66,8 @@ class HookPlugin(BeetsPlugin): return # For backwards compatibility, use a string formatter that decodes - # bytes (in particular, paths) to unicode strings. - formatter = CodingFormatter(sys.getfilesystemencoding()) + # bytes (in particular, paths) to strings. + formatter = BytesToStrFormatter() command_pieces = [ formatter.format(piece, event=event, **kwargs) for piece in shlex.split(command) diff --git a/beetsplug/info.py b/beetsplug/info.py index d759d6066..c4d5aacbf 100644 --- a/beetsplug/info.py +++ b/beetsplug/info.py @@ -215,7 +215,7 @@ class InfoPlugin(BeetsPlugin): summary = {} for data_emitter in data_collector( lib, - ui.decargs(args), + args, album=opts.album, ): try: @@ -232,7 +232,7 @@ class InfoPlugin(BeetsPlugin): if opts.keys_only: print_data_keys(data, item) else: - fmt = ui.decargs([opts.format])[0] if opts.format else None + fmt = [opts.format][0] if opts.format else None print_data(data, item, fmt) first = False diff --git a/beetsplug/ipfs.py b/beetsplug/ipfs.py index 1c8c89aa9..3c6425c06 100644 --- a/beetsplug/ipfs.py +++ b/beetsplug/ipfs.py @@ -74,7 +74,7 @@ class IPFSPlugin(BeetsPlugin): def func(lib, opts, args): if opts.add: - for album in lib.albums(ui.decargs(args)): + for album in lib.albums(args): if len(album.items()) == 0: self._log.info( "{0} does not contain items, aborting", album @@ -84,19 +84,19 @@ class IPFSPlugin(BeetsPlugin): album.store() if opts.get: - self.ipfs_get(lib, ui.decargs(args)) + self.ipfs_get(lib, args) if opts.publish: self.ipfs_publish(lib) if opts._import: - self.ipfs_import(lib, ui.decargs(args)) + self.ipfs_import(lib, args) if opts._list: - self.ipfs_list(lib, ui.decargs(args)) + self.ipfs_list(lib, args) if opts.play: - self.ipfs_play(lib, opts, ui.decargs(args)) + self.ipfs_play(lib, opts, args) cmd.func = func return [cmd] diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index 87f0cc427..00b688d4f 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -43,7 +43,7 @@ class KeyFinderPlugin(BeetsPlugin): return [cmd] def command(self, lib, opts, args): - self.find_key(lib.items(ui.decargs(args)), write=ui.should_write()) + self.find_key(lib.items(args), write=ui.should_write()) def imported(self, session, task): self.find_key(task.imported_items()) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 30b44e187..b67f1fae2 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -521,7 +521,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): if opts.album: # Fetch genres for whole albums - for album in lib.albums(ui.decargs(args)): + for album in lib.albums(args): album.genre, src = self._get_genre(album) self._log.info( 'genre for album "{0.album}" ({1}): {0.genre}', @@ -550,7 +550,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): else: # Just query singletons, i.e. items that are not part of # an album - for item in lib.items(ui.decargs(args)): + for item in lib.items(args): item.genre, src = self._get_genre(item) item.store() self._log.info( diff --git a/beetsplug/limit.py b/beetsplug/limit.py index 0a13a78aa..aae99a717 100644 --- a/beetsplug/limit.py +++ b/beetsplug/limit.py @@ -25,7 +25,7 @@ from itertools import islice from beets.dbcore import FieldQuery from beets.plugins import BeetsPlugin -from beets.ui import Subcommand, decargs, print_ +from beets.ui import Subcommand, print_ def lslimit(lib, opts, args): @@ -36,11 +36,10 @@ def lslimit(lib, opts, args): if (opts.head or opts.tail or 0) < 0: raise ValueError("Limit value must be non-negative") - query = decargs(args) if opts.album: - objs = lib.albums(query) + objs = lib.albums(args) else: - objs = lib.items(query) + objs = lib.items(args) if opts.head is not None: objs = islice(objs, opts.head) diff --git a/beetsplug/mbcollection.py b/beetsplug/mbcollection.py index 1c010bf50..7a1289d1b 100644 --- a/beetsplug/mbcollection.py +++ b/beetsplug/mbcollection.py @@ -70,10 +70,14 @@ class MusicBrainzCollectionPlugin(BeetsPlugin): if not collections["collection-list"]: raise ui.UserError("no collections exist for user") - # Get all collection IDs, avoiding event collections - collection_ids = [x["id"] for x in collections["collection-list"]] + # Get all release collection IDs, avoiding event collections + collection_ids = [ + x["id"] + for x in collections["collection-list"] + if x["entity-type"] == "release" + ] if not collection_ids: - raise ui.UserError("No collection found.") + raise ui.UserError("No release collection found.") # Check that the collection exists so we can present a nice error collection = self.config["collection"].as_str() diff --git a/beetsplug/mbsubmit.py b/beetsplug/mbsubmit.py index d215e616c..e23c0d610 100644 --- a/beetsplug/mbsubmit.py +++ b/beetsplug/mbsubmit.py @@ -86,7 +86,7 @@ class MBSubmitPlugin(BeetsPlugin): ) def func(lib, opts, args): - items = lib.items(ui.decargs(args)) + items = lib.items(args) self._mbsubmit(items) mbsubmit_cmd.func = func diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py index 94870232c..d38b25e9f 100644 --- a/beetsplug/mbsync.py +++ b/beetsplug/mbsync.py @@ -63,10 +63,9 @@ class MBSyncPlugin(BeetsPlugin): move = ui.should_move(opts.move) pretend = opts.pretend write = ui.should_write(opts.write) - query = ui.decargs(args) - self.singletons(lib, query, move, pretend, write) - self.albums(lib, query, move, pretend, write) + self.singletons(lib, args, move, pretend, write) + self.albums(lib, args, move, pretend, write) def singletons(self, lib, query, move, pretend, write): """Retrieve and apply info from the autotagger for items matched by diff --git a/beetsplug/metasync/__init__.py b/beetsplug/metasync/__init__.py index 2466efe54..f99e820b5 100644 --- a/beetsplug/metasync/__init__.py +++ b/beetsplug/metasync/__init__.py @@ -97,7 +97,6 @@ class MetaSyncPlugin(BeetsPlugin): def func(self, lib, opts, args): """Command handler for the metasync function.""" pretend = opts.pretend - query = ui.decargs(args) sources = [] for source in opts.sources: @@ -106,7 +105,7 @@ class MetaSyncPlugin(BeetsPlugin): sources = sources or self.config["source"].as_str_seq() meta_source_instances = {} - items = lib.items(query) + items = lib.items(args) # Avoid needlessly instantiating meta sources (can be expensive) if not items: diff --git a/beetsplug/missing.py b/beetsplug/missing.py index c4bbb83fd..8c328e647 100644 --- a/beetsplug/missing.py +++ b/beetsplug/missing.py @@ -25,7 +25,7 @@ from beets import config, plugins from beets.dbcore import types from beets.library import Album, Item, Library from beets.plugins import BeetsPlugin -from beets.ui import Subcommand, decargs, print_ +from beets.ui import Subcommand, print_ MB_ARTIST_QUERY = r"mb_albumartistid::^\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$" @@ -135,7 +135,7 @@ class MissingPlugin(BeetsPlugin): albms = self.config["album"].get() helper = self._missing_albums if albms else self._missing_tracks - helper(lib, decargs(args)) + helper(lib, args) self._command.func = _miss return [self._command] diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py index 20faf225f..52ae88e1f 100644 --- a/beetsplug/mpdstats.py +++ b/beetsplug/mpdstats.py @@ -27,6 +27,7 @@ from beets.util import displayable_path # much time should we wait between retries? RETRIES = 10 RETRY_INTERVAL = 5 +DUPLICATE_PLAY_THRESHOLD = 10.0 mpd_config = config["mpd"] @@ -143,7 +144,9 @@ class MPDStats: self.do_rating = mpd_config["rating"].get(bool) self.rating_mix = mpd_config["rating_mix"].get(float) - self.time_threshold = 10.0 # TODO: maybe add config option? + self.played_ratio_threshold = mpd_config["played_ratio_threshold"].get( + float + ) self.now_playing = None self.mpd = MPDClientWrapper(log) @@ -216,10 +219,8 @@ class MPDStats: Returns whether the change was manual (skipped previous song or not) """ - diff = abs(song["remaining"] - (time.time() - song["started"])) - - skipped = diff >= self.time_threshold - + elapsed = song["elapsed_at_start"] + (time.time() - song["started"]) + skipped = elapsed / song["duration"] < self.played_ratio_threshold if skipped: self.handle_skipped(song) else: @@ -256,13 +257,10 @@ class MPDStats: def on_play(self, status): path, songid = self.mpd.currentsong() - if not path: return played, duration = map(int, status["time"].split(":", 1)) - remaining = duration - played - if self.now_playing: if self.now_playing["path"] != path: self.handle_song_change(self.now_playing) @@ -273,7 +271,7 @@ class MPDStats: # after natural song start. diff = abs(time.time() - self.now_playing["started"]) - if diff <= self.time_threshold: + if diff <= DUPLICATE_PLAY_THRESHOLD: return if self.now_playing["path"] == path and played == 0: @@ -288,7 +286,8 @@ class MPDStats: self.now_playing = { "started": time.time(), - "remaining": remaining, + "elapsed_at_start": played, + "duration": duration, "path": path, "id": songid, "beets_item": self.get_item(path), @@ -337,6 +336,7 @@ class MPDStatsPlugin(plugins.BeetsPlugin): "host": os.environ.get("MPD_HOST", "localhost"), "port": int(os.environ.get("MPD_PORT", 6600)), "password": "", + "played_ratio_threshold": 0.85, } ) mpd_config["password"].redact = True diff --git a/beetsplug/parentwork.py b/beetsplug/parentwork.py index 463a455f5..ab2d39b2b 100644 --- a/beetsplug/parentwork.py +++ b/beetsplug/parentwork.py @@ -88,7 +88,7 @@ class ParentWorkPlugin(BeetsPlugin): force_parent = self.config["force"].get(bool) write = ui.should_write() - for item in lib.items(ui.decargs(args)): + for item in lib.items(args): changed = self.find_work(item, force_parent, verbose=True) if changed: item.store() diff --git a/beetsplug/play.py b/beetsplug/play.py index ddebd7d41..3e7ba0a9e 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -107,7 +107,7 @@ class PlayPlugin(BeetsPlugin): # Perform search by album and add folders rather than tracks to # playlist. if opts.album: - selection = lib.albums(ui.decargs(args)) + selection = lib.albums(args) paths = [] sort = lib.get_default_album_sort() @@ -120,7 +120,7 @@ class PlayPlugin(BeetsPlugin): # Perform item query and add tracks to playlist. else: - selection = lib.items(ui.decargs(args)) + selection = lib.items(args) paths = [item.path for item in selection] item_type = "track" diff --git a/beetsplug/random.py b/beetsplug/random.py index 05f2cdf77..c791af414 100644 --- a/beetsplug/random.py +++ b/beetsplug/random.py @@ -16,17 +16,16 @@ from beets.plugins import BeetsPlugin from beets.random import random_objs -from beets.ui import Subcommand, decargs, print_ +from beets.ui import Subcommand, print_ def random_func(lib, opts, args): """Select some random items or albums and print the results.""" # Fetch all the objects matching the query into a list. - query = decargs(args) if opts.album: - objs = list(lib.albums(query)) + objs = list(lib.albums(args)) else: - objs = list(lib.items(query)) + objs = list(lib.items(args)) # Print a random subset. objs = random_objs( diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 3aad8cd89..00b651d99 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -62,7 +62,7 @@ class FatalGstreamerPluginReplayGainError(FatalReplayGainError): loading the required plugins.""" -def call(args: list[Any], log: Logger, **kwargs: Any): +def call(args: list[str], log: Logger, **kwargs: Any): """Execute the command and return its output or raise a ReplayGainError on failure. """ @@ -73,11 +73,6 @@ def call(args: list[Any], log: Logger, **kwargs: Any): raise ReplayGainError( "{} exited with status {}".format(args[0], e.returncode) ) - except UnicodeEncodeError: - # Due to a bug in Python 2's subprocess on Windows, Unicode - # filenames can fail to encode on that platform. See: - # https://github.com/google-code-export/beets/issues/499 - raise ReplayGainError("argument encoding failed") def db_to_lufs(db: float) -> float: @@ -403,20 +398,18 @@ class FfmpegBackend(Backend): def _construct_cmd( self, item: Item, peak_method: PeakMethod | None - ) -> list[str | bytes]: + ) -> list[str]: """Construct the shell command to analyse items.""" return [ self._ffmpeg_path, "-nostats", "-hide_banner", "-i", - item.path, + str(item.filepath), "-map", "a:0", "-filter", - "ebur128=peak={}".format( - "none" if peak_method is None else peak_method.name - ), + f"ebur128=peak={'none' if peak_method is None else peak_method.name}", "-f", "null", "-", @@ -660,7 +653,7 @@ class CommandBackend(Backend): # tag-writing; this turns the mp3gain/aacgain tool into a gain # calculator rather than a tag manipulator because we take care # of changing tags ourselves. - cmd: list[bytes | str] = [self.command, "-o", "-s", "s"] + cmd: list[str] = [self.command, "-o", "-s", "s"] if self.noclip: # Adjust to avoid clipping. cmd = cmd + ["-k"] @@ -1039,7 +1032,7 @@ class AudioToolsBackend(Backend): os.fsdecode(syspath(item.path)) ) except OSError: - raise ReplayGainError(f"File {item.path} was not found") + raise ReplayGainError(f"File {item.filepath} was not found") except self._mod_audiotools.UnsupportedFile: raise ReplayGainError(f"Unsupported file type {item.format}") @@ -1530,7 +1523,7 @@ class ReplayGainPlugin(BeetsPlugin): self.open_pool(threads) if opts.album: - albums = lib.albums(ui.decargs(args)) + albums = lib.albums(args) self._log.info( "Analyzing {} albums ~ {} backend...".format( len(albums), self.backend_name @@ -1539,7 +1532,7 @@ class ReplayGainPlugin(BeetsPlugin): for album in albums: self.handle_album(album, write, force) else: - items = lib.items(ui.decargs(args)) + items = lib.items(args) self._log.info( "Analyzing {} tracks ~ {} backend...".format( len(items), self.backend_name diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index 630a4e6e6..813effb5f 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -58,7 +58,7 @@ class ScrubPlugin(BeetsPlugin): def commands(self): def scrub_func(lib, opts, args): # Walk through matching files and remove tags. - for item in lib.items(ui.decargs(args)): + for item in lib.items(args): self._log.info( "scrubbing: {0}", util.displayable_path(item.path) ) diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 5ea3c6bff..e65d59649 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -127,7 +127,7 @@ class SmartPlaylistPlugin(BeetsPlugin): def update_cmd(self, lib, opts, args): self.build_queries() if args: - args = set(ui.decargs(args)) + args = set(args) for a in list(args): if not a.endswith(".m3u"): args.add(f"{a}.m3u") diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index 595da4892..36790b56b 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -453,7 +453,7 @@ class SpotifyPlugin(MetadataSourcePlugin, BeetsPlugin): def queries(lib, opts, args): success = self._parse_opts(opts) if success: - results = self._match_library_tracks(lib, ui.decargs(args)) + results = self._match_library_tracks(lib, args) self._output_match_results(results) spotify_cmd = ui.Subcommand( @@ -491,7 +491,7 @@ class SpotifyPlugin(MetadataSourcePlugin, BeetsPlugin): ) def func(lib, opts, args): - items = lib.items(ui.decargs(args)) + items = lib.items(args) self._fetch_info(items, ui.should_write(), opts.force_refetch) sync_cmd.func = func diff --git a/beetsplug/thumbnails.py b/beetsplug/thumbnails.py index e11b75390..5460d3fec 100644 --- a/beetsplug/thumbnails.py +++ b/beetsplug/thumbnails.py @@ -28,7 +28,7 @@ from pathlib import PurePosixPath from xdg import BaseDirectory from beets.plugins import BeetsPlugin -from beets.ui import Subcommand, decargs +from beets.ui import Subcommand from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer @@ -78,7 +78,7 @@ class ThumbnailsPlugin(BeetsPlugin): def process_query(self, lib, opts, args): self.config.set_args(opts) if self._check_local_ok(): - for album in lib.albums(decargs(args)): + for album in lib.albums(args): self.process_album(album) def _check_local_ok(self): diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index c1b0b5029..559f0622c 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -308,18 +308,8 @@ def all_items(): def item_file(item_id): item = g.lib.get_item(item_id) - # On Windows under Python 2, Flask wants a Unicode path. On Python 3, it - # *always* wants a Unicode path. - if os.name == "nt": - item_path = util.syspath(item.path) - else: - item_path = os.fsdecode(item.path) - + item_path = util.syspath(item.path) base_filename = os.path.basename(item_path) - if isinstance(base_filename, bytes): - unicode_base_filename = util.displayable_path(base_filename) - else: - unicode_base_filename = base_filename try: # Imitate http.server behaviour @@ -327,7 +317,7 @@ def item_file(item_id): except UnicodeError: safe_filename = unidecode(base_filename) else: - safe_filename = unicode_base_filename + safe_filename = base_filename response = flask.send_file( item_path, as_attachment=True, download_name=safe_filename @@ -470,7 +460,7 @@ class WebPlugin(BeetsPlugin): ) def func(lib, opts, args): - args = ui.decargs(args) + args = args if args: self.config["host"] = args.pop(0) if args: diff --git a/beetsplug/zero.py b/beetsplug/zero.py index 7ee624ce7..05e55bfcd 100644 --- a/beetsplug/zero.py +++ b/beetsplug/zero.py @@ -21,7 +21,7 @@ from mediafile import MediaFile from beets.importer import Action from beets.plugins import BeetsPlugin -from beets.ui import Subcommand, decargs, input_yn +from beets.ui import Subcommand, input_yn __author__ = "baobab@heresiarch.info" @@ -75,11 +75,11 @@ class ZeroPlugin(BeetsPlugin): zero_command = Subcommand("zero", help="set fields to null") def zero_fields(lib, opts, args): - if not decargs(args) and not input_yn( + if not args and not input_yn( "Remove fields for all items? (Y/n)", True ): return - for item in lib.items(decargs(args)): + for item in lib.items(args): self.process_item(item) zero_command.func = zero_fields diff --git a/docs/changelog.rst b/docs/changelog.rst index 34f27f8b3..f214a022f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,11 @@ New features: * :doc:`plugins/playlist`: Support files with the `.m3u8` extension. :bug:`5829` * :doc:`plugins/web`: Display artist and album as part of the search results. +* :doc:`plugins/mbcollection`: When getting the user collections, only consider + collections of releases, and ignore collections of other entity types. +* :doc:`plugins/mpdstats`: Add new configuration option, + ``played_ratio_threshold``, to allow configuring the percentage the song must + be played for it to be counted as played instead of skipped. Bug fixes: @@ -69,6 +74,8 @@ Other changes: Autogenerated API references are now located in the `docs/api` subdirectory. * :doc:`/plugins/substitute`: Fix rST formatting for example cases so that each case is shown on separate lines. +* Refactored library.py file by splitting it into multiple modules within the + beets/library directory. 2.3.1 (May 14, 2025) -------------------- diff --git a/docs/plugins/mpdstats.rst b/docs/plugins/mpdstats.rst index cb2cf1606..865b615a7 100644 --- a/docs/plugins/mpdstats.rst +++ b/docs/plugins/mpdstats.rst @@ -58,6 +58,9 @@ configuration file. The available options are: Default: ``yes``. - **rating_mix**: Tune the way rating is calculated (see below). Default: 0.75. +- **played_ratio_threshold**: If a song was played for less than this percentage + of its duration it will be considered a skip. + Default: 0.85 A Word on Ratings ----------------- diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 6577b54fc..38f8c7559 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -19,6 +19,7 @@ from __future__ import annotations import os import shutil import unittest +from pathlib import Path from typing import TYPE_CHECKING from unittest.mock import patch @@ -244,13 +245,13 @@ class FetchImageTest(FetchImageTestCase): self.mock_response(self.URL, "image/png") self.source.fetch_image(self.candidate, self.settings) assert os.path.splitext(self.candidate.path)[1] == b".png" - self.assertExists(self.candidate.path) + assert Path(os.fsdecode(self.candidate.path)).exists() def test_does_not_rely_on_server_content_type(self): self.mock_response(self.URL, "image/jpeg", "image/png") self.source.fetch_image(self.candidate, self.settings) assert os.path.splitext(self.candidate.path)[1] == b".png" - self.assertExists(self.candidate.path) + assert Path(os.fsdecode(self.candidate.path)).exists() class FSArtTest(UseThePlugin): @@ -748,8 +749,8 @@ class ArtImporterTest(UseThePlugin): super().setUp() # Mock the album art fetcher to always return our test file. - self.art_file = os.path.join(self.temp_dir, b"tmpcover.jpg") - _common.touch(self.art_file) + self.art_file = self.temp_dir_path / "tmpcover.jpg" + self.art_file.touch() self.old_afa = self.plugin.art_for_album self.afa_response = fetchart.Candidate( logger, @@ -804,12 +805,10 @@ class ArtImporterTest(UseThePlugin): self.plugin.fetch_art(self.session, self.task) self.plugin.assign_art(self.session, self.task) - artpath = self.lib.albums()[0].artpath + artpath = self.lib.albums()[0].art_filepath if should_exist: - assert artpath == os.path.join( - os.path.dirname(self.i.path), b"cover.jpg" - ) - self.assertExists(artpath) + assert artpath == self.i.filepath.parent / "cover.jpg" + assert artpath.exists() else: assert artpath is None return artpath @@ -828,20 +827,20 @@ class ArtImporterTest(UseThePlugin): def test_leave_original_file_in_place(self): self._fetch_art(True) - self.assertExists(self.art_file) + assert self.art_file.exists() def test_delete_original_file(self): prev_move = config["import"]["move"].get() try: config["import"]["move"] = True self._fetch_art(True) - self.assertNotExists(self.art_file) + assert not self.art_file.exists() finally: config["import"]["move"] = prev_move def test_do_not_delete_original_if_already_in_place(self): artdest = os.path.join(os.path.dirname(self.i.path), b"cover.jpg") - shutil.copyfile(syspath(self.art_file), syspath(artdest)) + shutil.copyfile(self.art_file, syspath(artdest)) self.afa_response = fetchart.Candidate( logger, source_name="test", @@ -861,156 +860,135 @@ class ArtImporterTest(UseThePlugin): self.plugin.batch_fetch_art( self.lib, self.lib.albums(), force=False, quiet=False ) - self.assertExists(self.album.artpath) + assert self.album.art_filepath.exists() -class ArtForAlbumTest(UseThePlugin): - """Tests that fetchart.art_for_album respects the scale & filesize - configurations (e.g., minwidth, enforce_ratio, max_filesize) +class AlbumArtOperationTestCase(UseThePlugin): + """Base test case for album art operations. + + Provides common setup for testing album art processing operations by setting + up a mock filesystem source that returns a predefined test image. """ - IMG_225x225 = os.path.join(_common.RSRC, b"abbey.jpg") - IMG_348x348 = os.path.join(_common.RSRC, b"abbey-different.jpg") - IMG_500x490 = os.path.join(_common.RSRC, b"abbey-similar.jpg") + IMAGE_PATH = os.path.join(_common.RSRC, b"abbey-similar.jpg") + IMAGE_FILESIZE = os.stat(util.syspath(IMAGE_PATH)).st_size + IMAGE_WIDTH = 500 + IMAGE_HEIGHT = 490 + IMAGE_WIDTH_HEIGHT_DIFF = IMAGE_WIDTH - IMAGE_HEIGHT - IMG_225x225_SIZE = os.stat(util.syspath(IMG_225x225)).st_size - IMG_348x348_SIZE = os.stat(util.syspath(IMG_348x348)).st_size - - RESIZE_OP = "resize" - DEINTERLACE_OP = "deinterlace" - REFORMAT_OP = "reformat" - - def setUp(self): - super().setUp() - - self.old_fs_source_get = fetchart.FileSystem.get + @classmethod + def setUpClass(cls): + super().setUpClass() def fs_source_get(_self, album, settings, paths): if paths: yield fetchart.Candidate( - logger, source_name=_self.ID, path=self.image_file + logger, source_name=_self.ID, path=cls.IMAGE_PATH ) - fetchart.FileSystem.get = fs_source_get + patch("beetsplug.fetchart.FileSystem.get", fs_source_get).start() + cls.addClassCleanup(patch.stopall) - self.album = _common.Bag() + def get_album_art(self): + return self.plugin.art_for_album(_common.Bag(), [""], True) - def tearDown(self): - fetchart.FileSystem.get = self.old_fs_source_get - super().tearDown() - def assertImageIsValidArt(self, image_file, should_exist): - self.assertExists(image_file) - self.image_file = image_file +class AlbumArtOperationConfigurationTest(AlbumArtOperationTestCase): + """Check that scale & filesize configuration is respected. - candidate = self.plugin.art_for_album(self.album, [""], True) + Depending on `minwidth`, `enforce_ratio`, `margin_px`, and `margin_percent` + configuration the plugin should or should not return an art candidate. + """ - if should_exist: - assert candidate is not None - assert candidate.path == self.image_file - self.assertExists(candidate.path) - else: - assert candidate is None + def test_minwidth(self): + self.plugin.minwidth = self.IMAGE_WIDTH / 2 + assert self.get_album_art() - def _assert_image_operated(self, image_file, operation, should_operate): - self.image_file = image_file - with patch.object( - ArtResizer.shared, operation, return_value=self.image_file - ) as mock_operation: - self.plugin.art_for_album(self.album, [""], True) - assert mock_operation.called == should_operate + self.plugin.minwidth = self.IMAGE_WIDTH * 2 + assert not self.get_album_art() - def _require_backend(self): - """Skip the test if the art resizer doesn't have ImageMagick or - PIL (so comparisons and measurements are unavailable). - """ - if not ArtResizer.shared.local: - self.skipTest("ArtResizer has no local imaging backend available") - - def test_respect_minwidth(self): - self._require_backend() - self.plugin.minwidth = 300 - self.assertImageIsValidArt(self.IMG_225x225, False) - self.assertImageIsValidArt(self.IMG_348x348, True) - - def test_respect_enforce_ratio_yes(self): - self._require_backend() + def test_enforce_ratio(self): self.plugin.enforce_ratio = True - self.assertImageIsValidArt(self.IMG_500x490, False) - self.assertImageIsValidArt(self.IMG_225x225, True) + assert not self.get_album_art() - def test_respect_enforce_ratio_no(self): self.plugin.enforce_ratio = False - self.assertImageIsValidArt(self.IMG_500x490, True) + assert self.get_album_art() - def test_respect_enforce_ratio_px_above(self): - self._require_backend() + def test_enforce_ratio_with_px_margin(self): self.plugin.enforce_ratio = True - self.plugin.margin_px = 5 - self.assertImageIsValidArt(self.IMG_500x490, False) - def test_respect_enforce_ratio_px_below(self): - self._require_backend() + self.plugin.margin_px = self.IMAGE_WIDTH_HEIGHT_DIFF * 0.5 + assert not self.get_album_art() + + self.plugin.margin_px = self.IMAGE_WIDTH_HEIGHT_DIFF * 1.5 + assert self.get_album_art() + + def test_enforce_ratio_with_percent_margin(self): self.plugin.enforce_ratio = True - self.plugin.margin_px = 15 - self.assertImageIsValidArt(self.IMG_500x490, True) + diff_by_width = self.IMAGE_WIDTH_HEIGHT_DIFF / self.IMAGE_WIDTH - def test_respect_enforce_ratio_percent_above(self): - self._require_backend() - self.plugin.enforce_ratio = True - self.plugin.margin_percent = (500 - 490) / 500 * 0.5 - self.assertImageIsValidArt(self.IMG_500x490, False) + self.plugin.margin_percent = diff_by_width * 0.5 + assert not self.get_album_art() - def test_respect_enforce_ratio_percent_below(self): - self._require_backend() - self.plugin.enforce_ratio = True - self.plugin.margin_percent = (500 - 490) / 500 * 1.5 - self.assertImageIsValidArt(self.IMG_500x490, True) + self.plugin.margin_percent = diff_by_width * 1.5 + assert self.get_album_art() - def test_resize_if_necessary(self): - self._require_backend() - self.plugin.maxwidth = 300 - self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False) - self._assert_image_operated(self.IMG_348x348, self.RESIZE_OP, True) - def test_fileresize(self): - self._require_backend() - self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 - self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) +class AlbumArtPerformOperationTest(AlbumArtOperationTestCase): + """Test that the art is resized and deinterlaced if necessary.""" - def test_fileresize_if_necessary(self): - self._require_backend() - self.plugin.max_filesize = self.IMG_225x225_SIZE - self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False) - self.assertImageIsValidArt(self.IMG_225x225, True) + def setUp(self): + super().setUp() + self.resizer_mock = patch.object( + ArtResizer.shared, "resize", return_value=self.IMAGE_PATH + ).start() + self.deinterlacer_mock = patch.object( + ArtResizer.shared, "deinterlace", return_value=self.IMAGE_PATH + ).start() - def test_fileresize_no_scale(self): - self._require_backend() - self.plugin.maxwidth = 300 - self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 - self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) + def test_resize(self): + self.plugin.maxwidth = self.IMAGE_WIDTH / 2 + assert self.get_album_art() + assert self.resizer_mock.called - def test_fileresize_and_scale(self): - self._require_backend() - self.plugin.maxwidth = 200 - self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 - self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) + def test_file_resized(self): + self.plugin.max_filesize = self.IMAGE_FILESIZE // 2 + assert self.get_album_art() + assert self.resizer_mock.called - def test_deinterlace(self): - self._require_backend() + def test_file_not_resized(self): + self.plugin.max_filesize = self.IMAGE_FILESIZE + assert self.get_album_art() + assert not self.resizer_mock.called + + def test_file_resized_but_not_scaled(self): + self.plugin.maxwidth = self.IMAGE_WIDTH * 2 + self.plugin.max_filesize = self.IMAGE_FILESIZE // 2 + assert self.get_album_art() + assert self.resizer_mock.called + + def test_file_resized_and_scaled(self): + self.plugin.maxwidth = self.IMAGE_WIDTH / 2 + self.plugin.max_filesize = self.IMAGE_FILESIZE // 2 + assert self.get_album_art() + assert self.resizer_mock.called + + def test_deinterlaced(self): self.plugin.deinterlace = True - self._assert_image_operated(self.IMG_225x225, self.DEINTERLACE_OP, True) + assert self.get_album_art() + assert self.deinterlacer_mock.called + + def test_not_deinterlaced(self): self.plugin.deinterlace = False - self._assert_image_operated( - self.IMG_225x225, self.DEINTERLACE_OP, False - ) + assert self.get_album_art() + assert not self.deinterlacer_mock.called - def test_deinterlace_and_resize(self): - self._require_backend() - self.plugin.maxwidth = 300 + def test_deinterlaced_and_resized(self): + self.plugin.maxwidth = self.IMAGE_WIDTH / 2 self.plugin.deinterlace = True - self._assert_image_operated(self.IMG_348x348, self.DEINTERLACE_OP, True) - self._assert_image_operated(self.IMG_348x348, self.RESIZE_OP, True) + assert self.get_album_art() + assert self.deinterlacer_mock.called + assert self.resizer_mock.called class DeprecatedConfigTest(unittest.TestCase): diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 6dd28337a..dcf684ccc 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -18,6 +18,7 @@ import os.path import re import sys import unittest +from pathlib import Path import pytest from mediafile import MediaFile @@ -32,7 +33,6 @@ from beets.test.helper import ( capture_log, control_stdin, ) -from beets.util import bytestring_path, displayable_path from beetsplug import convert @@ -58,31 +58,11 @@ class ConvertMixin: shell_quote(sys.executable), shell_quote(stub), tag ) - def assertFileTag(self, path, tag): - """Assert that the path is a file and the files content ends - with `tag`. - """ - display_tag = tag - tag = tag.encode("utf-8") - self.assertIsFile(path) - with open(path, "rb") as f: - f.seek(-len(display_tag), os.SEEK_END) - assert f.read() == tag, ( - f"{displayable_path(path)} is not tagged with {display_tag}" - ) - - def assertNoFileTag(self, path, tag): - """Assert that the path is a file and the files content does not - end with `tag`. - """ - display_tag = tag - tag = tag.encode("utf-8") - self.assertIsFile(path) - with open(path, "rb") as f: - f.seek(-len(tag), os.SEEK_END) - assert f.read() != tag, ( - f"{displayable_path(path)} is unexpectedly tagged with {display_tag}" - ) + def file_endswith(self, path: Path, tag: str): + """Check the path is a file and if its content ends with `tag`.""" + assert path.exists() + assert path.is_file() + return path.read_bytes().endswith(tag.encode("utf-8")) class ConvertTestCase(ConvertMixin, PluginTestCase): @@ -106,7 +86,7 @@ class ImportConvertTest(AsIsImporterMixin, ImportHelper, ConvertTestCase): def test_import_converted(self): self.run_asis_importer() item = self.lib.items().get() - self.assertFileTag(item.path, "convert") + assert self.file_endswith(item.filepath, "convert") # FIXME: fails on windows @unittest.skipIf(sys.platform == "win32", "win32") @@ -117,7 +97,7 @@ class ImportConvertTest(AsIsImporterMixin, ImportHelper, ConvertTestCase): item = self.lib.items().get() assert item is not None - self.assertIsFile(item.path) + assert item.filepath.is_file() def test_delete_originals(self): self.config["convert"]["delete_originals"] = True @@ -159,11 +139,10 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): self.album = self.add_album_fixture(ext="ogg") self.item = self.album.items()[0] - self.convert_dest = bytestring_path( - os.path.join(self.temp_dir, b"convert_dest") - ) + self.convert_dest = self.temp_dir_path / "convert_dest" + self.converted_mp3 = self.convert_dest / "converted.mp3" self.config["convert"] = { - "dest": self.convert_dest, + "dest": str(self.convert_dest), "paths": {"default": "converted"}, "format": "mp3", "formats": { @@ -179,19 +158,16 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_convert(self): with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + assert self.file_endswith(self.converted_mp3, "mp3") def test_convert_with_auto_confirmation(self): self.run_convert("--yes") - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + assert self.file_endswith(self.converted_mp3, "mp3") def test_reject_confirmation(self): with control_stdin("n"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertNotExists(converted) + assert not self.converted_mp3.exists() def test_convert_keep_new(self): assert os.path.splitext(self.item.path)[1] == b".ogg" @@ -205,8 +181,7 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_format_option(self): with control_stdin("y"): self.run_convert("--format", "opus") - converted = os.path.join(self.convert_dest, b"converted.ops") - self.assertFileTag(converted, "opus") + assert self.file_endswith(self.convert_dest / "converted.ops", "opus") def test_embed_album_art(self): self.config["convert"]["embed"] = True @@ -218,12 +193,11 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - mediafile = MediaFile(converted) + mediafile = MediaFile(self.converted_mp3) assert mediafile.images[0].data == image_data def test_skip_existing(self): - converted = os.path.join(self.convert_dest, b"converted.mp3") + converted = self.converted_mp3 self.touch(converted, content="XXX") self.run_convert("--yes") with open(converted) as f: @@ -231,8 +205,7 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_pretend(self): self.run_convert("--pretend") - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertNotExists(converted) + assert not self.converted_mp3.exists() def test_empty_query(self): with capture_log("beets.convert") as logs: @@ -243,55 +216,51 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): self.config["convert"]["max_bitrate"] = 5000 with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + assert self.file_endswith(self.converted_mp3, "mp3") def test_transcode_when_maxbr_set_low_and_different_formats(self): self.config["convert"]["max_bitrate"] = 5 with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + assert self.file_endswith(self.converted_mp3, "mp3") def test_transcode_when_maxbr_set_to_none_and_different_formats(self): with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + assert self.file_endswith(self.converted_mp3, "mp3") def test_no_transcode_when_maxbr_set_high_and_same_formats(self): self.config["convert"]["max_bitrate"] = 5000 self.config["convert"]["format"] = "ogg" with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.ogg") - self.assertNoFileTag(converted, "ogg") + assert not self.file_endswith( + self.convert_dest / "converted.ogg", "ogg" + ) def test_transcode_when_maxbr_set_low_and_same_formats(self): self.config["convert"]["max_bitrate"] = 5 self.config["convert"]["format"] = "ogg" with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.ogg") - self.assertFileTag(converted, "ogg") + assert self.file_endswith(self.convert_dest / "converted.ogg", "ogg") def test_transcode_when_maxbr_set_to_none_and_same_formats(self): self.config["convert"]["format"] = "ogg" with control_stdin("y"): self.run_convert() - converted = os.path.join(self.convert_dest, b"converted.ogg") - self.assertNoFileTag(converted, "ogg") + assert not self.file_endswith( + self.convert_dest / "converted.ogg", "ogg" + ) def test_playlist(self): with control_stdin("y"): self.run_convert("--playlist", "playlist.m3u8") - m3u_created = os.path.join(self.convert_dest, b"playlist.m3u8") - assert os.path.exists(m3u_created) + assert (self.convert_dest / "playlist.m3u8").exists() def test_playlist_pretend(self): self.run_convert("--playlist", "playlist.m3u8", "--pretend") - m3u_created = os.path.join(self.convert_dest, b"playlist.m3u8") - assert not os.path.exists(m3u_created) + assert not (self.convert_dest / "playlist.m3u8").exists() @_common.slow_test() @@ -301,9 +270,9 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): def setUp(self): super().setUp() - self.convert_dest = os.path.join(self.temp_dir, b"convert_dest") + self.convert_dest = self.temp_dir_path / "convert_dest" self.config["convert"] = { - "dest": self.convert_dest, + "dest": str(self.convert_dest), "paths": {"default": "converted"}, "never_convert_lossy_files": True, "format": "mp3", @@ -316,23 +285,23 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): [item] = self.add_item_fixtures(ext="flac") with control_stdin("y"): self.run_convert_path(item) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + converted = self.convert_dest / "converted.mp3" + assert self.file_endswith(converted, "mp3") def test_transcode_from_lossy(self): self.config["convert"]["never_convert_lossy_files"] = False [item] = self.add_item_fixtures(ext="ogg") with control_stdin("y"): self.run_convert_path(item) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFileTag(converted, "mp3") + converted = self.convert_dest / "converted.mp3" + assert self.file_endswith(converted, "mp3") def test_transcode_from_lossy_prevented(self): [item] = self.add_item_fixtures(ext="ogg") with control_stdin("y"): self.run_convert_path(item) - converted = os.path.join(self.convert_dest, b"converted.ogg") - self.assertNoFileTag(converted, "mp3") + converted = self.convert_dest / "converted.ogg" + assert not self.file_endswith(converted, "mp3") class TestNoConvert: diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 278e04b9e..4e6c97ab2 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -134,22 +134,6 @@ class EditCommandTest(EditMixin, BeetsTestCase): {f: item[f] for f in item._fields} for item in self.album.items() ] - def assertCounts( - self, - mock_write, - album_count=ALBUM_COUNT, - track_count=TRACK_COUNT, - write_call_count=TRACK_COUNT, - title_starts_with="", - ): - """Several common assertions on Album, Track and call counts.""" - assert len(self.lib.albums()) == album_count - assert len(self.lib.items()) == track_count - assert mock_write.call_count == write_call_count - assert all( - i.title.startswith(title_starts_with) for i in self.lib.items() - ) - def test_title_edit_discard(self, mock_write): """Edit title for all items in the library, then discard changes.""" # Edit track titles. @@ -159,9 +143,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ["c"], ) - self.assertCounts( - mock_write, write_call_count=0, title_starts_with="t\u00eftle" - ) + assert mock_write.call_count == 0 self.assertItemFieldsModified(self.album.items(), self.items_orig, []) def test_title_edit_apply(self, mock_write): @@ -173,11 +155,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ["a"], ) - self.assertCounts( - mock_write, - write_call_count=self.TRACK_COUNT, - title_starts_with="modified t\u00eftle", - ) + assert mock_write.call_count == self.TRACK_COUNT self.assertItemFieldsModified( self.album.items(), self.items_orig, ["title", "mtime"] ) @@ -191,10 +169,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ["a"], ) - self.assertCounts( - mock_write, - write_call_count=1, - ) + assert mock_write.call_count == 1 # No changes except on last item. self.assertItemFieldsModified( list(self.album.items())[:-1], self.items_orig[:-1], [] @@ -210,9 +185,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): [], ) - self.assertCounts( - mock_write, write_call_count=0, title_starts_with="t\u00eftle" - ) + assert mock_write.call_count == 0 self.assertItemFieldsModified(self.album.items(), self.items_orig, []) def test_album_edit_apply(self, mock_write): @@ -226,7 +199,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ["a"], ) - self.assertCounts(mock_write, write_call_count=self.TRACK_COUNT) + assert mock_write.call_count == self.TRACK_COUNT self.assertItemFieldsModified( self.album.items(), self.items_orig, ["album", "mtime"] ) @@ -249,9 +222,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): # Even though a flexible attribute was written (which is not directly # written to the tags), write should still be called since templates # might use it. - self.assertCounts( - mock_write, write_call_count=1, title_starts_with="t\u00eftle" - ) + assert mock_write.call_count == 1 def test_a_album_edit_apply(self, mock_write): """Album query (-a), edit album field, apply changes.""" @@ -263,7 +234,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ) self.album.load() - self.assertCounts(mock_write, write_call_count=self.TRACK_COUNT) + assert mock_write.call_count == self.TRACK_COUNT assert self.album.album == "modified \u00e4lbum" self.assertItemFieldsModified( self.album.items(), self.items_orig, ["album", "mtime"] @@ -279,7 +250,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ) self.album.load() - self.assertCounts(mock_write, write_call_count=self.TRACK_COUNT) + assert mock_write.call_count == self.TRACK_COUNT assert self.album.albumartist == "the modified album artist" self.assertItemFieldsModified( self.album.items(), self.items_orig, ["albumartist", "mtime"] @@ -295,9 +266,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): ["n"], ) - self.assertCounts( - mock_write, write_call_count=0, title_starts_with="t\u00eftle" - ) + assert mock_write.call_count == 0 def test_invalid_yaml(self, mock_write): """Edit the yaml file incorrectly (resulting in a well-formed but @@ -309,9 +278,7 @@ class EditCommandTest(EditMixin, BeetsTestCase): [], ) - self.assertCounts( - mock_write, write_call_count=0, title_starts_with="t\u00eftle" - ) + assert mock_write.call_count == 0 @_common.slow_test() diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index f2f02137b..62b2bb7d1 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -13,6 +13,7 @@ # included in all copies or substantial portions of the Software. +import os import os.path import shutil import tempfile @@ -24,7 +25,12 @@ from mediafile import MediaFile from beets import art, config, logging, ui from beets.test import _common -from beets.test.helper import BeetsTestCase, FetchImageHelper, PluginMixin +from beets.test.helper import ( + BeetsTestCase, + FetchImageHelper, + IOMixin, + PluginMixin, +) from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer from test.test_art_resize import DummyIMBackend @@ -68,17 +74,13 @@ def require_artresizer_compare(test): return wrapper -class EmbedartCliTest(PluginMixin, FetchImageHelper, BeetsTestCase): +class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): plugin = "embedart" small_artpath = os.path.join(_common.RSRC, b"image-2x3.jpg") abbey_artpath = os.path.join(_common.RSRC, b"abbey.jpg") abbey_similarpath = os.path.join(_common.RSRC, b"abbey-similar.jpg") abbey_differentpath = os.path.join(_common.RSRC, b"abbey-different.jpg") - def setUp(self): - super().setUp() # Converter is threaded - self.io.install() - def _setup_data(self, artpath=None): if not artpath: artpath = self.small_artpath @@ -202,23 +204,21 @@ class EmbedartCliTest(PluginMixin, FetchImageHelper, BeetsTestCase): resource_path = os.path.join(_common.RSRC, b"image.mp3") album = self.add_album_fixture() trackpath = album.items()[0].path - albumpath = album.path shutil.copy(syspath(resource_path), syspath(trackpath)) self.run_command("extractart", "-n", "extracted") - self.assertExists(os.path.join(albumpath, b"extracted.png")) + assert (album.filepath / "extracted.png").exists() def test_extracted_extension(self): resource_path = os.path.join(_common.RSRC, b"image-jpeg.mp3") album = self.add_album_fixture() trackpath = album.items()[0].path - albumpath = album.path shutil.copy(syspath(resource_path), syspath(trackpath)) self.run_command("extractart", "-n", "extracted") - self.assertExists(os.path.join(albumpath, b"extracted.jpg")) + assert (album.filepath / "extracted.jpg").exists() def test_clear_art_with_yes_input(self): self._setup_data() diff --git a/test/plugins/test_hook.py b/test/plugins/test_hook.py index 993b95911..d15de1cec 100644 --- a/test/plugins/test_hook.py +++ b/test/plugins/test_hook.py @@ -15,7 +15,7 @@ from __future__ import annotations -import os.path +import os import sys import unittest from contextlib import contextmanager @@ -74,8 +74,7 @@ class HookCommandTest(HookTestCase): def setUp(self): super().setUp() - temp_dir = os.fsdecode(self.temp_dir) - self.paths = [os.path.join(temp_dir, e) for e in self.events] + self.paths = [str(self.temp_dir_path / e) for e in self.events] def _test_command( self, diff --git a/test/plugins/test_importadded.py b/test/plugins/test_importadded.py index d54c04b0e..1b198b31d 100644 --- a/test/plugins/test_importadded.py +++ b/test/plugins/test_importadded.py @@ -68,26 +68,23 @@ class ImportAddedTest(PluginMixin, AutotagImportTestCase): "No MediaFile found for Item " + displayable_path(item.path) ) - def assertEqualTimes(self, first, second, msg=None): - """For comparing file modification times at a sufficient precision""" - assert first == pytest.approx(second, rel=1e-4), msg - - def assertAlbumImport(self): + def test_import_album_with_added_dates(self): self.importer.run() + album = self.lib.albums().get() assert album.added == self.min_mtime for item in album.items(): assert item.added == self.min_mtime - def test_import_album_with_added_dates(self): - self.assertAlbumImport() - def test_import_album_inplace_with_added_dates(self): self.config["import"]["copy"] = False - self.config["import"]["move"] = False - self.config["import"]["link"] = False - self.config["import"]["hardlink"] = False - self.assertAlbumImport() + + self.importer.run() + + album = self.lib.albums().get() + assert album.added == self.min_mtime + for item in album.items(): + assert item.added == self.min_mtime def test_import_album_with_preserved_mtimes(self): self.config["importadded"]["preserve_mtimes"] = True @@ -95,10 +92,12 @@ class ImportAddedTest(PluginMixin, AutotagImportTestCase): album = self.lib.albums().get() assert album.added == self.min_mtime for item in album.items(): - self.assertEqualTimes(item.added, self.min_mtime) + assert item.added == pytest.approx(self.min_mtime, rel=1e-4) mediafile_mtime = os.path.getmtime(self.find_media_file(item).path) - self.assertEqualTimes(item.mtime, mediafile_mtime) - self.assertEqualTimes(os.path.getmtime(item.path), mediafile_mtime) + assert item.mtime == pytest.approx(mediafile_mtime, rel=1e-4) + assert os.path.getmtime(item.path) == pytest.approx( + mediafile_mtime, rel=1e-4 + ) def test_reimported_album_skipped(self): # Import and record the original added dates @@ -113,22 +112,21 @@ class ImportAddedTest(PluginMixin, AutotagImportTestCase): self.importer.run() # Verify the reimported items album = self.lib.albums().get() - self.assertEqualTimes(album.added, album_added_before) + assert album.added == pytest.approx(album_added_before, rel=1e-4) items_added_after = {item.path: item.added for item in album.items()} for item_path, added_after in items_added_after.items(): - self.assertEqualTimes( - items_added_before[item_path], - added_after, - "reimport modified Item.added for " - + displayable_path(item_path), - ) + assert items_added_before[item_path] == pytest.approx( + added_after, rel=1e-4 + ), "reimport modified Item.added for " + displayable_path(item_path) def test_import_singletons_with_added_dates(self): self.config["import"]["singletons"] = True self.importer.run() for item in self.lib.items(): mfile = self.find_media_file(item) - self.assertEqualTimes(item.added, os.path.getmtime(mfile.path)) + assert item.added == pytest.approx( + os.path.getmtime(mfile.path), rel=1e-4 + ) def test_import_singletons_with_preserved_mtimes(self): self.config["import"]["singletons"] = True @@ -136,9 +134,11 @@ class ImportAddedTest(PluginMixin, AutotagImportTestCase): self.importer.run() for item in self.lib.items(): mediafile_mtime = os.path.getmtime(self.find_media_file(item).path) - self.assertEqualTimes(item.added, mediafile_mtime) - self.assertEqualTimes(item.mtime, mediafile_mtime) - self.assertEqualTimes(os.path.getmtime(item.path), mediafile_mtime) + assert item.added == pytest.approx(mediafile_mtime, rel=1e-4) + assert item.mtime == pytest.approx(mediafile_mtime, rel=1e-4) + assert os.path.getmtime(item.path) == pytest.approx( + mediafile_mtime, rel=1e-4 + ) def test_reimported_singletons_skipped(self): self.config["import"]["singletons"] = True @@ -155,9 +155,6 @@ class ImportAddedTest(PluginMixin, AutotagImportTestCase): # Verify the reimported items items_added_after = {item.path: item.added for item in self.lib.items()} for item_path, added_after in items_added_after.items(): - self.assertEqualTimes( - items_added_before[item_path], - added_after, - "reimport modified Item.added for " - + displayable_path(item_path), - ) + assert items_added_before[item_path] == pytest.approx( + added_after, rel=1e-4 + ), "reimport modified Item.added for " + displayable_path(item_path) diff --git a/test/plugins/test_importfeeds.py b/test/plugins/test_importfeeds.py index 5f1f915ad..d525bd801 100644 --- a/test/plugins/test_importfeeds.py +++ b/test/plugins/test_importfeeds.py @@ -12,8 +12,8 @@ class ImportfeedsTestTest(BeetsTestCase): def setUp(self): super().setUp() self.importfeeds = ImportFeedsPlugin() - self.feeds_dir = os.path.join(os.fsdecode(self.temp_dir), "importfeeds") - config["importfeeds"]["dir"] = self.feeds_dir + self.feeds_dir = self.temp_dir_path / "importfeeds" + config["importfeeds"]["dir"] = str(self.feeds_dir) def test_multi_format_album_playlist(self): config["importfeeds"]["formats"] = "m3u_multi" @@ -24,10 +24,8 @@ class ImportfeedsTestTest(BeetsTestCase): self.lib.add(item) self.importfeeds.album_imported(self.lib, album) - playlist_path = os.path.join( - self.feeds_dir, os.listdir(self.feeds_dir)[0] - ) - assert playlist_path.endswith("album_name.m3u") + playlist_path = self.feeds_dir / next(self.feeds_dir.iterdir()) + assert str(playlist_path).endswith("album_name.m3u") with open(playlist_path) as playlist: assert item_path in playlist.read() @@ -43,9 +41,7 @@ class ImportfeedsTestTest(BeetsTestCase): self.lib.add(item) self.importfeeds.album_imported(self.lib, album) - playlist = os.path.join( - self.feeds_dir, config["importfeeds"]["m3u_name"].get() - ) + playlist = self.feeds_dir / config["importfeeds"]["m3u_name"].get() playlist_subdir = os.path.dirname(playlist) assert os.path.isdir(playlist_subdir) assert os.path.isfile(playlist) @@ -62,7 +58,7 @@ class ImportfeedsTestTest(BeetsTestCase): self.importfeeds.import_begin(self) self.importfeeds.album_imported(self.lib, album) date = datetime.datetime.now().strftime("%Y%m%d_%Hh%M") - playlist = os.path.join(self.feeds_dir, f"imports_{date}.m3u") + playlist = self.feeds_dir / f"imports_{date}.m3u" assert os.path.isfile(playlist) with open(playlist) as playlist_contents: assert item_path in playlist_contents.read() diff --git a/test/plugins/test_permissions.py b/test/plugins/test_permissions.py index 274cd92ac..475e98194 100644 --- a/test/plugins/test_permissions.py +++ b/test/plugins/test_permissions.py @@ -6,7 +6,6 @@ from unittest.mock import Mock, patch from beets.test._common import touch from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin -from beets.util import displayable_path from beetsplug.permissions import ( check_permissions, convert_perm, @@ -23,57 +22,25 @@ class PermissionsPluginTest(AsIsImporterMixin, PluginMixin, ImportTestCase): self.config["permissions"] = {"file": "777", "dir": "777"} def test_permissions_on_album_imported(self): - self.do_thing(True) + self.import_and_check_permissions() def test_permissions_on_item_imported(self): self.config["import"]["singletons"] = True - self.do_thing(True) + self.import_and_check_permissions() - @patch("os.chmod", Mock()) - def test_failing_to_set_permissions(self): - self.do_thing(False) - - def do_thing(self, expect_success): + def import_and_check_permissions(self): if platform.system() == "Windows": self.skipTest("permissions not available on Windows") - def get_stat(v): - return ( - os.stat(os.path.join(self.temp_dir, b"import", *v)).st_mode - & 0o777 - ) - - typs = ["file", "dir"] - - track_file = (b"album", b"track_1.mp3") - self.exp_perms = { - True: { - k: convert_perm(self.config["permissions"][k].get()) - for k in typs - }, - False: {k: get_stat(v) for (k, v) in zip(typs, (track_file, ()))}, - } + track_file = os.path.join(self.import_dir, b"album", b"track_1.mp3") + assert os.stat(track_file).st_mode & 0o777 != 511 self.run_asis_importer() item = self.lib.items().get() - self.assertPerms(item.path, "file", expect_success) - - for path in dirs_in_library(self.lib.directory, item.path): - self.assertPerms(path, "dir", expect_success) - - def assertPerms(self, path, typ, expect_success): - for x in [ - (True, self.exp_perms[expect_success][typ], "!="), - (False, self.exp_perms[not expect_success][typ], "=="), - ]: - msg = "{} : {} {} {}".format( - displayable_path(path), - oct(os.stat(path).st_mode), - x[2], - oct(x[1]), - ) - assert x[0] == check_permissions(path, x[1]), msg + paths = (item.path, *dirs_in_library(self.lib.directory, item.path)) + for path in paths: + assert os.stat(path).st_mode & 0o777 == 511 def test_convert_perm_from_string(self): assert convert_perm("10") == 8 diff --git a/test/plugins/test_player.py b/test/plugins/test_player.py index b17a78c17..a7c613d8f 100644 --- a/test/plugins/test_player.py +++ b/test/plugins/test_player.py @@ -311,7 +311,7 @@ class BPDTestHelper(PluginTestCase): """ # Create a config file: config = { - "pluginpath": [os.fsdecode(self.temp_dir)], + "pluginpath": [str(self.temp_dir_path)], "plugins": "bpd", # use port 0 to let the OS choose a free port "bpd": {"host": host, "port": 0, "control_port": 0}, @@ -320,7 +320,7 @@ class BPDTestHelper(PluginTestCase): config["bpd"]["password"] = password config_file = tempfile.NamedTemporaryFile( mode="wb", - dir=os.fsdecode(self.temp_dir), + dir=str(self.temp_dir_path), suffix=".yaml", delete=False, ) diff --git a/test/plugins/test_playlist.py b/test/plugins/test_playlist.py index ee4059b70..9d9ce0303 100644 --- a/test/plugins/test_playlist.py +++ b/test/plugins/test_playlist.py @@ -72,12 +72,10 @@ class PlaylistTestCase(PluginTestCase): self.lib.add(i3) self.lib.add_album([i3]) - self.playlist_dir = os.path.join( - os.fsdecode(self.temp_dir), "playlists" - ) - os.makedirs(self.playlist_dir) + self.playlist_dir = self.temp_dir_path / "playlists" + self.playlist_dir.mkdir(parents=True, exist_ok=True) self.config["directory"] = self.music_dir - self.config["playlist"]["playlist_dir"] = self.playlist_dir + self.config["playlist"]["playlist_dir"] = str(self.playlist_dir) self.setup_test() self.load_plugins() @@ -222,7 +220,7 @@ class PlaylistTestRelativeToPls(PlaylistQueryTest, PlaylistTestCase): ) self.config["playlist"]["relative_to"] = "playlist" - self.config["playlist"]["playlist_dir"] = self.playlist_dir + self.config["playlist"]["playlist_dir"] = str(self.playlist_dir) class PlaylistUpdateTest: diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index ade745c17..c8e516e8b 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -13,7 +13,8 @@ # included in all copies or substantial portions of the Software. -from os import fsdecode, path, remove +from os import path, remove +from pathlib import Path from shutil import rmtree from tempfile import mkdtemp from unittest.mock import MagicMock, Mock, PropertyMock @@ -26,7 +27,7 @@ from beets.dbcore.query import FixedFieldSort, MultipleSort, NullSort from beets.library import Album, Item, parse_query_string from beets.test.helper import BeetsTestCase, PluginTestCase from beets.ui import UserError -from beets.util import CHAR_REPLACE, bytestring_path, syspath +from beets.util import CHAR_REPLACE, syspath from beetsplug.smartplaylist import SmartPlaylistPlugin @@ -165,9 +166,9 @@ class SmartPlaylistTest(BeetsTestCase): pl = b"$title-my.m3u", (q, None), (a_q, None) spl._matched_playlists = [pl] - dir = bytestring_path(mkdtemp()) + dir = mkdtemp() config["smartplaylist"]["relative_to"] = False - config["smartplaylist"]["playlist_dir"] = fsdecode(dir) + config["smartplaylist"]["playlist_dir"] = str(dir) try: spl.update_playlists(lib) except Exception: @@ -177,10 +178,9 @@ class SmartPlaylistTest(BeetsTestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, b"ta_ga_da-my_playlist_.m3u") - self.assertExists(m3u_filepath) - with open(syspath(m3u_filepath), "rb") as f: - content = f.read() + m3u_filepath = Path(dir, "ta_ga_da-my_playlist_.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() rmtree(syspath(dir)) assert content == b"/tagada.mp3\n" @@ -208,11 +208,11 @@ class SmartPlaylistTest(BeetsTestCase): pl = b"$title-my.m3u", (q, None), (a_q, None) spl._matched_playlists = [pl] - dir = bytestring_path(mkdtemp()) + dir = mkdtemp() config["smartplaylist"]["output"] = "extm3u" config["smartplaylist"]["prefix"] = "http://beets:8337/files" config["smartplaylist"]["relative_to"] = False - config["smartplaylist"]["playlist_dir"] = fsdecode(dir) + config["smartplaylist"]["playlist_dir"] = str(dir) try: spl.update_playlists(lib) except Exception: @@ -222,10 +222,9 @@ class SmartPlaylistTest(BeetsTestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, b"ta_ga_da-my_playlist_.m3u") - self.assertExists(m3u_filepath) - with open(syspath(m3u_filepath), "rb") as f: - content = f.read() + m3u_filepath = Path(dir, "ta_ga_da-my_playlist_.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() rmtree(syspath(dir)) assert ( @@ -260,10 +259,10 @@ class SmartPlaylistTest(BeetsTestCase): pl = b"$title-my.m3u", (q, None), (a_q, None) spl._matched_playlists = [pl] - dir = bytestring_path(mkdtemp()) + dir = mkdtemp() config["smartplaylist"]["output"] = "extm3u" config["smartplaylist"]["relative_to"] = False - config["smartplaylist"]["playlist_dir"] = fsdecode(dir) + config["smartplaylist"]["playlist_dir"] = str(dir) config["smartplaylist"]["fields"] = ["id", "genre"] try: spl.update_playlists(lib) @@ -274,10 +273,9 @@ class SmartPlaylistTest(BeetsTestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, b"ta_ga_da-my_playlist_.m3u") - self.assertExists(m3u_filepath) - with open(syspath(m3u_filepath), "rb") as f: - content = f.read() + m3u_filepath = Path(dir, "ta_ga_da-my_playlist_.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() rmtree(syspath(dir)) assert ( @@ -307,10 +305,10 @@ class SmartPlaylistTest(BeetsTestCase): pl = b"$title-my.m3u", (q, None), (a_q, None) spl._matched_playlists = [pl] - dir = bytestring_path(mkdtemp()) + dir = mkdtemp() tpl = "http://beets:8337/item/$id/file" config["smartplaylist"]["uri_format"] = tpl - config["smartplaylist"]["playlist_dir"] = fsdecode(dir) + config["smartplaylist"]["playlist_dir"] = dir # The following options should be ignored when uri_format is set config["smartplaylist"]["relative_to"] = "/data" config["smartplaylist"]["prefix"] = "/prefix" @@ -324,10 +322,9 @@ class SmartPlaylistTest(BeetsTestCase): lib.items.assert_called_once_with(q, None) lib.albums.assert_called_once_with(a_q, None) - m3u_filepath = path.join(dir, b"ta_ga_da-my_playlist_.m3u") - self.assertExists(m3u_filepath) - with open(syspath(m3u_filepath), "rb") as f: - content = f.read() + m3u_filepath = Path(dir, "ta_ga_da-my_playlist_.m3u") + assert m3u_filepath.exists() + content = m3u_filepath.read_bytes() rmtree(syspath(dir)) assert content == b"http://beets:8337/item/3/file\n" @@ -346,22 +343,20 @@ class SmartPlaylistCLITest(PluginTestCase): {"name": "all.m3u", "query": ""}, ] ) - config["smartplaylist"]["playlist_dir"].set(fsdecode(self.temp_dir)) + config["smartplaylist"]["playlist_dir"].set(str(self.temp_dir_path)) def test_splupdate(self): with pytest.raises(UserError): self.run_with_output("splupdate", "tagada") self.run_with_output("splupdate", "my_playlist") - m3u_path = path.join(self.temp_dir, b"my_playlist.m3u") - self.assertExists(m3u_path) - with open(syspath(m3u_path), "rb") as f: - assert f.read() == self.item.path + b"\n" + m3u_path = self.temp_dir_path / "my_playlist.m3u" + assert m3u_path.exists() + assert m3u_path.read_bytes() == self.item.path + b"\n" remove(syspath(m3u_path)) self.run_with_output("splupdate", "my_playlist.m3u") - with open(syspath(m3u_path), "rb") as f: - assert f.read() == self.item.path + b"\n" + assert m3u_path.read_bytes() == self.item.path + b"\n" remove(syspath(m3u_path)) self.run_with_output("splupdate") diff --git a/test/plugins/test_thumbnails.py b/test/plugins/test_thumbnails.py index bd3e22714..fadac34c2 100644 --- a/test/plugins/test_thumbnails.py +++ b/test/plugins/test_thumbnails.py @@ -232,8 +232,7 @@ class ThumbnailsTest(BeetsTestCase): ) @patch("beetsplug.thumbnails.ThumbnailsPlugin._check_local_ok", Mock()) - @patch("beetsplug.thumbnails.decargs") - def test_invokations(self, mock_decargs): + def test_invokations(self): plugin = ThumbnailsPlugin() plugin.process_album = Mock() album = Mock() @@ -243,7 +242,6 @@ class ThumbnailsTest(BeetsTestCase): album2 = Mock() lib.albums.return_value = [album, album2] plugin.process_query(lib, Mock(), None) - lib.albums.assert_called_once_with(mock_decargs.return_value) plugin.process_album.assert_has_calls( [call(album), call(album2)], any_order=True ) diff --git a/test/test_art_resize.py b/test/test_art_resize.py index 8dd4d0e89..34bf810b9 100644 --- a/test/test_art_resize.py +++ b/test/test_art_resize.py @@ -16,6 +16,7 @@ import os import unittest +from pathlib import Path from unittest.mock import patch from beets.test import _common @@ -65,7 +66,7 @@ class ArtResizerFileSizeTest(CleanupModulesMixin, BeetsTestCase): max_filesize=0, ) # check valid path returned - max_filesize hasn't broken resize command - self.assertExists(im_95_qual) + assert Path(os.fsdecode(im_95_qual)).exists() # Attempt a lower filesize with same quality im_a = backend.resize( @@ -74,7 +75,7 @@ class ArtResizerFileSizeTest(CleanupModulesMixin, BeetsTestCase): quality=95, max_filesize=0.9 * os.stat(syspath(im_95_qual)).st_size, ) - self.assertExists(im_a) + assert Path(os.fsdecode(im_a)).exists() # target size was achieved assert ( os.stat(syspath(im_a)).st_size @@ -88,7 +89,7 @@ class ArtResizerFileSizeTest(CleanupModulesMixin, BeetsTestCase): quality=75, max_filesize=0, ) - self.assertExists(im_75_qual) + assert Path(os.fsdecode(im_75_qual)).exists() im_b = backend.resize( 225, @@ -96,7 +97,7 @@ class ArtResizerFileSizeTest(CleanupModulesMixin, BeetsTestCase): quality=95, max_filesize=0.9 * os.stat(syspath(im_75_qual)).st_size, ) - self.assertExists(im_b) + assert Path(os.fsdecode(im_b)).exists() # Check high (initial) quality still gives a smaller filesize assert ( os.stat(syspath(im_b)).st_size diff --git a/test/test_datequery.py b/test/test_datequery.py index 9c968e998..1063a62c1 100644 --- a/test/test_datequery.py +++ b/test/test_datequery.py @@ -29,122 +29,68 @@ from beets.dbcore.query import ( from beets.test.helper import ItemInDBTestCase -def _date(string): - return datetime.strptime(string, "%Y-%m-%dT%H:%M:%S") +class TestDateInterval: + now = datetime.now().replace(microsecond=0, second=0).isoformat() - -def _datepattern(datetimedate): - return datetimedate.strftime("%Y-%m-%dT%H:%M:%S") - - -class DateIntervalTest(unittest.TestCase): - def test_year_precision_intervals(self): - self.assertContains("2000..2001", "2000-01-01T00:00:00") - self.assertContains("2000..2001", "2001-06-20T14:15:16") - self.assertContains("2000..2001", "2001-12-31T23:59:59") - self.assertExcludes("2000..2001", "1999-12-31T23:59:59") - self.assertExcludes("2000..2001", "2002-01-01T00:00:00") - - self.assertContains("2000..", "2000-01-01T00:00:00") - self.assertContains("2000..", "2099-10-11T00:00:00") - self.assertExcludes("2000..", "1999-12-31T23:59:59") - - self.assertContains("..2001", "2001-12-31T23:59:59") - self.assertExcludes("..2001", "2002-01-01T00:00:00") - - self.assertContains("-1d..1d", _datepattern(datetime.now())) - self.assertExcludes("-2d..-1d", _datepattern(datetime.now())) - - def test_day_precision_intervals(self): - self.assertContains("2000-06-20..2000-06-20", "2000-06-20T00:00:00") - self.assertContains("2000-06-20..2000-06-20", "2000-06-20T10:20:30") - self.assertContains("2000-06-20..2000-06-20", "2000-06-20T23:59:59") - self.assertExcludes("2000-06-20..2000-06-20", "2000-06-19T23:59:59") - self.assertExcludes("2000-06-20..2000-06-20", "2000-06-21T00:00:00") - - def test_month_precision_intervals(self): - self.assertContains("1999-12..2000-02", "1999-12-01T00:00:00") - self.assertContains("1999-12..2000-02", "2000-02-15T05:06:07") - self.assertContains("1999-12..2000-02", "2000-02-29T23:59:59") - self.assertExcludes("1999-12..2000-02", "1999-11-30T23:59:59") - self.assertExcludes("1999-12..2000-02", "2000-03-01T00:00:00") - - def test_hour_precision_intervals(self): - # test with 'T' separator - self.assertExcludes( - "2000-01-01T12..2000-01-01T13", "2000-01-01T11:59:59" - ) - self.assertContains( - "2000-01-01T12..2000-01-01T13", "2000-01-01T12:00:00" - ) - self.assertContains( - "2000-01-01T12..2000-01-01T13", "2000-01-01T12:30:00" - ) - self.assertContains( - "2000-01-01T12..2000-01-01T13", "2000-01-01T13:30:00" - ) - self.assertContains( - "2000-01-01T12..2000-01-01T13", "2000-01-01T13:59:59" - ) - self.assertExcludes( - "2000-01-01T12..2000-01-01T13", "2000-01-01T14:00:00" - ) - self.assertExcludes( - "2000-01-01T12..2000-01-01T13", "2000-01-01T14:30:00" - ) - - # test non-range query - self.assertContains("2008-12-01T22", "2008-12-01T22:30:00") - self.assertExcludes("2008-12-01T22", "2008-12-01T23:30:00") - - def test_minute_precision_intervals(self): - self.assertExcludes( - "2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:29:59" - ) - self.assertContains( - "2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:30:00" - ) - self.assertContains( - "2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:30:30" - ) - self.assertContains( - "2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:31:59" - ) - self.assertExcludes( - "2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:32:00" - ) - - def test_second_precision_intervals(self): - self.assertExcludes( - "2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:49" - ) - self.assertContains( - "2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:50" - ) - self.assertContains( - "2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:55" - ) - self.assertExcludes( - "2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:56" - ) - - def test_unbounded_endpoints(self): - self.assertContains("..", date=datetime.max) - self.assertContains("..", date=datetime.min) - self.assertContains("..", "1000-01-01T00:00:00") - - def assertContains(self, interval_pattern, date_pattern=None, date=None): - if date is None: - date = _date(date_pattern) - (start, end) = _parse_periods(interval_pattern) + @pytest.mark.parametrize( + "pattern, datestr, include", + [ + # year precision + ("2000..2001", "2000-01-01T00:00:00", True), + ("2000..2001", "2001-06-20T14:15:16", True), + ("2000..2001", "2001-12-31T23:59:59", True), + ("2000..2001", "1999-12-31T23:59:59", False), + ("2000..2001", "2002-01-01T00:00:00", False), + ("2000..", "2000-01-01T00:00:00", True), + ("2000..", "2099-10-11T00:00:00", True), + ("2000..", "1999-12-31T23:59:59", False), + ("..2001", "2001-12-31T23:59:59", True), + ("..2001", "2002-01-01T00:00:00", False), + ("-1d..1d", now, True), + ("-2d..-1d", now, False), + # month precision + ("2000-06-20..2000-06-20", "2000-06-20T00:00:00", True), + ("2000-06-20..2000-06-20", "2000-06-20T10:20:30", True), + ("2000-06-20..2000-06-20", "2000-06-20T23:59:59", True), + ("2000-06-20..2000-06-20", "2000-06-19T23:59:59", False), + ("2000-06-20..2000-06-20", "2000-06-21T00:00:00", False), + # day precision + ("1999-12..2000-02", "1999-12-01T00:00:00", True), + ("1999-12..2000-02", "2000-02-15T05:06:07", True), + ("1999-12..2000-02", "2000-02-29T23:59:59", True), + ("1999-12..2000-02", "1999-11-30T23:59:59", False), + ("1999-12..2000-02", "2000-03-01T00:00:00", False), + # hour precision with 'T' separator + ("2000-01-01T12..2000-01-01T13", "2000-01-01T11:59:59", False), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T12:00:00", True), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T12:30:00", True), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T13:30:00", True), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T13:59:59", True), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T14:00:00", False), + ("2000-01-01T12..2000-01-01T13", "2000-01-01T14:30:00", False), + # hour precision non-range query + ("2008-12-01T22", "2008-12-01T22:30:00", True), + ("2008-12-01T22", "2008-12-01T23:30:00", False), + # minute precision + ("2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:29:59", False), + ("2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:30:00", True), + ("2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:30:30", True), + ("2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:31:59", True), + ("2000-01-01T12:30..2000-01-01T12:31", "2000-01-01T12:32:00", False), + # second precision + ("2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:49", False), + ("2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:50", True), + ("2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:55", True), + ("2000-01-01T12:30:50..2000-01-01T12:30:55", "2000-01-01T12:30:56", False), # unbounded # noqa: E501 + ("..", datetime.max.isoformat(), True), + ("..", datetime.min.isoformat(), True), + ("..", "1000-01-01T00:00:00", True), + ], + ) # fmt: skip + def test_intervals(self, pattern, datestr, include): + (start, end) = _parse_periods(pattern) interval = DateInterval.from_periods(start, end) - assert interval.contains(date) - - def assertExcludes(self, interval_pattern, date_pattern): - date = _date(date_pattern) - (start, end) = _parse_periods(interval_pattern) - interval = DateInterval.from_periods(start, end) - assert not interval.contains(date) + assert interval.contains(datetime.fromisoformat(datestr)) == include def _parsetime(s): diff --git a/test/test_files.py b/test/test_files.py index 8be94f328..8b08a3fab 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -19,6 +19,7 @@ import shutil import stat import unittest from os.path import join +from pathlib import Path import pytest @@ -27,7 +28,7 @@ from beets import util from beets.test import _common from beets.test._common import item, touch from beets.test.helper import NEEDS_REFLINK, BeetsTestCase -from beets.util import MoveOperation, bytestring_path, syspath +from beets.util import MoveOperation, syspath class MoveTest(BeetsTestCase): @@ -35,11 +36,8 @@ class MoveTest(BeetsTestCase): super().setUp() # make a temporary file - self.path = join(self.temp_dir, b"temp.mp3") - shutil.copy( - syspath(join(_common.RSRC, b"full.mp3")), - syspath(self.path), - ) + self.path = self.temp_dir_path / "temp.mp3" + shutil.copy(self.resource_path, self.path) # add it to a temporary library self.i = beets.library.Item.from_path(self.path) @@ -52,57 +50,57 @@ class MoveTest(BeetsTestCase): self.i.artist = "one" self.i.album = "two" self.i.title = "three" - self.dest = join(self.libdir, b"one", b"two", b"three.mp3") + self.dest = self.lib_path / "one" / "two" / "three.mp3" - self.otherdir = join(self.temp_dir, b"testotherdir") + self.otherdir = self.temp_dir_path / "testotherdir" def test_move_arrives(self): self.i.move() - self.assertExists(self.dest) + assert self.dest.exists() def test_move_to_custom_dir(self): - self.i.move(basedir=self.otherdir) - self.assertExists(join(self.otherdir, b"one", b"two", b"three.mp3")) + self.i.move(basedir=os.fsencode(self.otherdir)) + assert (self.otherdir / "one" / "two" / "three.mp3").exists() def test_move_departs(self): self.i.move() - self.assertNotExists(self.path) + assert not self.path.exists() def test_move_in_lib_prunes_empty_dir(self): self.i.move() - old_path = self.i.path - self.assertExists(old_path) + old_path = self.i.filepath + assert old_path.exists() self.i.artist = "newArtist" self.i.move() - self.assertNotExists(old_path) - self.assertNotExists(os.path.dirname(old_path)) + assert not old_path.exists() + assert not old_path.parent.exists() def test_copy_arrives(self): self.i.move(operation=MoveOperation.COPY) - self.assertExists(self.dest) + assert self.dest.exists() def test_copy_does_not_depart(self): self.i.move(operation=MoveOperation.COPY) - self.assertExists(self.path) + assert self.path.exists() def test_reflink_arrives(self): self.i.move(operation=MoveOperation.REFLINK_AUTO) - self.assertExists(self.dest) + assert self.dest.exists() def test_reflink_does_not_depart(self): self.i.move(operation=MoveOperation.REFLINK_AUTO) - self.assertExists(self.path) + assert self.path.exists() @NEEDS_REFLINK def test_force_reflink_arrives(self): self.i.move(operation=MoveOperation.REFLINK) - self.assertExists(self.dest) + assert self.dest.exists() @NEEDS_REFLINK def test_force_reflink_does_not_depart(self): self.i.move(operation=MoveOperation.REFLINK) - self.assertExists(self.path) + assert self.path.exists() def test_move_changes_path(self): self.i.move() @@ -164,14 +162,14 @@ class MoveTest(BeetsTestCase): @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_arrives(self): self.i.move(operation=MoveOperation.LINK) - self.assertExists(self.dest) + assert self.dest.exists() assert os.path.islink(syspath(self.dest)) - assert bytestring_path(os.readlink(syspath(self.dest))) == self.path + assert self.dest.resolve() == self.path @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_does_not_depart(self): self.i.move(operation=MoveOperation.LINK) - self.assertExists(self.path) + assert self.path.exists() @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_changes_path(self): @@ -181,7 +179,7 @@ class MoveTest(BeetsTestCase): @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_hardlink_arrives(self): self.i.move(operation=MoveOperation.HARDLINK) - self.assertExists(self.dest) + assert self.dest.exists() s1 = os.stat(syspath(self.path)) s2 = os.stat(syspath(self.dest)) assert (s1[stat.ST_INO], s1[stat.ST_DEV]) == ( @@ -192,7 +190,7 @@ class MoveTest(BeetsTestCase): @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_hardlink_does_not_depart(self): self.i.move(operation=MoveOperation.HARDLINK) - self.assertExists(self.path) + assert self.path.exists() @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_hardlink_changes_path(self): @@ -264,24 +262,24 @@ class AlbumFileTest(BeetsTestCase): assert b"newAlbumName" in self.i.path def test_albuminfo_move_moves_file(self): - oldpath = self.i.path + oldpath = self.i.filepath self.ai.album = "newAlbumName" self.ai.move() self.ai.store() self.i.load() - self.assertNotExists(oldpath) - self.assertExists(self.i.path) + assert not oldpath.exists() + assert self.i.filepath.exists() def test_albuminfo_move_copies_file(self): - oldpath = self.i.path + oldpath = self.i.filepath self.ai.album = "newAlbumName" self.ai.move(operation=MoveOperation.COPY) self.ai.store() self.i.load() - self.assertExists(oldpath) - self.assertExists(self.i.path) + assert oldpath.exists() + assert self.i.filepath.exists() @NEEDS_REFLINK def test_albuminfo_move_reflinks_file(self): @@ -314,29 +312,30 @@ class ArtFileTest(BeetsTestCase): # Make an album. self.ai = self.lib.add_album((self.i,)) # Make an art file too. - self.art = self.lib.get_album(self.i).art_destination("something.jpg") - touch(self.art) - self.ai.artpath = self.art + art_bytes = self.lib.get_album(self.i).art_destination("something.jpg") + self.art = Path(os.fsdecode(art_bytes)) + self.art.touch() + self.ai.artpath = art_bytes self.ai.store() # Alternate destination dir. self.otherdir = os.path.join(self.temp_dir, b"testotherdir") def test_art_deleted_when_items_deleted(self): - self.assertExists(self.art) + assert self.art.exists() self.ai.remove(True) - self.assertNotExists(self.art) + assert not self.art.exists() def test_art_moves_with_album(self): - self.assertExists(self.art) + assert self.art.exists() oldpath = self.i.path self.ai.album = "newAlbum" self.ai.move() self.i.load() assert self.i.path != oldpath - self.assertNotExists(self.art) + assert not self.art.exists() newart = self.lib.get_album(self.i).art_destination(self.art) - self.assertExists(newart) + assert Path(os.fsdecode(newart)).exists() def test_art_moves_with_album_to_custom_dir(self): # Move the album to another directory. @@ -345,10 +344,10 @@ class ArtFileTest(BeetsTestCase): self.i.load() # Art should be in new directory. - self.assertNotExists(self.art) - newart = self.lib.get_album(self.i).artpath - self.assertExists(newart) - assert b"testotherdir" in newart + assert not self.art.exists() + newart = self.lib.get_album(self.i).art_filepath + assert newart.exists() + assert "testotherdir" in str(newart) def test_setart_copies_image(self): util.remove(self.art) @@ -363,7 +362,7 @@ class ArtFileTest(BeetsTestCase): assert ai.artpath is None ai.set_art(newart) - self.assertExists(ai.artpath) + assert ai.art_filepath.exists() def test_setart_to_existing_art_works(self): util.remove(self.art) @@ -380,7 +379,7 @@ class ArtFileTest(BeetsTestCase): # Set the art again. ai.set_art(ai.artpath) - self.assertExists(ai.artpath) + assert ai.art_filepath.exists() def test_setart_to_existing_but_unset_art_works(self): newart = os.path.join(self.libdir, b"newart.jpg") @@ -397,7 +396,7 @@ class ArtFileTest(BeetsTestCase): # Set the art again. ai.set_art(artdest) - self.assertExists(ai.artpath) + assert ai.art_filepath.exists() def test_setart_to_conflicting_file_gets_new_path(self): newart = os.path.join(self.libdir, b"newart.jpg") @@ -442,34 +441,34 @@ class ArtFileTest(BeetsTestCase): os.chmod(syspath(ai.artpath), 0o777) def test_move_last_file_moves_albumart(self): - oldartpath = self.lib.albums()[0].artpath - self.assertExists(oldartpath) + oldartpath = self.lib.albums()[0].art_filepath + assert oldartpath.exists() self.ai.album = "different_album" self.ai.store() self.ai.items()[0].move() - artpath = self.lib.albums()[0].artpath - assert b"different_album" in artpath - self.assertExists(artpath) - self.assertNotExists(oldartpath) + artpath = self.lib.albums()[0].art_filepath + assert "different_album" in str(artpath) + assert artpath.exists() + assert not oldartpath.exists() def test_move_not_last_file_does_not_move_albumart(self): i2 = item() i2.albumid = self.ai.id self.lib.add(i2) - oldartpath = self.lib.albums()[0].artpath - self.assertExists(oldartpath) + oldartpath = self.lib.albums()[0].art_filepath + assert oldartpath.exists() self.i.album = "different_album" self.i.album_id = None # detach from album self.i.move() - artpath = self.lib.albums()[0].artpath - assert b"different_album" not in artpath + artpath = self.lib.albums()[0].art_filepath + assert "different_album" not in str(artpath) assert artpath == oldartpath - self.assertExists(oldartpath) + assert oldartpath.exists() class RemoveTest(BeetsTestCase): @@ -486,37 +485,32 @@ class RemoveTest(BeetsTestCase): self.ai = self.lib.add_album((self.i,)) def test_removing_last_item_prunes_empty_dir(self): - parent = os.path.dirname(self.i.path) - self.assertExists(parent) + assert self.i.filepath.parent.exists() self.i.remove(True) - self.assertNotExists(parent) + assert not self.i.filepath.parent.exists() def test_removing_last_item_preserves_nonempty_dir(self): - parent = os.path.dirname(self.i.path) - touch(os.path.join(parent, b"dummy.txt")) + (self.i.filepath.parent / "dummy.txt").touch() self.i.remove(True) - self.assertExists(parent) + assert self.i.filepath.parent.exists() def test_removing_last_item_prunes_dir_with_blacklisted_file(self): - parent = os.path.dirname(self.i.path) - touch(os.path.join(parent, b".DS_Store")) + (self.i.filepath.parent / ".DS_Store").touch() self.i.remove(True) - self.assertNotExists(parent) + assert not self.i.filepath.parent.exists() def test_removing_without_delete_leaves_file(self): - path = self.i.path self.i.remove(False) - self.assertExists(path) + assert self.i.filepath.parent.exists() def test_removing_last_item_preserves_library_dir(self): self.i.remove(True) - self.assertExists(self.libdir) + assert self.lib_path.exists() def test_removing_item_outside_of_library_deletes_nothing(self): self.lib.directory = os.path.join(self.temp_dir, b"xxx") - parent = os.path.dirname(self.i.path) self.i.remove(True) - self.assertExists(parent) + assert self.i.filepath.parent.exists() def test_removing_last_item_in_album_with_albumart_prunes_dir(self): artfile = os.path.join(self.temp_dir, b"testart.jpg") @@ -524,55 +518,54 @@ class RemoveTest(BeetsTestCase): self.ai.set_art(artfile) self.ai.store() - parent = os.path.dirname(self.i.path) self.i.remove(True) - self.assertNotExists(parent) + assert not self.i.filepath.parent.exists() -# Tests that we can "delete" nonexistent files. -class SoftRemoveTest(BeetsTestCase): +class FilePathTestCase(BeetsTestCase): def setUp(self): super().setUp() - self.path = os.path.join(self.temp_dir, b"testfile") - touch(self.path) + self.path = self.temp_dir_path / "testfile" + self.path.touch() + +# Tests that we can "delete" nonexistent files. +class SoftRemoveTest(FilePathTestCase): def test_soft_remove_deletes_file(self): util.remove(self.path, True) - self.assertNotExists(self.path) + assert not self.path.exists() def test_soft_remove_silent_on_no_file(self): try: - util.remove(self.path + b"XXX", True) + util.remove(self.path / "XXX", True) except OSError: self.fail("OSError when removing path") -class SafeMoveCopyTest(BeetsTestCase): +class SafeMoveCopyTest(FilePathTestCase): def setUp(self): super().setUp() - self.path = os.path.join(self.temp_dir, b"testfile") - touch(self.path) - self.otherpath = os.path.join(self.temp_dir, b"testfile2") - touch(self.otherpath) - self.dest = self.path + b".dest" + self.otherpath = self.temp_dir_path / "testfile2" + self.otherpath.touch() + self.dest = Path(f"{self.path}.dest") def test_successful_move(self): util.move(self.path, self.dest) - self.assertExists(self.dest) - self.assertNotExists(self.path) + assert self.dest.exists() + assert not self.path.exists() def test_successful_copy(self): util.copy(self.path, self.dest) - self.assertExists(self.dest) - self.assertExists(self.path) + assert self.dest.exists() + assert self.path.exists() @NEEDS_REFLINK def test_successful_reflink(self): util.reflink(self.path, self.dest) - self.assertExists(self.dest) - self.assertExists(self.path) + assert self.dest.exists() + assert self.path.exists() def test_unsuccessful_move(self): with pytest.raises(util.FilesystemError): @@ -588,31 +581,31 @@ class SafeMoveCopyTest(BeetsTestCase): def test_self_move(self): util.move(self.path, self.path) - self.assertExists(self.path) + assert self.path.exists() def test_self_copy(self): util.copy(self.path, self.path) - self.assertExists(self.path) + assert self.path.exists() class PruneTest(BeetsTestCase): def setUp(self): super().setUp() - self.base = os.path.join(self.temp_dir, b"testdir") - os.mkdir(syspath(self.base)) - self.sub = os.path.join(self.base, b"subdir") - os.mkdir(syspath(self.sub)) + self.base = self.temp_dir_path / "testdir" + self.base.mkdir() + self.sub = self.base / "subdir" + self.sub.mkdir() def test_prune_existent_directory(self): util.prune_dirs(self.sub, self.base) - self.assertExists(self.base) - self.assertNotExists(self.sub) + assert self.base.exists() + assert not self.sub.exists() def test_prune_nonexistent_directory(self): - util.prune_dirs(os.path.join(self.sub, b"another"), self.base) - self.assertExists(self.base) - self.assertNotExists(self.sub) + util.prune_dirs(self.sub / "another", self.base) + assert self.base.exists() + assert not self.sub.exists() class WalkTest(BeetsTestCase): @@ -678,12 +671,9 @@ class UniquePathTest(BeetsTestCase): class MkDirAllTest(BeetsTestCase): - def test_parent_exists(self): - path = os.path.join(self.temp_dir, b"foo", b"bar", b"baz", b"qux.mp3") - util.mkdirall(path) - self.assertIsDir(os.path.join(self.temp_dir, b"foo", b"bar", b"baz")) - - def test_child_does_not_exist(self): - path = os.path.join(self.temp_dir, b"foo", b"bar", b"baz", b"qux.mp3") - util.mkdirall(path) - self.assertNotExists(path) + def test_mkdirall(self): + child = self.temp_dir_path / "foo" / "bar" / "baz" / "quz.mp3" + util.mkdirall(child) + assert not child.exists() + assert child.parent.exists() + assert child.parent.is_dir() diff --git a/test/test_importer.py b/test/test_importer.py index 9bb0e8a63..9ec160568 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -15,6 +15,8 @@ """Tests for the general importer functionality.""" +from __future__ import annotations + import os import re import shutil @@ -22,6 +24,7 @@ import stat import sys import unicodedata import unittest +from functools import cached_property from io import StringIO from pathlib import Path from tarfile import TarFile @@ -43,6 +46,7 @@ from beets.test.helper import ( AutotagStub, BeetsTestCase, ImportTestCase, + IOMixin, PluginMixin, capture_log, has_program, @@ -50,84 +54,71 @@ from beets.test.helper import ( from beets.util import bytestring_path, displayable_path, syspath +class PathsMixin: + import_media: list[MediaFile] + + @cached_property + def track_import_path(self) -> Path: + return Path(self.import_media[0].path) + + @cached_property + def album_path(self) -> Path: + return self.track_import_path.parent + + @cached_property + def track_lib_path(self): + return self.lib_path / "Tag Artist" / "Tag Album" / "Tag Track 1.mp3" + + @_common.slow_test() -class NonAutotaggedImportTest(AsIsImporterMixin, ImportTestCase): +class NonAutotaggedImportTest(PathsMixin, AsIsImporterMixin, ImportTestCase): db_on_disk = True def test_album_created_with_track_artist(self): self.run_asis_importer() + albums = self.lib.albums() assert len(albums) == 1 assert albums[0].albumartist == "Tag Artist" def test_import_copy_arrives(self): self.run_asis_importer() - for mediafile in self.import_media: - self.assert_file_in_lib( - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) + + assert self.track_lib_path.exists() def test_threaded_import_copy_arrives(self): config["threaded"] = True self.run_asis_importer() - for mediafile in self.import_media: - self.assert_file_in_lib( - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) + assert self.track_lib_path.exists() def test_import_with_move_deletes_import_files(self): - for mediafile in self.import_media: - self.assertExists(mediafile.path) - self.run_asis_importer(move=True) - for mediafile in self.import_media: - self.assertNotExists(mediafile.path) - - def test_import_with_move_prunes_directory_empty(self): - self.assertExists(os.path.join(self.import_dir, b"album")) - self.run_asis_importer(move=True) - self.assertNotExists(os.path.join(self.import_dir, b"album")) - - def test_import_with_move_prunes_with_extra_clutter(self): - self.touch(os.path.join(self.import_dir, b"album", b"alog.log")) + assert self.album_path.exists() + assert self.track_import_path.exists() + (self.album_path / "alog.log").touch() config["clutter"] = ["*.log"] - self.assertExists(os.path.join(self.import_dir, b"album")) self.run_asis_importer(move=True) - self.assertNotExists(os.path.join(self.import_dir, b"album")) + + assert not self.track_import_path.exists() + assert not self.album_path.exists() def test_threaded_import_move_arrives(self): self.run_asis_importer(move=True, threaded=True) - for mediafile in self.import_media: - self.assert_file_in_lib( - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) - def test_threaded_import_move_deletes_import(self): - self.run_asis_importer(move=True, threaded=True) - for mediafile in self.import_media: - self.assertNotExists(mediafile.path) + assert self.track_lib_path.exists() + assert not self.track_import_path.exists() def test_import_without_delete_retains_files(self): self.run_asis_importer(delete=False) - for mediafile in self.import_media: - self.assertExists(mediafile.path) + + assert self.track_import_path.exists() def test_import_with_delete_removes_files(self): self.run_asis_importer(delete=True) - for mediafile in self.import_media: - self.assertNotExists(mediafile.path) - def test_import_with_delete_prunes_directory_empty(self): - self.assertExists(os.path.join(self.import_dir, b"album")) - self.run_asis_importer(delete=True) - self.assertNotExists(os.path.join(self.import_dir, b"album")) + assert not self.album_path.exists() + assert not self.track_import_path.exists() def test_album_mb_albumartistids(self): self.run_asis_importer() @@ -137,63 +128,38 @@ class NonAutotaggedImportTest(AsIsImporterMixin, ImportTestCase): @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_import_link_arrives(self): self.run_asis_importer(link=True) - for mediafile in self.import_media: - filename = os.path.join( - self.libdir, - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) - self.assertExists(filename) - assert os.path.islink(syspath(filename)) - self.assert_equal_path( - util.bytestring_path(os.readlink(syspath(filename))), - mediafile.path, - ) + + assert self.track_lib_path.exists() + assert self.track_lib_path.is_symlink() + assert self.track_lib_path.resolve() == self.track_import_path @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_import_hardlink_arrives(self): self.run_asis_importer(hardlink=True) - for mediafile in self.import_media: - filename = os.path.join( - self.libdir, - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) - self.assertExists(filename) - s1 = os.stat(syspath(mediafile.path)) - s2 = os.stat(syspath(filename)) - assert (s1[stat.ST_INO], s1[stat.ST_DEV]) == ( - s2[stat.ST_INO], - s2[stat.ST_DEV], - ) + + assert self.track_lib_path.exists() + media_stat = self.track_import_path.stat() + lib_media_stat = self.track_lib_path.stat() + assert media_stat[stat.ST_INO] == lib_media_stat[stat.ST_INO] + assert media_stat[stat.ST_DEV] == lib_media_stat[stat.ST_DEV] @NEEDS_REFLINK def test_import_reflink_arrives(self): # Detecting reflinks is currently tricky due to various fs # implementations, we'll just check the file exists. self.run_asis_importer(reflink=True) - for mediafile in self.import_media: - self.assert_file_in_lib( - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) + + assert self.track_lib_path.exists() def test_import_reflink_auto_arrives(self): # Should pass regardless of reflink support due to fallback. self.run_asis_importer(reflink="auto") - for mediafile in self.import_media: - self.assert_file_in_lib( - b"Tag Artist", - b"Tag Album", - util.bytestring_path(f"{mediafile.title}.mp3"), - ) + + assert self.track_lib_path.exists() def create_archive(session): - (handle, path) = mkstemp(dir=os.fsdecode(session.temp_dir)) + handle, path = mkstemp(dir=session.temp_dir_path) path = bytestring_path(path) os.close(handle) archive = ZipFile(os.fsdecode(path), mode="w") @@ -218,10 +184,10 @@ class RmTempTest(BeetsTestCase): zip_path = create_archive(self) archive_task = importer.ArchiveImportTask(zip_path) archive_task.extract() - tmp_path = archive_task.toppath - self.assertExists(tmp_path) + tmp_path = Path(os.fsdecode(archive_task.toppath)) + assert tmp_path.exists() archive_task.finalize(self) - self.assertNotExists(tmp_path) + assert not tmp_path.exists() class ImportZipTest(AsIsImporterMixin, ImportTestCase): @@ -275,56 +241,36 @@ class ImportSingletonTest(AutotagImportTestCase): self.prepare_album_for_import(1) self.importer = self.setup_singleton_importer() - def test_apply_asis_adds_track(self): - assert self.lib.items().get() is None - + def test_apply_asis_adds_only_singleton_track(self): self.importer.add_choice(importer.Action.ASIS) self.importer.run() + + # album not added + assert not self.lib.albums() assert self.lib.items().get().title == "Tag Track 1" - - def test_apply_asis_does_not_add_album(self): - assert self.lib.albums().get() is None - - self.importer.add_choice(importer.Action.ASIS) - self.importer.run() - assert self.lib.albums().get() is None - - def test_apply_asis_adds_singleton_path(self): - self.assert_lib_dir_empty() - - self.importer.add_choice(importer.Action.ASIS) - self.importer.run() - self.assert_file_in_lib(b"singletons", b"Tag Track 1.mp3") + assert (self.lib_path / "singletons" / "Tag Track 1.mp3").exists() def test_apply_candidate_adds_track(self): - assert self.lib.items().get() is None - self.importer.add_choice(importer.Action.APPLY) self.importer.run() + + assert not self.lib.albums() assert self.lib.items().get().title == "Applied Track 1" + assert (self.lib_path / "singletons" / "Applied Track 1.mp3").exists() - def test_apply_candidate_does_not_add_album(self): - self.importer.add_choice(importer.Action.APPLY) - self.importer.run() - assert self.lib.albums().get() is None - - def test_apply_candidate_adds_singleton_path(self): - self.assert_lib_dir_empty() - - self.importer.add_choice(importer.Action.APPLY) - self.importer.run() - self.assert_file_in_lib(b"singletons", b"Applied Track 1.mp3") - - def test_skip_does_not_add_first_track(self): + def test_skip_does_not_add_track(self): self.importer.add_choice(importer.Action.SKIP) self.importer.run() - assert self.lib.items().get() is None - def test_skip_adds_other_tracks(self): + assert not self.lib.items() + + def test_skip_first_add_second_asis(self): self.prepare_album_for_import(2) + self.importer.add_choice(importer.Action.SKIP) self.importer.add_choice(importer.Action.ASIS) self.importer.run() + assert len(self.lib.items()) == 1 def test_import_single_files(self): @@ -373,7 +319,7 @@ class ImportSingletonTest(AutotagImportTestCase): item.remove() # Autotagged. - assert self.lib.albums().get() is None + assert not self.lib.albums() self.importer.clear_choices() self.importer.add_choice(importer.Action.APPLY) self.importer.run() @@ -386,7 +332,7 @@ class ImportSingletonTest(AutotagImportTestCase): assert item.disc == disc -class ImportTest(AutotagImportTestCase): +class ImportTest(PathsMixin, AutotagImportTestCase): """Test APPLY, ASIS and SKIP choices.""" def setUp(self): @@ -394,48 +340,23 @@ class ImportTest(AutotagImportTestCase): self.prepare_album_for_import(1) self.setup_importer() - def test_apply_asis_adds_album(self): - assert self.lib.albums().get() is None - + def test_asis_moves_album_and_track(self): self.importer.add_choice(importer.Action.ASIS) self.importer.run() + assert self.lib.albums().get().album == "Tag Album" + item = self.lib.items().get() + assert item.title == "Tag Track 1" + assert item.filepath.exists() - def test_apply_asis_adds_tracks(self): - assert self.lib.items().get() is None - self.importer.add_choice(importer.Action.ASIS) - self.importer.run() - assert self.lib.items().get().title == "Tag Track 1" - - def test_apply_asis_adds_album_path(self): - self.assert_lib_dir_empty() - - self.importer.add_choice(importer.Action.ASIS) - self.importer.run() - self.assert_file_in_lib(b"Tag Artist", b"Tag Album", b"Tag Track 1.mp3") - - def test_apply_candidate_adds_album(self): - assert self.lib.albums().get() is None - + def test_apply_moves_album_and_track(self): self.importer.add_choice(importer.Action.APPLY) self.importer.run() + assert self.lib.albums().get().album == "Applied Album" - - def test_apply_candidate_adds_tracks(self): - assert self.lib.items().get() is None - - self.importer.add_choice(importer.Action.APPLY) - self.importer.run() - assert self.lib.items().get().title == "Applied Track 1" - - def test_apply_candidate_adds_album_path(self): - self.assert_lib_dir_empty() - - self.importer.add_choice(importer.Action.APPLY) - self.importer.run() - self.assert_file_in_lib( - b"Applied Artist", b"Applied Album", b"Applied Track 1.mp3" - ) + item = self.lib.items().get() + assert item.title == "Applied Track 1" + assert item.filepath.exists() def test_apply_from_scratch_removes_other_metadata(self): config["import"]["from_scratch"] = True @@ -464,35 +385,35 @@ class ImportTest(AutotagImportTestCase): assert self.lib.items().get().bitrate == bitrate def test_apply_with_move_deletes_import(self): + assert self.track_import_path.exists() + config["import"]["move"] = True - - import_file = os.path.join(self.import_dir, b"album", b"track_1.mp3") - self.assertExists(import_file) - self.importer.add_choice(importer.Action.APPLY) self.importer.run() - self.assertNotExists(import_file) + + assert not self.track_import_path.exists() def test_apply_with_delete_deletes_import(self): + assert self.track_import_path.exists() + config["import"]["delete"] = True - - import_file = os.path.join(self.import_dir, b"album", b"track_1.mp3") - self.assertExists(import_file) - self.importer.add_choice(importer.Action.APPLY) self.importer.run() - self.assertNotExists(import_file) + + assert not self.track_import_path.exists() def test_skip_does_not_add_track(self): self.importer.add_choice(importer.Action.SKIP) self.importer.run() - assert self.lib.items().get() is None + + assert not self.lib.items() def test_skip_non_album_dirs(self): - self.assertIsDir(os.path.join(self.import_dir, b"album")) + assert (self.import_path / "album").exists() self.touch(b"cruft", dir=self.import_dir) self.importer.add_choice(importer.Action.APPLY) self.importer.run() + assert len(self.lib.albums()) == 1 def test_unmatched_tracks_not_added(self): @@ -596,24 +517,21 @@ class ImportTracksTest(AutotagImportTestCase): self.setup_importer() def test_apply_tracks_adds_singleton_track(self): - assert self.lib.items().get() is None - assert self.lib.albums().get() is None - self.importer.add_choice(importer.Action.TRACKS) self.importer.add_choice(importer.Action.APPLY) self.importer.add_choice(importer.Action.APPLY) self.importer.run() + assert self.lib.items().get().title == "Applied Track 1" - assert self.lib.albums().get() is None + assert not self.lib.albums() def test_apply_tracks_adds_singleton_path(self): - self.assert_lib_dir_empty() - self.importer.add_choice(importer.Action.TRACKS) self.importer.add_choice(importer.Action.APPLY) self.importer.add_choice(importer.Action.APPLY) self.importer.run() - self.assert_file_in_lib(b"singletons", b"Applied Track 1.mp3") + + assert (self.lib_path / "singletons" / "Applied Track 1.mp3").exists() class ImportCompilationTest(AutotagImportTestCase): @@ -721,7 +639,7 @@ class ImportCompilationTest(AutotagImportTestCase): assert asserted_multi_artists_1 -class ImportExistingTest(AutotagImportTestCase): +class ImportExistingTest(PathsMixin, AutotagImportTestCase): """Test importing files that are already in the library directory.""" def setUp(self): @@ -731,20 +649,23 @@ class ImportExistingTest(AutotagImportTestCase): self.reimporter = self.setup_importer(import_dir=self.libdir) self.importer = self.setup_importer() - def test_does_not_duplicate_item(self): + def tearDown(self): + super().tearDown() + self.matcher.restore() + + @cached_property + def applied_track_path(self) -> Path: + return Path(str(self.track_lib_path).replace("Tag", "Applied")) + + def test_does_not_duplicate_item_nor_album(self): self.importer.run() assert len(self.lib.items()) == 1 - - self.reimporter.add_choice(importer.Action.APPLY) - self.reimporter.run() - assert len(self.lib.items()) == 1 - - def test_does_not_duplicate_album(self): - self.importer.run() assert len(self.lib.albums()) == 1 self.reimporter.add_choice(importer.Action.APPLY) self.reimporter.run() + + assert len(self.lib.items()) == 1 assert len(self.lib.albums()) == 1 def test_does_not_duplicate_singleton_track(self): @@ -758,33 +679,19 @@ class ImportExistingTest(AutotagImportTestCase): self.reimporter.run() assert len(self.lib.items()) == 1 - def test_asis_updates_metadata(self): + def test_asis_updates_metadata_and_moves_file(self): self.importer.run() + medium = MediaFile(self.lib.items().get().path) medium.title = "New Title" medium.save() self.reimporter.add_choice(importer.Action.ASIS) self.reimporter.run() + assert self.lib.items().get().title == "New Title" - - def test_asis_updated_moves_file(self): - self.importer.run() - medium = MediaFile(self.lib.items().get().path) - medium.title = "New Title" - medium.save() - - old_path = os.path.join( - b"Applied Artist", b"Applied Album", b"Applied Track 1.mp3" - ) - self.assert_file_in_lib(old_path) - - self.reimporter.add_choice(importer.Action.ASIS) - self.reimporter.run() - self.assert_file_in_lib( - b"Applied Artist", b"Applied Album", b"New Title.mp3" - ) - self.assert_file_not_in_lib(old_path) + assert not self.applied_track_path.exists() + assert self.applied_track_path.with_name("New Title.mp3").exists() def test_asis_updated_without_copy_does_not_move_file(self): self.importer.run() @@ -792,49 +699,24 @@ class ImportExistingTest(AutotagImportTestCase): medium.title = "New Title" medium.save() - old_path = os.path.join( - b"Applied Artist", b"Applied Album", b"Applied Track 1.mp3" - ) - self.assert_file_in_lib(old_path) - config["import"]["copy"] = False self.reimporter.add_choice(importer.Action.ASIS) self.reimporter.run() - self.assert_file_not_in_lib( - b"Applied Artist", b"Applied Album", b"New Title.mp3" - ) - self.assert_file_in_lib(old_path) + + assert self.applied_track_path.exists() + assert not self.applied_track_path.with_name("New Title.mp3").exists() def test_outside_file_is_copied(self): config["import"]["copy"] = False self.importer.run() - self.assert_equal_path( - self.lib.items().get().path, self.import_media[0].path - ) + assert self.lib.items().get().filepath == self.track_import_path self.reimporter = self.setup_importer() self.reimporter.add_choice(importer.Action.APPLY) self.reimporter.run() - new_path = os.path.join( - b"Applied Artist", b"Applied Album", b"Applied Track 1.mp3" - ) - self.assert_file_in_lib(new_path) - self.assert_equal_path( - self.lib.items().get().path, os.path.join(self.libdir, new_path) - ) - - def test_outside_file_is_moved(self): - config["import"]["copy"] = False - self.importer.run() - self.assert_equal_path( - self.lib.items().get().path, self.import_media[0].path - ) - - self.reimporter = self.setup_importer(move=True) - self.reimporter.add_choice(importer.Action.APPLY) - self.reimporter.run() - self.assertNotExists(self.import_media[0].path) + assert self.applied_track_path.exists() + assert self.lib.items().get().filepath == self.applied_track_path class GroupAlbumsImportTest(AutotagImportTestCase): @@ -1050,12 +932,12 @@ class ImportDuplicateAlbumTest(PluginMixin, ImportTestCase): def test_remove_duplicate_album(self): item = self.lib.items().get() assert item.title == "t\xeftle 0" - self.assertExists(item.path) + assert item.filepath.exists() self.importer.default_resolution = self.importer.Resolution.REMOVE self.importer.run() - self.assertNotExists(item.path) + assert not item.filepath.exists() assert len(self.lib.albums()) == 1 assert len(self.lib.items()) == 1 item = self.lib.items().get() @@ -1065,7 +947,7 @@ class ImportDuplicateAlbumTest(PluginMixin, ImportTestCase): config["import"]["autotag"] = False item = self.lib.items().get() assert item.title == "t\xeftle 0" - self.assertExists(item.path) + assert item.filepath.exists() # Imported item has the same artist and album as the one in the # library. @@ -1081,7 +963,7 @@ class ImportDuplicateAlbumTest(PluginMixin, ImportTestCase): self.importer.default_resolution = self.importer.Resolution.REMOVE self.importer.run() - self.assertExists(item.path) + assert item.filepath.exists() assert len(self.lib.albums()) == 2 assert len(self.lib.items()) == 2 @@ -1168,12 +1050,12 @@ class ImportDuplicateSingletonTest(ImportTestCase): def test_remove_duplicate(self): item = self.lib.items().get() assert item.mb_trackid == "old trackid" - self.assertExists(item.path) + assert item.filepath.exists() self.importer.default_resolution = self.importer.Resolution.REMOVE self.importer.run() - self.assertNotExists(item.path) + assert not item.filepath.exists() assert len(self.lib.items()) == 1 item = self.lib.items().get() assert item.mb_trackid == "new trackid" @@ -1566,14 +1448,14 @@ class ReimportTest(AutotagImportTestCase): replaced_album = self._album() replaced_album.set_art(art_source) replaced_album.store() - old_artpath = replaced_album.artpath + old_artpath = replaced_album.art_filepath self.importer.run() new_album = self._album() new_artpath = new_album.art_destination(art_source) assert new_album.artpath == new_artpath - self.assertExists(new_artpath) + assert new_album.art_filepath.exists() if new_artpath != old_artpath: - self.assertNotExists(old_artpath) + assert not old_artpath.exists() def test_reimported_album_has_new_flexattr(self): self._setup_session() @@ -1588,13 +1470,11 @@ class ReimportTest(AutotagImportTestCase): assert self._album().data_source == "match_source" -class ImportPretendTest(AutotagImportTestCase): +class ImportPretendTest(IOMixin, AutotagImportTestCase): """Test the pretend commandline option""" def setUp(self): super().setUp() - self.io.install() - self.album_track_path = self.prepare_album_for_import(1)[0] self.single_path = self.prepare_track_for_import(2, self.import_path) self.album_path = self.album_track_path.parent @@ -1624,7 +1504,7 @@ class ImportPretendTest(AutotagImportTestCase): ] def test_import_pretend_empty(self): - empty_path = Path(os.fsdecode(self.temp_dir)) / "empty" + empty_path = self.temp_dir_path / "empty" empty_path.mkdir() importer = self.setup_importer(pretend=True, import_dir=empty_path) diff --git a/test/test_library.py b/test/test_library.py index 2d232c88f..35791bad7 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -194,7 +194,7 @@ class DestinationTest(BeetsTestCase): def create_temp_dir(self, **kwargs): kwargs["prefix"] = "." - super().create_temp_dir(**kwargs) + return super().create_temp_dir(**kwargs) def setUp(self): super().setUp() diff --git a/test/test_ui.py b/test/test_ui.py index 8bb0218d5..713e69891 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -21,6 +21,7 @@ import shutil import subprocess import sys import unittest +from pathlib import Path from unittest.mock import Mock, patch import pytest @@ -32,6 +33,7 @@ from beets.autotag.match import distance from beets.test import _common from beets.test.helper import ( BeetsTestCase, + IOMixin, PluginTestCase, capture_stdout, control_stdin, @@ -107,15 +109,12 @@ class ListTest(BeetsTestCase): assert "the album" not in stdout.getvalue() -class RemoveTest(BeetsTestCase): +class RemoveTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() - self.io.install() - # Copy a file into the library. - self.item_path = os.path.join(_common.RSRC, b"full.mp3") - self.i = library.Item.from_path(self.item_path) + self.i = library.Item.from_path(self.resource_path) self.lib.add(self.i) self.i.move(operation=MoveOperation.COPY) @@ -124,29 +123,29 @@ class RemoveTest(BeetsTestCase): commands.remove_items(self.lib, "", False, False, False) items = self.lib.items() assert len(list(items)) == 0 - self.assertExists(self.i.path) + assert self.i.filepath.exists() def test_remove_items_with_delete(self): self.io.addinput("y") commands.remove_items(self.lib, "", False, True, False) items = self.lib.items() assert len(list(items)) == 0 - self.assertNotExists(self.i.path) + assert not self.i.filepath.exists() def test_remove_items_with_force_no_delete(self): commands.remove_items(self.lib, "", False, False, True) items = self.lib.items() assert len(list(items)) == 0 - self.assertExists(self.i.path) + assert self.i.filepath.exists() def test_remove_items_with_force_delete(self): commands.remove_items(self.lib, "", False, True, True) items = self.lib.items() assert len(list(items)) == 0 - self.assertNotExists(self.i.path) + assert not self.i.filepath.exists() def test_remove_items_select_with_delete(self): - i2 = library.Item.from_path(self.item_path) + i2 = library.Item.from_path(self.resource_path) self.lib.add(i2) i2.move(operation=MoveOperation.COPY) @@ -444,21 +443,16 @@ class MoveTest(BeetsTestCase): def setUp(self): super().setUp() - self.io.install() - - self.itempath = os.path.join(self.libdir, b"srcfile") - shutil.copy( - syspath(os.path.join(_common.RSRC, b"full.mp3")), - syspath(self.itempath), - ) + self.initial_item_path = self.lib_path / "srcfile" + shutil.copy(self.resource_path, self.initial_item_path) # Add a file to the library but don't copy it in yet. - self.i = library.Item.from_path(self.itempath) + self.i = library.Item.from_path(self.initial_item_path) self.lib.add(self.i) self.album = self.lib.add_album([self.i]) # Alternate destination directory. - self.otherdir = os.path.join(self.temp_dir, b"testotherdir") + self.otherdir = self.temp_dir_path / "testotherdir" def _move( self, @@ -477,79 +471,77 @@ class MoveTest(BeetsTestCase): self._move() self.i.load() assert b"libdir" in self.i.path - self.assertExists(self.i.path) - self.assertNotExists(self.itempath) + assert self.i.filepath.exists() + assert not self.initial_item_path.exists() def test_copy_item(self): self._move(copy=True) self.i.load() assert b"libdir" in self.i.path - self.assertExists(self.i.path) - self.assertExists(self.itempath) + assert self.i.filepath.exists() + assert self.initial_item_path.exists() def test_move_album(self): self._move(album=True) self.i.load() assert b"libdir" in self.i.path - self.assertExists(self.i.path) - self.assertNotExists(self.itempath) + assert self.i.filepath.exists() + assert not self.initial_item_path.exists() def test_copy_album(self): self._move(copy=True, album=True) self.i.load() assert b"libdir" in self.i.path - self.assertExists(self.i.path) - self.assertExists(self.itempath) + assert self.i.filepath.exists() + assert self.initial_item_path.exists() def test_move_item_custom_dir(self): self._move(dest=self.otherdir) self.i.load() assert b"testotherdir" in self.i.path - self.assertExists(self.i.path) - self.assertNotExists(self.itempath) + assert self.i.filepath.exists() + assert not self.initial_item_path.exists() def test_move_album_custom_dir(self): self._move(dest=self.otherdir, album=True) self.i.load() assert b"testotherdir" in self.i.path - self.assertExists(self.i.path) - self.assertNotExists(self.itempath) + assert self.i.filepath.exists() + assert not self.initial_item_path.exists() def test_pretend_move_item(self): self._move(dest=self.otherdir, pretend=True) self.i.load() - assert b"srcfile" in self.i.path + assert self.i.filepath == self.initial_item_path def test_pretend_move_album(self): self._move(album=True, pretend=True) self.i.load() - assert b"srcfile" in self.i.path + assert self.i.filepath == self.initial_item_path def test_export_item_custom_dir(self): self._move(dest=self.otherdir, export=True) self.i.load() - assert self.i.path == self.itempath - self.assertExists(self.otherdir) + assert self.i.filepath == self.initial_item_path + assert self.otherdir.exists() def test_export_album_custom_dir(self): self._move(dest=self.otherdir, album=True, export=True) self.i.load() - assert self.i.path == self.itempath - self.assertExists(self.otherdir) + assert self.i.filepath == self.initial_item_path + assert self.otherdir.exists() def test_pretend_export_item(self): self._move(dest=self.otherdir, pretend=True, export=True) self.i.load() - assert b"srcfile" in self.i.path - self.assertNotExists(self.otherdir) + assert self.i.filepath == self.initial_item_path + assert not self.otherdir.exists() -class UpdateTest(BeetsTestCase): +class UpdateTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() - self.io.install() - # Copy a file into the library. item_path = os.path.join(_common.RSRC, b"full.mp3") item_path_two = os.path.join(_common.RSRC, b"full.flac") @@ -606,12 +598,12 @@ class UpdateTest(BeetsTestCase): assert not self.lib.albums() def test_delete_removes_album_art(self): - artpath = self.album.artpath - self.assertExists(artpath) + art_filepath = self.album.art_filepath + assert art_filepath.exists() util.remove(self.i.path) util.remove(self.i2.path) self._update() - self.assertNotExists(artpath) + assert not art_filepath.exists() def test_modified_metadata_detected(self): mf = MediaFile(syspath(self.i.path)) @@ -742,11 +734,7 @@ class UpdateTest(BeetsTestCase): assert item.lyrics != "new lyrics" -class PrintTest(BeetsTestCase): - def setUp(self): - super().setUp() - self.io.install() - +class PrintTest(IOMixin, unittest.TestCase): def test_print_without_locale(self): lang = os.environ.get("LANG") if lang: @@ -841,9 +829,7 @@ class ConfigTest(TestPluginTestCase): del os.environ["BEETSDIR"] # Also set APPDATA, the Windows equivalent of setting $HOME. - appdata_dir = os.fsdecode( - os.path.join(self.temp_dir, b"AppData", b"Roaming") - ) + appdata_dir = self.temp_dir_path / "AppData" / "Roaming" self._orig_cwd = os.getcwd() self.test_cmd = self._make_test_cmd() @@ -851,27 +837,21 @@ class ConfigTest(TestPluginTestCase): # Default user configuration if platform.system() == "Windows": - self.user_config_dir = os.fsencode( - os.path.join(appdata_dir, "beets") - ) + self.user_config_dir = appdata_dir / "beets" else: - self.user_config_dir = os.path.join( - self.temp_dir, b".config", b"beets" - ) - os.makedirs(syspath(self.user_config_dir)) - self.user_config_path = os.path.join( - self.user_config_dir, b"config.yaml" - ) + self.user_config_dir = self.temp_dir_path / ".config" / "beets" + self.user_config_dir.mkdir(parents=True, exist_ok=True) + self.user_config_path = self.user_config_dir / "config.yaml" # Custom BEETSDIR - self.beetsdir = os.path.join(self.temp_dir, b"beetsdir") - self.cli_config_path = os.path.join( - os.fsdecode(self.temp_dir), "config.yaml" - ) - os.makedirs(syspath(self.beetsdir)) + self.beetsdir = self.temp_dir_path / "beetsdir" + self.beetsdir.mkdir(parents=True, exist_ok=True) + + self.env_config_path = str(self.beetsdir / "config.yaml") + self.cli_config_path = str(self.temp_dir_path / "config.yaml") self.env_patcher = patch( "os.environ", - {"HOME": os.fsdecode(self.temp_dir), "APPDATA": appdata_dir}, + {"HOME": str(self.temp_dir_path), "APPDATA": str(appdata_dir)}, ) self.env_patcher.start() @@ -970,9 +950,8 @@ class ConfigTest(TestPluginTestCase): assert config["anoption"].get() == "cli overwrite" def test_cli_config_file_overwrites_beetsdir_defaults(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) - env_config_path = os.path.join(self.beetsdir, b"config.yaml") - with open(env_config_path, "w") as file: + os.environ["BEETSDIR"] = str(self.beetsdir) + with open(self.env_config_path, "w") as file: file.write("anoption: value") with open(self.cli_config_path, "w") as file: @@ -1019,39 +998,25 @@ class ConfigTest(TestPluginTestCase): file.write("statefile: state") self.run_command("--config", self.cli_config_path, "test", lib=None) - self.assert_equal_path( - util.bytestring_path(config["library"].as_filename()), - os.path.join(self.user_config_dir, b"beets.db"), - ) - self.assert_equal_path( - util.bytestring_path(config["statefile"].as_filename()), - os.path.join(self.user_config_dir, b"state"), - ) + assert config["library"].as_path() == self.user_config_dir / "beets.db" + assert config["statefile"].as_path() == self.user_config_dir / "state" def test_cli_config_paths_resolve_relative_to_beetsdir(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) + os.environ["BEETSDIR"] = str(self.beetsdir) with open(self.cli_config_path, "w") as file: file.write("library: beets.db\n") file.write("statefile: state") self.run_command("--config", self.cli_config_path, "test", lib=None) - self.assert_equal_path( - util.bytestring_path(config["library"].as_filename()), - os.path.join(self.beetsdir, b"beets.db"), - ) - self.assert_equal_path( - util.bytestring_path(config["statefile"].as_filename()), - os.path.join(self.beetsdir, b"state"), - ) + assert config["library"].as_path() == self.beetsdir / "beets.db" + assert config["statefile"].as_path() == self.beetsdir / "state" def test_command_line_option_relative_to_working_dir(self): config.read() os.chdir(syspath(self.temp_dir)) self.run_command("--library", "foo.db", "test", lib=None) - self.assert_equal_path( - config["library"].as_filename(), os.path.join(os.getcwd(), "foo.db") - ) + assert config["library"].as_path() == Path.cwd() / "foo.db" def test_cli_config_file_loads_plugin_commands(self): with open(self.cli_config_path, "w") as file: @@ -1063,24 +1028,23 @@ class ConfigTest(TestPluginTestCase): self.unload_plugins() def test_beetsdir_config(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) + os.environ["BEETSDIR"] = str(self.beetsdir) - env_config_path = os.path.join(self.beetsdir, b"config.yaml") - with open(env_config_path, "w") as file: + with open(self.env_config_path, "w") as file: file.write("anoption: overwrite") config.read() assert config["anoption"].get() == "overwrite" def test_beetsdir_points_to_file_error(self): - beetsdir = os.path.join(self.temp_dir, b"beetsfile") + beetsdir = str(self.temp_dir_path / "beetsfile") open(beetsdir, "a").close() - os.environ["BEETSDIR"] = os.fsdecode(beetsdir) + os.environ["BEETSDIR"] = beetsdir with pytest.raises(ConfigError): self.run_command("test") def test_beetsdir_config_does_not_load_default_user_config(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) + os.environ["BEETSDIR"] = str(self.beetsdir) with open(self.user_config_path, "w") as file: file.write("anoption: value") @@ -1089,41 +1053,27 @@ class ConfigTest(TestPluginTestCase): assert not config["anoption"].exists() def test_default_config_paths_resolve_relative_to_beetsdir(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) + os.environ["BEETSDIR"] = str(self.beetsdir) config.read() - self.assert_equal_path( - util.bytestring_path(config["library"].as_filename()), - os.path.join(self.beetsdir, b"library.db"), - ) - self.assert_equal_path( - util.bytestring_path(config["statefile"].as_filename()), - os.path.join(self.beetsdir, b"state.pickle"), - ) + assert config["library"].as_path() == self.beetsdir / "library.db" + assert config["statefile"].as_path() == self.beetsdir / "state.pickle" def test_beetsdir_config_paths_resolve_relative_to_beetsdir(self): - os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) + os.environ["BEETSDIR"] = str(self.beetsdir) - env_config_path = os.path.join(self.beetsdir, b"config.yaml") - with open(env_config_path, "w") as file: + with open(self.env_config_path, "w") as file: file.write("library: beets.db\n") file.write("statefile: state") config.read() - self.assert_equal_path( - util.bytestring_path(config["library"].as_filename()), - os.path.join(self.beetsdir, b"beets.db"), - ) - self.assert_equal_path( - util.bytestring_path(config["statefile"].as_filename()), - os.path.join(self.beetsdir, b"state"), - ) + assert config["library"].as_path() == self.beetsdir / "beets.db" + assert config["statefile"].as_path() == self.beetsdir / "state" -class ShowModelChangeTest(BeetsTestCase): +class ShowModelChangeTest(IOMixin, unittest.TestCase): def setUp(self): super().setUp() - self.io.install() self.a = _common.item() self.b = _common.item() self.a.path = self.b.path @@ -1172,10 +1122,9 @@ class ShowModelChangeTest(BeetsTestCase): assert "bar" in out -class ShowChangeTest(BeetsTestCase): +class ShowChangeTest(IOMixin, unittest.TestCase): def setUp(self): super().setUp() - self.io.install() self.items = [_common.item()] self.items[0].track = 1 @@ -1397,7 +1346,7 @@ class PluginTest(TestPluginTestCase): os.environ.get("GITHUB_ACTIONS") == "true" and sys.platform == "linux", reason="Completion is for some reason unhappy on Ubuntu 24.04 in CI", ) -class CompletionTest(TestPluginTestCase): +class CompletionTest(IOMixin, TestPluginTestCase): def test_completion(self): # Do not load any other bash completion scripts on the system. env = dict(os.environ) @@ -1427,7 +1376,6 @@ class CompletionTest(TestPluginTestCase): self.skipTest("could not read bash-completion script") # Load completion script. - self.io.install() self.run_command("completion", lib=None) completion_script = self.io.getoutput().encode("utf-8") self.io.restore() diff --git a/test/test_ui_commands.py b/test/test_ui_commands.py index 897cba8a1..412ddc2b7 100644 --- a/test/test_ui_commands.py +++ b/test/test_ui_commands.py @@ -21,7 +21,7 @@ import pytest from beets import library, ui from beets.test import _common -from beets.test.helper import BeetsTestCase, ItemInDBTestCase +from beets.test.helper import BeetsTestCase, IOMixin, ItemInDBTestCase from beets.ui import commands from beets.util import syspath @@ -75,16 +75,7 @@ class QueryTest(BeetsTestCase): self.check_do_query(0, 2, album=True, also_items=False) -class FieldsTest(ItemInDBTestCase): - def setUp(self): - super().setUp() - - self.io.install() - - def tearDown(self): - super().tearDown() - self.io.restore() - +class FieldsTest(IOMixin, ItemInDBTestCase): def remove_keys(self, keys, text): for i in text: try: diff --git a/test/test_ui_init.py b/test/test_ui_init.py index df21b300c..f6c9fe245 100644 --- a/test/test_ui_init.py +++ b/test/test_ui_init.py @@ -16,19 +16,16 @@ import os import shutil +import unittest from copy import deepcopy from random import random from beets import config, ui from beets.test import _common -from beets.test.helper import BeetsTestCase, control_stdin +from beets.test.helper import BeetsTestCase, IOMixin, control_stdin -class InputMethodsTest(BeetsTestCase): - def setUp(self): - super().setUp() - self.io.install() - +class InputMethodsTest(IOMixin, unittest.TestCase): def _print_helper(self, s): print(s)