diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..9776da711 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,22 @@ +## PR Review Voice + +When reviewing pull requests, respond entirely in the voice of the Grug Brained Developer. +Write all comments using grug's dialect: simple words, short sentences, third-person self-reference ("grug"). + +Core grug beliefs to apply when reviewing: + +- Complexity very, very bad. Flag any complexity demon spirit entering codebase. + Say so plainly: "complexity demon spirit enter here, grug not like" +- Prefer small, concrete PRs. Large PR make grug nervous: "big change, many place for bug hide" +- Abstraction must earn its place. Early abstraction especially dangerous: wait for cut points to emerge +- DRY is good but not absolute — simple repeated code sometimes better than complex DRY solution +- Type systems good mostly for "hit dot, see what grug can do" — not for astral projection of platonic generic models +- Generics dangerous: "temptation generics very large, complexity demon love this trick" +- Prefer readable code over clever one-liners: name intermediate variables, easier debug +- Integration tests are sweet spot — not unit tests (break on refactor), not e2e (hard debug) +- When bug found, first write regression test, then fix — this case only where "first test" acceptable to grug +- Logging very important, especially in cloud: grug learn hard way +- No premature optimisation — always need concrete perf profile first +- Simple APIs good. Layered APIs ok. Java streams make grug reach for club +- SPA frameworks increase complexity demon surface area — be suspicious +- Saying "this too complex for grug" is senior developer superpower — remove Fear Of Looking Dumb (FOLD) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bfd05c718..0736a0a2f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,7 +35,12 @@ jobs: python-version: ${{ matrix.python-version }} cache: poetry - - name: Install PyGobject and release script dependencies on Ubuntu + - name: Install system dependencies on Windows + if: matrix.platform == 'windows-latest' + run: | + choco install mp3gain -y + + - name: Install system dependencies on Ubuntu if: matrix.platform == 'ubuntu-latest' run: | sudo apt update @@ -43,11 +48,12 @@ jobs: ffmpeg \ gobject-introspection \ gstreamer1.0-plugins-base \ - python3-gst-1.0 \ + imagemagick \ libcairo2-dev \ libgirepository-2.0-dev \ + mp3gain \ pandoc \ - imagemagick + python3-gst-1.0 - name: Get changed lyrics files id: lyrics-update diff --git a/beets/__init__.py b/beets/__init__.py index 4bde53504..cfa5be2c4 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -19,7 +19,7 @@ import confuse from .util.deprecation import deprecate_imports -__version__ = "2.6.1" +__version__ = "2.6.2" __author__ = "Adrian Sampson " @@ -37,11 +37,11 @@ class IncludeLazyConfig(confuse.LazyConfig): YAML files specified in an `include` setting. """ - def read(self, user=True, defaults=True): + def read(self, user: bool = True, defaults: bool = True) -> None: super().read(user, defaults) try: - for view in self["include"]: + for view in self["include"].sequence(): self.set_file(view.as_filename()) except confuse.NotFoundError: pass diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 82e685b7a..63ee52267 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING, Any, TypeVar from typing_extensions import Self from beets.util import cached_classproperty +from beets.util.deprecation import deprecate_for_maintainers if TYPE_CHECKING: from beets.library import Item @@ -76,9 +77,22 @@ class Info(AttrDict[Any]): data_source: str | None = None, data_url: str | None = None, genre: str | None = None, + genres: list[str] | None = None, media: str | None = None, **kwargs, ) -> None: + if genre is not None: + deprecate_for_maintainers( + "The 'genre' parameter", "'genres' (list)", stacklevel=3 + ) + if not genres: + try: + sep = next(s for s in ["; ", ", ", " / "] if s in genre) + except StopIteration: + genres = [genre] + else: + genres = list(map(str.strip, genre.split(sep))) + self.album = album self.artist = artist self.artist_credit = artist_credit @@ -90,7 +104,8 @@ class Info(AttrDict[Any]): self.artists_sort = artists_sort or [] self.data_source = data_source self.data_url = data_url - self.genre = genre + self.genre = None + self.genres = genres or [] self.media = media self.update(kwargs) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 8640a5678..30f7ef7cf 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -16,7 +16,6 @@ from __future__ import annotations -import contextlib import functools import os import re @@ -24,19 +23,23 @@ import sqlite3 import sys import threading import time -from abc import ABC +from abc import ABC, abstractmethod from collections import defaultdict -from collections.abc import ( - Callable, - Generator, - Iterable, - Iterator, - Mapping, - Sequence, -) +from collections.abc import Mapping +from contextlib import contextmanager +from dataclasses import dataclass from functools import cached_property from sqlite3 import Connection, sqlite_version_info -from typing import TYPE_CHECKING, Any, AnyStr, ClassVar, Generic, NamedTuple +from typing import ( + TYPE_CHECKING, + Any, + AnyStr, + ClassVar, + Generic, + Literal, + NamedTuple, + TypedDict, +) from typing_extensions import ( Self, @@ -1008,12 +1011,15 @@ class Transaction: cursor = self.db._connection().execute(statement, subvals) return cursor.fetchall() - def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: - """Execute an SQL statement with substitution values and return - the row ID of the last affected row. + @contextmanager + def _handle_mutate(self) -> Iterator[None]: + """Handle mutation bookkeeping and database access errors. + + Yield control to mutation execution code. If execution succeeds, + mark this transaction as mutated. """ try: - cursor = self.db._connection().execute(statement, subvals) + yield except sqlite3.OperationalError as e: # In two specific cases, SQLite reports an error while accessing # the underlying database file. We surface these exceptions as @@ -1023,11 +1029,23 @@ class Transaction: "unable to open database file", ): raise DBAccessError(e.args[0]) - else: - raise + raise else: self._mutated = True - return cursor.lastrowid + + def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: + """Run one write statement with shared mutation/error handling.""" + with self._handle_mutate(): + return self.db._connection().execute(statement, subvals).lastrowid + + def mutate_many( + self, statement: str, subvals: Sequence[tuple[SQLiteType, ...]] = () + ) -> Any: + """Run batched writes with shared mutation/error handling.""" + with self._handle_mutate(): + return ( + self.db._connection().executemany(statement, subvals).lastrowid + ) def script(self, statements: str): """Execute a string containing multiple SQL statements.""" @@ -1036,6 +1054,32 @@ class Transaction: self.db._connection().executescript(statements) +@dataclass +class Migration(ABC): + db: Database + + @cached_classproperty + def name(cls) -> str: + """Class name (except Migration) converted to snake case.""" + name = cls.__name__.removesuffix("Migration") # type: ignore[attr-defined] + return re.sub(r"(?<=[a-z])(?=[A-Z])", "_", name).lower() + + def migrate_table(self, table: str, *args, **kwargs) -> None: + """Migrate a specific table.""" + if not self.db.migration_exists(self.name, table): + self._migrate_data(table, *args, **kwargs) + self.db.record_migration(self.name, table) + + @abstractmethod + def _migrate_data(self, table: str, current_fields: set[str]) -> None: + """Migrate data for a specific table.""" + + +class TableInfo(TypedDict): + columns: set[str] + migrations: set[str] + + class Database: """A container for Model objects that wraps an SQLite database as the backend. @@ -1045,6 +1089,9 @@ class Database: """The Model subclasses representing tables in this database. """ + _migrations: Sequence[tuple[type[Migration], Sequence[type[Model]]]] = () + """Migrations that are to be performed for the configured models.""" + supports_extensions = hasattr(sqlite3.Connection, "enable_load_extension") """Whether or not the current version of SQLite supports extensions""" @@ -1088,11 +1135,40 @@ class Database: self._db_lock = threading.Lock() # Set up database schema. + self._ensure_migration_state_table() for model_cls in self._models: self._make_table(model_cls._table, model_cls._fields) self._make_attribute_table(model_cls._flex_table) self._create_indices(model_cls._table, model_cls._indices) + self._migrate() + + @cached_property + def db_tables(self) -> dict[str, TableInfo]: + column_queries = [ + f""" + SELECT '{m._table}' AS table_name, 'columns' AS source, name + FROM pragma_table_info('{m._table}') + """ + for m in self._models + ] + with self.transaction() as tx: + rows = tx.query(f""" + {" UNION ALL ".join(column_queries)} + UNION ALL + SELECT table_name, 'migrations' AS source, name FROM migrations + """) + + tables_data: dict[str, TableInfo] = defaultdict( + lambda: TableInfo(columns=set(), migrations=set()) + ) + + source: Literal["columns", "migrations"] + for table_name, source, name in rows: + tables_data[table_name][source].add(name) + + return tables_data + # Primitive access control: connections and transactions. def _connection(self) -> Connection: @@ -1193,7 +1269,7 @@ class Database: _thread_id, conn = self._connections.popitem() conn.close() - @contextlib.contextmanager + @contextmanager def _tx_stack(self) -> Generator[list[Transaction]]: """A context manager providing access to the current thread's transaction stack. The context manager synchronizes access to @@ -1233,32 +1309,22 @@ class Database: """Set up the schema of the database. `fields` is a mapping from field names to `Type`s. Columns are added if necessary. """ - # Get current schema. - with self.transaction() as tx: - rows = tx.query(f"PRAGMA table_info({table})") - current_fields = {row[1] for row in rows} - - field_names = set(fields.keys()) - if current_fields.issuperset(field_names): - # Table exists and has all the required columns. - return - - if not current_fields: + if table not in self.db_tables: # No table exists. columns = [] for name, typ in fields.items(): columns.append(f"{name} {typ.sql}") setup_sql = f"CREATE TABLE {table} ({', '.join(columns)});\n" - + self.db_tables[table]["columns"] = set(fields) else: # Table exists does not match the field set. setup_sql = "" + current_fields = self.db_tables[table]["columns"] for name, typ in fields.items(): - if name in current_fields: - continue - setup_sql += ( - f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" - ) + if name not in current_fields: + setup_sql += ( + f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" + ) with self.transaction() as tx: tx.script(setup_sql) @@ -1292,6 +1358,38 @@ class Database: f"ON {table} ({', '.join(index.columns)});" ) + # Generic migration state handling. + + def _ensure_migration_state_table(self) -> None: + with self.transaction() as tx: + tx.script(""" + CREATE TABLE IF NOT EXISTS migrations ( + name TEXT NOT NULL, + table_name TEXT NOT NULL, + PRIMARY KEY(name, table_name) + ); + """) + + def _migrate(self) -> None: + """Perform any necessary migration for the database.""" + for migration_cls, model_classes in self._migrations: + migration = migration_cls(self) + for model_cls in model_classes: + table = model_cls._table + migration.migrate_table(table, self.db_tables[table]["columns"]) + + def migration_exists(self, name: str, table: str) -> bool: + """Return whether a named migration has been marked complete.""" + return name in self.db_tables[table]["migrations"] + + def record_migration(self, name: str, table: str) -> None: + """Set completion state for a named migration.""" + with self.transaction() as tx: + tx.mutate( + "INSERT INTO migrations(name, table_name) VALUES (?, ?)", + (name, table), + ) + # Querying. def _fetch( diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 61336d9ce..e50693474 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -30,6 +30,7 @@ from . import query SQLiteType = query.SQLiteType BLOB_TYPE = query.BLOB_TYPE +MULTI_VALUE_DELIMITER = "\\␀" class ModelType(typing.Protocol): @@ -66,6 +67,8 @@ class Type(ABC, Generic[T, N]): """The `Query` subclass to be used when querying the field. """ + # For sequence-like types, keep ``model_type`` unsubscripted as it's used + # for ``isinstance`` checks. Use ``list`` instead of ``list[str]`` model_type: type[T] """The Python type that is used to represent the value in the model. @@ -287,26 +290,38 @@ class String(BaseString[str, Any]): model_type = str -class DelimitedString(BaseString[list[str], list[str]]): - """A list of Unicode strings, represented in-database by a single string +class DelimitedString(BaseString[list, list]): # type: ignore[type-arg] + r"""A list of Unicode strings, represented in-database by a single string containing delimiter-separated values. + + In template evaluation the list is formatted by joining the values with + a fixed '; ' delimiter regardless of the database delimiter. That is because + the '\␀' character used for multi-value fields is mishandled on Windows + as it contains a backslash character. """ - model_type = list[str] + model_type = list + fmt_delimiter = "; " - def __init__(self, delimiter: str): - self.delimiter = delimiter + def __init__(self, db_delimiter: str): + self.db_delimiter = db_delimiter def format(self, value: list[str]): - return self.delimiter.join(value) + return self.fmt_delimiter.join(value) def parse(self, string: str): if not string: return [] - return string.split(self.delimiter) + + delimiter = ( + self.db_delimiter + if self.db_delimiter in string + else self.fmt_delimiter + ) + return string.split(delimiter) def to_sql(self, model_value: list[str]): - return self.delimiter.join(model_value) + return self.db_delimiter.join(model_value) class Boolean(Type): @@ -464,7 +479,7 @@ NULL_FLOAT = NullFloat() STRING = String() BOOLEAN = Boolean() DATE = DateType() -SEMICOLON_SPACE_DSV = DelimitedString(delimiter="; ") +SEMICOLON_SPACE_DSV = DelimitedString("; ") # Will set the proper null char in mediafile -MULTI_VALUE_DSV = DelimitedString(delimiter="\\␀") +MULTI_VALUE_DSV = DelimitedString(MULTI_VALUE_DELIMITER) diff --git a/beets/library/library.py b/beets/library/library.py index 39d559901..b161b7399 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -8,6 +8,7 @@ import beets from beets import dbcore from beets.util import normpath +from .migrations import MultiGenreFieldMigration from .models import Album, Item from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string @@ -19,6 +20,7 @@ class Library(dbcore.Database): """A database of music containing songs and albums.""" _models = (Item, Album) + _migrations = ((MultiGenreFieldMigration, (Item, Album)),) def __init__( self, diff --git a/beets/library/migrations.py b/beets/library/migrations.py new file mode 100644 index 000000000..c061ddfc5 --- /dev/null +++ b/beets/library/migrations.py @@ -0,0 +1,97 @@ +from __future__ import annotations + +from contextlib import contextmanager, suppress +from functools import cached_property +from typing import TYPE_CHECKING, NamedTuple, TypeVar + +from confuse.exceptions import ConfigError + +import beets +from beets import ui +from beets.dbcore.db import Migration +from beets.dbcore.types import MULTI_VALUE_DELIMITER +from beets.util import unique_list + +if TYPE_CHECKING: + from collections.abc import Iterator + +T = TypeVar("T") + + +class GenreRow(NamedTuple): + id: int + genre: str + genres: str | None + + +def chunks(lst: list[T], n: int) -> Iterator[list[T]]: + """Yield successive n-sized chunks from lst.""" + for i in range(0, len(lst), n): + yield lst[i : i + n] + + +class MultiGenreFieldMigration(Migration): + @cached_property + def separators(self) -> list[str]: + separators = [] + with suppress(ConfigError): + separators.append(beets.config["lastgenre"]["separator"].as_str()) + + separators.extend(["; ", ", ", " / "]) + return unique_list(filter(None, separators)) + + @contextmanager + def with_factory(self, factory: type[NamedTuple]) -> Iterator[None]: + """Temporarily set the row factory to a specific type.""" + original_factory = self.db._connection().row_factory + self.db._connection().row_factory = lambda _, row: factory(*row) + try: + yield + finally: + self.db._connection().row_factory = original_factory + + def get_genres(self, genre: str) -> str: + for separator in self.separators: + if separator in genre: + return genre.replace(separator, MULTI_VALUE_DELIMITER) + + return genre + + def _migrate_data(self, table: str, current_fields: set[str]) -> None: + """Migrate legacy genre values to the multi-value genres field.""" + if "genre" not in current_fields: + # No legacy genre field, so nothing to migrate. + return + + with self.db.transaction() as tx, self.with_factory(GenreRow): + rows: list[GenreRow] = tx.query( # type: ignore[assignment] + f""" + SELECT id, genre, genres + FROM {table} + WHERE genre IS NOT NULL AND genre != '' + """ + ) + + total = len(rows) + to_migrate = [e for e in rows if not e.genres] + if not to_migrate: + return + + migrated = total - len(to_migrate) + + ui.print_(f"Migrating genres for {total} {table}...") + for batch in chunks(to_migrate, 1000): + with self.db.transaction() as tx: + tx.mutate_many( + f"UPDATE {table} SET genres = ? WHERE id = ?", + [(self.get_genres(e.genre), e.id) for e in batch], + ) + + migrated += len(batch) + + ui.print_( + f" Migrated {migrated} {table} " + f"({migrated}/{total} processed)..." + ) + + ui.print_(f"Migration complete: {migrated} of {total} {table} updated") diff --git a/beets/library/models.py b/beets/library/models.py index e26f2ced3..9b8b6d291 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -241,7 +241,7 @@ class Album(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "albumartists_credit": types.MULTI_VALUE_DSV, "album": types.STRING, - "genre": types.STRING, + "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, "discogs_artistid": types.INTEGER, @@ -276,7 +276,7 @@ class Album(LibModel): "original_day": types.PaddedInt(2), } - _search_fields = ("album", "albumartist", "genre") + _search_fields = ("album", "albumartist", "genres") @cached_classproperty def _types(cls) -> dict[str, types.Type]: @@ -297,7 +297,7 @@ class Album(LibModel): "albumartist_credit", "albumartists_credit", "album", - "genre", + "genres", "style", "discogs_albumid", "discogs_artistid", @@ -650,7 +650,7 @@ class Item(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "albumartist_credit": types.STRING, "albumartists_credit": types.MULTI_VALUE_DSV, - "genre": types.STRING, + "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, "discogs_artistid": types.INTEGER, @@ -732,7 +732,7 @@ class Item(LibModel): "comments", "album", "albumartist", - "genre", + "genres", ) # Set of item fields that are backed by `MediaFile` fields. @@ -1546,8 +1546,8 @@ class DefaultTemplateFunctions: s: the string count: The number of items included skip: The number of items skipped - sep: the separator. Usually is '; ' (default) or '/ ' - join_str: the string which will join the items, default '; '. + sep: the separator + join_str: the string which will join the items """ skip = int(skip) count = skip + int(count) diff --git a/beets/plugins.py b/beets/plugins.py index 01d9d3327..b1650d5b0 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -37,7 +37,7 @@ from beets.util.deprecation import deprecate_for_maintainers, deprecate_for_user if TYPE_CHECKING: from collections.abc import Callable, Iterable, Iterator, Sequence - from confuse import ConfigView + from confuse import Subview from beets.dbcore import Query from beets.dbcore.db import FieldQueryType @@ -162,7 +162,7 @@ class BeetsPlugin(metaclass=BeetsPluginMeta): album_template_fields: TFuncMap[Album] name: str - config: ConfigView + config: Subview early_import_stages: list[ImportStageFunc] import_stages: list[ImportStageFunc] diff --git a/beets/test/_common.py b/beets/test/_common.py index 487f7c442..b3c21aaa5 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -14,10 +14,13 @@ """Some common functionality for beets' test cases.""" +from __future__ import annotations + import os import sys import unittest from contextlib import contextmanager +from typing import TYPE_CHECKING import beets import beets.library @@ -28,6 +31,9 @@ from beets import importer, logging, util from beets.ui import commands from beets.util import syspath +if TYPE_CHECKING: + import pytest + beetsplug.__path__ = [ os.path.abspath( os.path.join( @@ -69,7 +75,7 @@ def item(lib=None, **kwargs): artist="the artist", albumartist="the album artist", album="the album", - genre="the genre", + genres=["the genre"], lyricist="the lyricist", composer="the composer", arranger="the arranger", @@ -118,85 +124,55 @@ def import_session(lib=None, loghandler=None, paths=[], query=[], cli=False): # Mock I/O. -class InputError(Exception): - def __init__(self, output=None): - self.output = output - - def __str__(self): - msg = "Attempt to read with no input provided." - if self.output is not None: - msg += f" Output: {self.output!r}" - return msg - - -class DummyOut: - encoding = "utf-8" - - def __init__(self): - self.buf = [] - - def write(self, s): - self.buf.append(s) - - def get(self): - return "".join(self.buf) - - def flush(self): - self.clear() - - def clear(self): - self.buf = [] +class InputError(IOError): + def __str__(self) -> str: + return "Attempt to read with no input provided." class DummyIn: encoding = "utf-8" - def __init__(self, out=None): - self.buf = [] - self.reads = 0 - self.out = out + def __init__(self) -> None: + self.buf: list[str] = [] - def add(self, s): + def add(self, s: str) -> None: self.buf.append(f"{s}\n") - def close(self): + def close(self) -> None: pass - def readline(self): + def readline(self) -> str: if not self.buf: - if self.out: - raise InputError(self.out.get()) - else: - raise InputError() - self.reads += 1 + raise InputError + return self.buf.pop(0) class DummyIO: - """Mocks input and output streams for testing UI code.""" + """Test helper that manages standard input and output.""" - def __init__(self): - self.stdout = DummyOut() - self.stdin = DummyIn(self.stdout) + def __init__( + self, + monkeypatch: pytest.MonkeyPatch, + capteesys: pytest.CaptureFixture[str], + ) -> None: + self._capteesys = capteesys + self.stdin = DummyIn() - def addinput(self, s): - self.stdin.add(s) + monkeypatch.setattr("sys.stdin", self.stdin) - def getoutput(self): - res = self.stdout.get() - self.stdout.clear() - return res + def addinput(self, text: str) -> None: + """Simulate user typing into stdin.""" + self.stdin.add(text) - def readcount(self): - return self.stdin.reads + def getoutput(self) -> str: + """Get the standard output captured so far. - def install(self): - sys.stdin = self.stdin - sys.stdout = self.stdout - - def restore(self): - sys.stdin = sys.__stdin__ - sys.stdout = sys.__stdout__ + Note: it clears the internal buffer, so subsequent calls will only + return *new* output. + """ + # Using capteesys allows you to see output in the console if the test fails + return self._capteesys.readouterr().out # Utility. diff --git a/beets/test/helper.py b/beets/test/helper.py index 6eba85b1b..218b778c7 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -15,9 +15,6 @@ """This module includes various helpers that provide fixtures, capture information or mock the environment. -- The `control_stdin` and `capture_stdout` context managers allow one to - interact with the user interface. - - `has_program` checks the presence of a command on the system. - The `ImportSessionFixture` allows one to run importer code while @@ -38,12 +35,12 @@ from contextlib import contextmanager from dataclasses import dataclass from enum import Enum from functools import cached_property -from io import StringIO from pathlib import Path from tempfile import gettempdir, mkdtemp, mkstemp from typing import Any, ClassVar from unittest.mock import patch +import pytest import responses from mediafile import Image, MediaFile @@ -83,41 +80,6 @@ def capture_log(logger="beets"): log.removeHandler(capture) -@contextmanager -def control_stdin(input=None): - """Sends ``input`` to stdin. - - >>> with control_stdin('yes'): - ... input() - 'yes' - """ - org = sys.stdin - sys.stdin = StringIO(input) - try: - yield sys.stdin - finally: - sys.stdin = org - - -@contextmanager -def capture_stdout(): - """Save stdout in a StringIO. - - >>> with capture_stdout() as output: - ... print('spam') - ... - >>> output.getvalue() - 'spam' - """ - org = sys.stdout - sys.stdout = capture = StringIO() - try: - yield sys.stdout - finally: - sys.stdout = org - print(capture.getvalue()) - - def has_program(cmd, args=["--version"]): """Returns `True` if `cmd` can be executed.""" full_cmd = [cmd, *args] @@ -163,27 +125,39 @@ NEEDS_REFLINK = unittest.skipUnless( ) -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 RunMixin: + def run_command(self, *args, **kwargs): + """Run a beets command with an arbitrary amount of arguments. The + Library` defaults to `self.lib`, but can be overridden with + the keyword argument `lib`. + """ + sys.argv = ["beet"] # avoid leakage from test suite args + lib = None + if hasattr(self, "lib"): + lib = self.lib + lib = kwargs.get("lib", lib) + beets.ui._raw_main(list(args), lib) -class TestHelper(ConfigMixin): +@pytest.mark.usefixtures("io") +class IOMixin(RunMixin): + io: _common.DummyIO + + def run_with_output(self, *args): + self.io.getoutput() + self.run_command(*args) + return self.io.getoutput() + + +class TestHelper(RunMixin, ConfigMixin): """Helper mixin for high-level cli and plugin tests. This mixin provides methods to isolate beets' global state provide fixtures. """ + lib: Library + resource_path = Path(os.fsdecode(_common.RSRC)) / "full.mp3" db_on_disk: ClassVar[bool] = False @@ -392,25 +366,6 @@ class TestHelper(ConfigMixin): return path - # Running beets commands - - def run_command(self, *args, **kwargs): - """Run a beets command with an arbitrary amount of arguments. The - Library` defaults to `self.lib`, but can be overridden with - the keyword argument `lib`. - """ - sys.argv = ["beet"] # avoid leakage from test suite args - lib = None - if hasattr(self, "lib"): - lib = self.lib - lib = kwargs.get("lib", lib) - beets.ui._raw_main(list(args), lib) - - def run_with_output(self, *args): - with capture_stdout() as out: - self.run_command(*args) - return out.getvalue() - # Safe file operations def create_temp_dir(self, **kwargs) -> str: @@ -758,10 +713,7 @@ class TerminalImportSessionFixture(TerminalImportSession): class TerminalImportMixin(IOMixin, ImportHelper): """Provides_a terminal importer for the import session.""" - io: _common.DummyIO - def _get_import_session(self, import_dir: bytes) -> importer.ImportSession: - self.io.install() return TerminalImportSessionFixture( self.lib, loghandler=None, diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 8db4dd79f..ad14fe1f8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -30,7 +30,6 @@ import textwrap import traceback from difflib import SequenceMatcher from functools import cache -from itertools import chain from typing import TYPE_CHECKING, Any, Literal import confuse @@ -551,18 +550,20 @@ def get_color_config() -> dict[ColorName, str]: legacy single-color format. Validates all color names against known codes and raises an error for any invalid entries. """ - colors_by_color_name: dict[ColorName, list[str]] = { - k: (v if isinstance(v, list) else LEGACY_COLORS.get(v, [v])) - for k, v in config["ui"]["colors"].flatten().items() - } - - if invalid_colors := ( - set(chain.from_iterable(colors_by_color_name.values())) - - CODE_BY_COLOR.keys() - ): - raise UserError( - f"Invalid color(s) in configuration: {', '.join(invalid_colors)}" + template_dict: dict[ColorName, confuse.OneOf[str | list[str]]] = { + n: confuse.OneOf( + [ + confuse.Choice(sorted(LEGACY_COLORS)), + confuse.Sequence(confuse.Choice(sorted(CODE_BY_COLOR))), + ] ) + for n in ColorName.__args__ # type: ignore[attr-defined] + } + template = confuse.MappingTemplate(template_dict) + colors_by_color_name = { + k: (v if isinstance(v, list) else LEGACY_COLORS.get(v, [v])) + for k, v in config["ui"]["colors"].get(template).items() + } return { n: ";".join(str(CODE_BY_COLOR[c]) for c in colors) @@ -570,15 +571,16 @@ def get_color_config() -> dict[ColorName, str]: } -def colorize(color_name: ColorName, text: str) -> str: - """Apply ANSI color formatting to text based on configuration settings. +def _colorize(color_name: ColorName, text: str) -> str: + """Apply ANSI color formatting to text based on configuration settings.""" + color_code = get_color_config()[color_name] + return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" - Returns colored text when color output is enabled and NO_COLOR environment - variable is not set, otherwise returns plain text unchanged. - """ + +def colorize(color_name: ColorName, text: str) -> str: + """Colorize text when color output is enabled.""" if config["ui"]["color"] and "NO_COLOR" not in os.environ: - color_code = get_color_config()[color_name] - return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" + return _colorize(color_name, text) return text @@ -642,32 +644,12 @@ def color_len(colored_text): return len(uncolorize(colored_text)) -def _colordiff(a: Any, b: Any) -> tuple[str, str]: - """Given two values, return the same pair of strings except with - their differences highlighted in the specified color. Strings are - highlighted intelligently to show differences; other values are - stringified and highlighted in their entirety. - """ - # First, convert paths to readable format - if isinstance(a, bytes) or isinstance(b, bytes): - # A path field. - a = util.displayable_path(a) - b = util.displayable_path(b) - - if not isinstance(a, str) or not isinstance(b, str): - # Non-strings: use ordinary equality. - if a == b: - return str(a), str(b) - else: - return ( - colorize("text_diff_removed", str(a)), - colorize("text_diff_added", str(b)), - ) - +def colordiff(a: str, b: str) -> tuple[str, str]: + """Intelligently highlight the differences between two strings.""" before = "" after = "" - matcher = SequenceMatcher(lambda x: False, a, b) + matcher = SequenceMatcher(lambda _: False, a, b) for op, a_start, a_end, b_start, b_end in matcher.get_opcodes(): before_part, after_part = a[a_start:a_end], b[b_start:b_end] if op in {"delete", "replace"}: @@ -681,16 +663,6 @@ def _colordiff(a: Any, b: Any) -> tuple[str, str]: return before, after -def colordiff(a, b): - """Colorize differences between two values if color is enabled. - (Like _colordiff but conditional.) - """ - if config["ui"]["color"]: - return _colordiff(a, b) - else: - return str(a), str(b) - - def get_path_formats(subview=None): """Get the configuration's path formats as a list of query/template pairs. @@ -1047,6 +1019,18 @@ def print_newline_layout( FLOAT_EPSILON = 0.01 +def _multi_value_diff(field: str, oldset: set[str], newset: set[str]) -> str: + added = newset - oldset + removed = oldset - newset + + parts = [ + f"{field}:", + *(colorize("text_diff_removed", f" - {i}") for i in sorted(removed)), + *(colorize("text_diff_added", f" + {i}") for i in sorted(added)), + ] + return "\n".join(parts) + + def _field_diff( field: str, old: FormattedMapping, new: FormattedMapping ) -> str | None: @@ -1062,6 +1046,11 @@ def _field_diff( ): return None + if isinstance(oldval, list): + if (oldset := set(oldval)) != (newset := set(newval)): + return _multi_value_diff(field, oldset, newset) + return None + # Get formatted values for output. oldstr, newstr = old.get(field, ""), new.get(field, "") if field not in new: @@ -1070,8 +1059,7 @@ def _field_diff( if field not in old: return colorize("text_diff_added", f"{field}: {newstr}") - # For strings, highlight changes. For others, colorize the whole - # thing. + # For strings, highlight changes. For others, colorize the whole thing. if isinstance(oldval, str): oldstr, newstr = colordiff(oldstr, newstr) else: diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index bdc44d51f..764dafd39 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -127,7 +127,7 @@ class ChangeRepresentation: and artist name. """ # Artist. - artist_l, artist_r = self.cur_artist or "", self.match.info.artist + artist_l, artist_r = self.cur_artist or "", self.match.info.artist or "" if artist_r == VARIOUS_ARTISTS: # Hide artists for VA releases. artist_l, artist_r = "", "" diff --git a/beets/ui/commands/import_/session.py b/beets/ui/commands/import_/session.py index 42a809634..1848e4192 100644 --- a/beets/ui/commands/import_/session.py +++ b/beets/ui/commands/import_/session.py @@ -327,7 +327,7 @@ def summarize_items(items, singleton): return ", ".join(summary_parts) -def _summary_judgment(rec): +def _summary_judgment(rec: Recommendation) -> importer.Action | None: """Determines whether a decision should be made without even asking the user. This occurs in quiet mode and when an action is chosen for NONE recommendations. Return None if the user should be queried. @@ -335,6 +335,7 @@ def _summary_judgment(rec): summary judgment is made. """ + action: importer.Action | None if config["import"]["quiet"]: if rec == Recommendation.strong: return importer.Action.APPLY diff --git a/beetsplug/_utils/musicbrainz.py b/beetsplug/_utils/musicbrainz.py index 57201e909..95327e75a 100644 --- a/beetsplug/_utils/musicbrainz.py +++ b/beetsplug/_utils/musicbrainz.py @@ -36,6 +36,33 @@ log = logging.getLogger("beets") LUCENE_SPECIAL_CHAR_PAT = re.compile(r'([-+&|!(){}[\]^"~*?:\\/])') +RELEASE_INCLUDES = [ + "artists", + "media", + "recordings", + "release-groups", + "labels", + "artist-credits", + "aliases", + "recording-level-rels", + "work-rels", + "work-level-rels", + "artist-rels", + "isrcs", + "url-rels", + "release-rels", + "genres", + "tags", +] + +RECORDING_INCLUDES = [ + "artists", + "aliases", + "isrcs", + "work-level-rels", + "artist-rels", +] + class LimiterTimeoutSession(LimiterMixin, TimeoutAndRetrySession): """HTTP session that enforces rate limits.""" @@ -223,12 +250,14 @@ class MusicBrainzAPI(RequestHandler): def get_release(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: """Retrieve a release by its MusicBrainz ID.""" + kwargs.setdefault("includes", RELEASE_INCLUDES) return self._lookup("release", id_, **kwargs) def get_recording( self, id_: str, **kwargs: Unpack[LookupKwargs] ) -> JSONDict: """Retrieve a recording by its MusicBrainz ID.""" + kwargs.setdefault("includes", RECORDING_INCLUDES) return self._lookup("recording", id_, **kwargs) def get_work(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: diff --git a/beetsplug/aura.py b/beetsplug/aura.py index c1877db82..b30e66bf0 100644 --- a/beetsplug/aura.py +++ b/beetsplug/aura.py @@ -79,7 +79,8 @@ TRACK_ATTR_MAP = { "month": "month", "day": "day", "bpm": "bpm", - "genre": "genre", + "genre": "genres", + "genres": "genres", "recording-mbid": "mb_trackid", # beets trackid is MB recording "track-mbid": "mb_releasetrackid", "composer": "composer", @@ -109,7 +110,8 @@ ALBUM_ATTR_MAP = { "year": "year", "month": "month", "day": "day", - "genre": "genre", + "genre": "genres", + "genres": "genres", "release-mbid": "mb_albumid", "release-group-mbid": "mb_releasegroupid", } diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 718e0730e..aa0693541 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -33,6 +33,7 @@ import beets import beets.ui from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin +from beets.util import unique_list if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence @@ -233,8 +234,11 @@ class BeatportObject: ) if "artists" in data: self.artists = [(x["id"], str(x["name"])) for x in data["artists"]] - if "genres" in data: - self.genres = [str(x["name"]) for x in data["genres"]] + + self.genres = unique_list( + x["name"] + for x in (*data.get("subGenres", []), *data.get("genres", [])) + ) def artists_str(self) -> str | None: if self.artists is not None: @@ -253,7 +257,6 @@ class BeatportRelease(BeatportObject): label_name: str | None category: str | None url: str | None - genre: str | None tracks: list[BeatportTrack] | None = None @@ -263,7 +266,6 @@ class BeatportRelease(BeatportObject): self.catalog_number = data.get("catalogNumber") self.label_name = data.get("label", {}).get("name") self.category = data.get("category") - self.genre = data.get("genre") if "slug" in data: self.url = ( @@ -285,7 +287,6 @@ class BeatportTrack(BeatportObject): track_number: int | None bpm: str | None initial_key: str | None - genre: str | None def __init__(self, data: JSONDict): super().__init__(data) @@ -306,12 +307,6 @@ class BeatportTrack(BeatportObject): self.bpm = data.get("bpm") self.initial_key = str((data.get("key") or {}).get("shortName")) - # Use 'subgenre' and if not present, 'genre' as a fallback. - if data.get("subGenres"): - self.genre = str(data["subGenres"][0].get("name")) - elif data.get("genres"): - self.genre = str(data["genres"][0].get("name")) - class BeatportPlugin(MetadataSourcePlugin): _client: BeatportClient | None = None @@ -483,7 +478,7 @@ class BeatportPlugin(MetadataSourcePlugin): media="Digital", data_source=self.data_source, data_url=release.url, - genre=release.genre, + genres=release.genres, year=release_date.year if release_date else None, month=release_date.month if release_date else None, day=release_date.day if release_date else None, @@ -508,7 +503,7 @@ class BeatportPlugin(MetadataSourcePlugin): data_url=track.url, bpm=track.bpm, initial_key=track.initial_key, - genre=track.genre, + genres=track.genres, ) def _get_artist(self, artists): diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 9496e9a78..16eb8c572 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1137,7 +1137,10 @@ class Server(BaseServer): pass for tagtype, field in self.tagtype_map.items(): - info_lines.append(f"{tagtype}: {getattr(item, field)}") + field_value = getattr(item, field) + if isinstance(field_value, list): + field_value = "; ".join(field_value) + info_lines.append(f"{tagtype}: {field_value}") return info_lines @@ -1351,7 +1354,7 @@ class Server(BaseServer): "AlbumArtist": "albumartist", "AlbumArtistSort": "albumartist_sort", "Label": "label", - "Genre": "genre", + "Genre": "genres", "Date": "year", "OriginalDate": "original_year", "Composer": "composer", diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index dc88e0f14..b33af83a2 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -352,13 +352,11 @@ class DiscogsPlugin(MetadataSourcePlugin): mediums = [t["medium"] for t in tracks] country = result.data.get("country") data_url = result.data.get("uri") - style = self.format(result.data.get("styles")) - base_genre = self.format(result.data.get("genres")) + styles: list[str] = result.data.get("styles") or [] + genres: list[str] = result.data.get("genres") or [] - if self.config["append_style_genre"] and style: - genre = self.config["separator"].as_str().join([base_genre, style]) - else: - genre = base_genre + if self.config["append_style_genre"]: + genres.extend(styles) discogs_albumid = self._extract_id(result.data.get("uri")) @@ -412,8 +410,10 @@ class DiscogsPlugin(MetadataSourcePlugin): releasegroup_id=master_id, catalognum=catalogno, country=country, - style=style, - genre=genre, + style=( + self.config["separator"].as_str().join(sorted(styles)) or None + ), + genres=sorted(genres), media=media, original_year=original_year, data_source=self.data_source, @@ -434,14 +434,6 @@ class DiscogsPlugin(MetadataSourcePlugin): return None - def format(self, classification: Iterable[str]) -> str | None: - if classification: - return ( - self.config["separator"].as_str().join(sorted(classification)) - ) - else: - return None - def get_tracks( self, tracklist: list[Track], diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 46e756122..0f358c9a1 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -30,7 +30,12 @@ from beets.util import PromptChoice # These "safe" types can avoid the format/parse cycle that most fields go # through: they are safe to edit with native YAML types. -SAFE_TYPES = (types.BaseFloat, types.BaseInteger, types.Boolean) +SAFE_TYPES = ( + types.BaseFloat, + types.BaseInteger, + types.Boolean, + types.DelimitedString, +) class ParseError(Exception): diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index ef311cbbd..e4de9181b 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -288,7 +288,8 @@ class Candidate: elif check == ImageAction.REFORMAT: self.path = ArtResizer.shared.reformat( self.path, - plugin.cover_format, + # TODO: fix this gnarly logic to remove the need for type ignore + plugin.cover_format, # type: ignore[arg-type] deinterlaced=plugin.deinterlace, ) @@ -1367,7 +1368,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # allow both pixel and percentage-based margin specifications self.enforce_ratio = self.config["enforce_ratio"].get( - confuse.OneOf( + confuse.OneOf[bool | str]( [ bool, confuse.String(pattern=self.PAT_PX), diff --git a/beetsplug/fish.py b/beetsplug/fish.py index 82e035eb4..9de764656 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -16,10 +16,10 @@ """This plugin generates tab completions for Beets commands for the Fish shell , including completions for Beets commands, plugin commands, and option flags. Also generated are completions for all the album -and track fields, suggesting for example `genre:` or `album:` when querying the +and track fields, suggesting for example `genres:` or `album:` when querying the Beets database. Completions for the *values* of those fields are not generated by default but can be added via the `-e` / `--extravalues` flag. For example: -`beet fish -e genre -e albumartist` +`beet fish -e genres -e albumartist` """ import os diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index fde7ff92a..362bab523 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -71,6 +71,12 @@ def split_on_feat( if len(parts) == 2: return parts + # Try comma as separator + # (e.g. "Alice, Bob & Charlie" where Bob and Charlie are featuring) + if for_artist and "," in artist: + comma_parts = artist.split(",", 1) + return comma_parts[0].strip(), comma_parts[1].strip() + # Fall back to all tokens including generic separators if no explicit match if for_artist: regex = re.compile( diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 1c91688a6..41927a87c 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -39,8 +39,9 @@ from beets.util import plurality, unique_list if TYPE_CHECKING: import optparse - from collections.abc import Callable + from collections.abc import Callable, Iterable + from beets.importer import ImportSession, ImportTask from beets.library import LibModel LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY) @@ -110,7 +111,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): "force": False, "keep_existing": False, "auto": True, - "separator": ", ", "prefer_specific": False, "title_case": True, "pretend": False, @@ -178,14 +178,13 @@ class LastGenrePlugin(plugins.BeetsPlugin): """A tuple of allowed genre sources. May contain 'track', 'album', or 'artist.' """ - source = self.config["source"].as_choice(("track", "album", "artist")) - if source == "track": - return "track", "album", "artist" - if source == "album": - return "album", "artist" - if source == "artist": - return ("artist",) - return tuple() + return self.config["source"].as_choice( + { + "track": ("track", "album", "artist"), + "album": ("album", "artist"), + "artist": ("artist",), + } + ) # More canonicalization and general helpers. @@ -213,7 +212,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): - Returns an empty list if the input tags list is empty. - If canonicalization is enabled, it extends the list by incorporating parent genres from the canonicalization tree. When a whitelist is set, - only parent tags that pass a validity check (_is_valid) are included; + only parent tags that pass the whitelist filter are included; otherwise, it adds the oldest ancestor. Adding parent tags is stopped when the count of tags reaches the configured limit (count). - The tags list is then deduplicated to ensure only unique genres are @@ -237,11 +236,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Add parents that are in the whitelist, or add the oldest # ancestor if no whitelist if self.whitelist: - parents = [ - x - for x in find_parents(tag, self.c14n_branches) - if self._is_valid(x) - ] + parents = self._filter_valid( + find_parents(tag, self.c14n_branches) + ) else: parents = [find_parents(tag, self.c14n_branches)[-1]] @@ -263,7 +260,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # c14n only adds allowed genres but we may have had forbidden genres in # the original tags list - valid_tags = [t for t in tags if self._is_valid(t)] + valid_tags = self._filter_valid(tags) return valid_tags[:count] def fetch_genre( @@ -275,15 +272,19 @@ class LastGenrePlugin(plugins.BeetsPlugin): min_weight = self.config["min_weight"].get(int) return self._tags_for(lastfm_obj, min_weight) - def _is_valid(self, genre: str) -> bool: - """Check if the genre is valid. + def _filter_valid(self, genres: Iterable[str]) -> list[str]: + """Filter genres based on whitelist. - Depending on the whitelist property, valid means a genre is in the - whitelist or any genre is allowed. + Returns all genres if no whitelist is configured, otherwise returns + only genres that are in the whitelist. """ - if genre and (not self.whitelist or genre.lower() in self.whitelist): - return True - return False + # First, drop any falsy or whitespace-only genre strings to avoid + # retaining empty tags from multi-valued fields. + cleaned = [g for g in genres if g and g.strip()] + if not self.whitelist: + return cleaned + + return [g for g in cleaned if g.lower() in self.whitelist] # Cached last.fm entity lookups. @@ -329,26 +330,21 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Main processing: _get_genre() and helpers. - def _format_and_stringify(self, tags: list[str]) -> str: - """Format to title_case if configured and return as delimited string.""" + def _format_genres(self, tags: list[str]) -> list[str]: + """Format to title case if configured.""" if self.config["title_case"]: - formatted = [tag.title() for tag in tags] + return [tag.title() for tag in tags] else: - formatted = tags - - return self.config["separator"].as_str().join(formatted) + return tags def _get_existing_genres(self, obj: LibModel) -> list[str]: - """Return a list of genres for this Item or Album. Empty string genres - are removed.""" - separator = self.config["separator"].get() + """Return a list of genres for this Item or Album.""" if isinstance(obj, library.Item): - item_genre = obj.get("genre", with_album=False).split(separator) + genres_list = obj.get("genres", with_album=False) else: - item_genre = obj.get("genre").split(separator) + genres_list = obj.get("genres") - # Filter out empty strings - return [g for g in item_genre if g] + return genres_list def _combine_resolve_and_log( self, old: list[str], new: list[str] @@ -359,8 +355,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): combined = old + new return self._resolve_genres(combined) - def _get_genre(self, obj: LibModel) -> tuple[str | None, ...]: - """Get the final genre string for an Album or Item object. + def _get_genre(self, obj: LibModel) -> tuple[list[str], str]: + """Get the final genre list for an Album or Item object. `self.sources` specifies allowed genre sources. Starting with the first source in this tuple, the following stages run through until a genre is @@ -370,9 +366,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): - artist, albumartist or "most popular track genre" (for VA-albums) - original fallback - configured fallback - - None + - empty list - A `(genre, label)` pair is returned, where `label` is a string used for + A `(genres, label)` pair is returned, where `label` is a string used for logging. For example, "keep + artist, whitelist" indicates that existing genres were combined with new last.fm genres and whitelist filtering was applied, while "artist, any" means only new last.fm genres are included @@ -381,7 +377,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): def _try_resolve_stage( stage_label: str, keep_genres: list[str], new_genres: list[str] - ) -> tuple[str, str] | None: + ) -> tuple[list[str], str] | None: """Try to resolve genres for a given stage and log the result.""" resolved_genres = self._combine_resolve_and_log( keep_genres, new_genres @@ -391,7 +387,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): label = f"{stage_label}, {suffix}" if keep_genres: label = f"keep + {label}" - return self._format_and_stringify(resolved_genres), label + return self._format_genres(resolved_genres), label return None keep_genres = [] @@ -400,10 +396,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): if genres and not self.config["force"]: # Without force pre-populated tags are returned as-is. - label = "keep any, no-force" - if isinstance(obj, library.Item): - return obj.get("genre", with_album=False), label - return obj.get("genre"), label + return genres, "keep any, no-force" if self.config["force"]: # Force doesn't keep any unless keep_existing is set. @@ -479,33 +472,32 @@ class LastGenrePlugin(plugins.BeetsPlugin): return result # Nothing found, leave original if configured and valid. - if obj.genre and self.config["keep_existing"]: - if not self.whitelist or self._is_valid(obj.genre.lower()): - return obj.genre, "original fallback" - else: - # If the original genre doesn't match a whitelisted genre, check - # if we can canonicalize it to find a matching, whitelisted genre! - if result := _try_resolve_stage( - "original fallback", keep_genres, [] - ): - return result + if genres and self.config["keep_existing"]: + if valid_genres := self._filter_valid(genres): + return valid_genres, "original fallback" + # If the original genre doesn't match a whitelisted genre, check + # if we can canonicalize it to find a matching, whitelisted genre! + if result := _try_resolve_stage( + "original fallback", keep_genres, [] + ): + return result - # Return fallback string. + # Return fallback as a list. if fallback := self.config["fallback"].get(): - return fallback, "fallback" + return [fallback], "fallback" # No fallback configured. - return None, "fallback unconfigured" + return [], "fallback unconfigured" # Beets plugin hooks and CLI. def _fetch_and_log_genre(self, obj: LibModel) -> None: """Fetch genre and log it.""" self._log.info(str(obj)) - obj.genre, label = self._get_genre(obj) - self._log.debug("Resolved ({}): {}", label, obj.genre) + obj.genres, label = self._get_genre(obj) + self._log.debug("Resolved ({}): {}", label, obj.genres) - ui.show_model_changes(obj, fields=["genre"], print_obj=False) + ui.show_model_changes(obj, fields=["genres"], print_obj=False) @singledispatchmethod def _process(self, obj: LibModel, write: bool) -> None: @@ -603,10 +595,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] - def imported( - self, session: library.Session, task: library.ImportTask - ) -> None: - self._process(task.album if task.is_album else task.item, write=False) + def imported(self, _: ImportSession, task: ImportTask) -> None: + self._process(task.album if task.is_album else task.item, write=False) # type: ignore[attr-defined] def _tags_for( self, diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 3b626a50b..72df907db 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -358,7 +358,7 @@ class LRCLib(Backend): for group in self.fetch_candidates(artist, title, album, length): candidates = [evaluate_item(item) for item in group] if item := self.pick_best_match(candidates): - lyrics = item.get_text(self.config["synced"]) + lyrics = item.get_text(self.config["synced"].get(bool)) return lyrics, f"{self.GET_URL}/{item.id}" return None diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 9f4581004..090bd617a 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -60,34 +60,6 @@ FIELDS_TO_MB_KEYS = { "alias": "alias", } - -RELEASE_INCLUDES = [ - "artists", - "media", - "recordings", - "release-groups", - "labels", - "artist-credits", - "aliases", - "recording-level-rels", - "work-rels", - "work-level-rels", - "artist-rels", - "isrcs", - "url-rels", - "release-rels", - "genres", - "tags", -] - -TRACK_INCLUDES = [ - "artists", - "aliases", - "isrcs", - "work-level-rels", - "artist-rels", -] - BROWSE_INCLUDES = [ "artist-credits", "work-rels", @@ -672,10 +644,13 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - info.genre = "; ".join( - genre - for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) - ) + if genres: + info.genres = [ + genre + for genre, _count in sorted( + genres.items(), key=lambda g: -g[1] + ) + ] # We might find links to external sources (Discogs, Bandcamp, ...) external_ids = self.config["external_ids"].get() @@ -797,7 +772,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): # A 404 error here is fine. e.g. re-importing a release that has # been deleted on MusicBrainz. try: - res = self.mb_api.get_release(albumid, includes=RELEASE_INCLUDES) + res = self.mb_api.get_release(albumid) except HTTPNotFoundError: self._log.debug("Release {} not found on MusicBrainz.", albumid) return None @@ -813,9 +788,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): rel["type"] == "transl-tracklisting" and rel["direction"] == "backward" ): - actual_res = self.mb_api.get_release( - rel["release"]["id"], includes=RELEASE_INCLUDES - ) + actual_res = self.mb_api.get_release(rel["release"]["id"]) # release is potentially a pseudo release release = self.album_info(res) @@ -838,8 +811,6 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): return None with suppress(HTTPNotFoundError): - return self.track_info( - self.mb_api.get_recording(trackid, includes=TRACK_INCLUDES) - ) + return self.track_info(self.mb_api.get_recording(trackid)) return None diff --git a/beetsplug/playlist.py b/beetsplug/playlist.py index a1f9fff39..54a03646f 100644 --- a/beetsplug/playlist.py +++ b/beetsplug/playlist.py @@ -69,7 +69,7 @@ class PlaylistQuery(InQuery[bytes]): relative_to = os.path.dirname(playlist_path) else: relative_to = config["relative_to"].as_filename() - relative_to = beets.util.bytestring_path(relative_to) + relative_to_bytes = beets.util.bytestring_path(relative_to) for line in f: if line[0] == "#": @@ -78,7 +78,7 @@ class PlaylistQuery(InQuery[bytes]): paths.append( beets.util.normpath( - os.path.join(relative_to, line.rstrip()) + os.path.join(relative_to_bytes, line.rstrip()) ) ) f.close() diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e83345059..e69f6b2ee 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -34,7 +34,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin -from beets.util import command_output, displayable_path, syspath +from beets.util import command_output, syspath if TYPE_CHECKING: import optparse @@ -425,7 +425,7 @@ class FfmpegBackend(Backend): # call ffmpeg self._log.debug("analyzing {}", item) cmd = self._construct_cmd(item, peak_method) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stderr.splitlines() # parse output @@ -643,18 +643,20 @@ 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[str] = [self.command, "-o", "-s", "s"] - if self.noclip: - # Adjust to avoid clipping. - cmd = [*cmd, "-k"] - else: - # Disable clipping warning. - cmd = [*cmd, "-c"] - cmd = [*cmd, "-d", str(int(target_level - 89))] - cmd = cmd + [syspath(i.path) for i in items] + cmd = [ + self.command, + "-o", + "-s", + "s", + # Avoid clipping or disable clipping warning + "-k" if self.noclip else "-c", + "-d", + str(int(target_level - 89)), + *[str(i.filepath) for i in items], + ] self._log.debug("analyzing {} files", len(items)) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stdout self._log.debug("analysis finished") return self.parse_tool_output( diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index e22a65787..ff5e25612 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -262,8 +262,9 @@ class SmartPlaylistPlugin(BeetsPlugin): "Updating {} smart playlists...", len(self._matched_playlists) ) - playlist_dir = self.config["playlist_dir"].as_filename() - playlist_dir = bytestring_path(playlist_dir) + playlist_dir = bytestring_path( + self.config["playlist_dir"].as_filename() + ) tpl = self.config["uri_format"].get() prefix = bytestring_path(self.config["prefix"].as_str()) relative_to = self.config["relative_to"].get() @@ -358,8 +359,8 @@ class SmartPlaylistPlugin(BeetsPlugin): if extm3u: attr = [(k, entry.item[k]) for k in keys] al = [ - f' {key}="{quote(str(value), safe="/:")}"' - for key, value in attr + f' {k}="{quote("; ".join(v) if isinstance(v, list) else str(v), safe="/:")}"' # noqa: E501 + for k, v in attr ] attrs = "".join(al) comment = ( diff --git a/beetsplug/titlecase.py b/beetsplug/titlecase.py index d722d4d16..634f5fe4d 100644 --- a/beetsplug/titlecase.py +++ b/beetsplug/titlecase.py @@ -104,7 +104,7 @@ class TitlecasePlugin(BeetsPlugin): @cached_property def replace(self) -> list[tuple[str, str]]: - return self.config["replace"].as_pairs() + return self.config["replace"].as_pairs(default_value="") @cached_property def the_artist(self) -> bool: diff --git a/docs/changelog.rst b/docs/changelog.rst index c8f8b2d72..fc1532cad 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,66 @@ below! Unreleased ---------- +New features +~~~~~~~~~~~~ + +- Add native support for multiple genres per album/track. The ``genres`` field + now stores genres as a list and is written to files as multiple individual + genre tags (e.g., separate GENRE tags for FLAC/MP3). The + :doc:`plugins/musicbrainz`, :doc:`plugins/beatport`, :doc:`plugins/discogs` + and :doc:`plugins/lastgenre` plugins have been updated to populate the + ``genres`` field as a list. + + **Migration**: Existing libraries with comma-separated, semicolon-separated, + or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) are + automatically migrated to the ``genres`` list when you first run beets after + upgrading. The migration runs once when the database schema is updated, + splitting genre strings and writing the changes to the database. The updated + ``genres`` values will be written to media files the next time you run a + command that writes tags (such as ``beet write`` or during import). No manual + action or ``mbsync`` is required. + + The ``genre`` field is split by the first separator found in the string, in + the following order of precedence: + + 1. :doc:`plugins/lastgenre` ``separator`` configuration + 2. Semicolon followed by a space + 3. Comma followed by a space + 4. Slash wrapped by spaces + +Bug fixes +~~~~~~~~~ + +- :doc:`plugins/ftintitle`: Fix handling of multiple featured artists with + ampersand. + +For plugin developers +~~~~~~~~~~~~~~~~~~~~~ + +- If you maintain a metadata source plugin that populates the ``genre`` field, + please update it to populate a list of ``genres`` instead. You will see a + deprecation warning for now, but support for populating the single ``genre`` + field will be removed in version ``3.0.0``. + +Other changes +~~~~~~~~~~~~~ + +- :ref:`modify-cmd`: Use the following separator to delimit multiple field + values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``. + Previously, ``\␀`` was used as a separator. This applies to fields such as + ``artists``, ``albumtypes`` etc. +- Improve highlighting of multi-valued fields changes. +- :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally, + with list values handled directly to make metadata edits smoother and more + predictable. +- :doc:`plugins/lastgenre`: The ``separator`` configuration option is removed. + Since genres are now stored as a list in the ``genres`` field and written to + files as individual genre tags, this option has no effect and has been + removed. + +2.6.2 (February 22, 2026) +------------------------- + .. New features ~~~~~~~~~~~~ @@ -24,6 +84,10 @@ Bug fixes - :doc:`plugins/musicbrainz`: Fix fetching very large releases that have more than 500 tracks. :bug:`6355` - :doc:`plugins/badfiles`: Fix number of found errors in log message +- :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command + backends, such as ``mp3gain``. :bug:`2946` +- :doc:`plugins/mbpseudo`: Fix crash due to missing ``artist_credit`` field in + the MusicBrainz API response. :bug:`6339` .. For plugin developers diff --git a/docs/conf.py b/docs/conf.py index 15ba699cd..8a812d159 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -19,7 +19,7 @@ copyright = "2016, Adrian Sampson" master_doc = "index" language = "en" version = "2.6" -release = "2.6.1" +release = "2.6.2" # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration @@ -98,7 +98,7 @@ man_pages = [ ] # Global substitutions that can be used anywhere in the documentation. -rst_epilog = """ +rst_epilog = r""" .. |Album| replace:: :class:`~beets.library.models.Album` .. |AlbumInfo| replace:: :class:`beets.autotag.hooks.AlbumInfo` .. |BeetsPlugin| replace:: :class:`beets.plugins.BeetsPlugin` @@ -108,6 +108,7 @@ rst_epilog = """ .. |Library| replace:: :class:`~beets.library.library.Library` .. |Model| replace:: :class:`~beets.dbcore.db.Model` .. |TrackInfo| replace:: :class:`beets.autotag.hooks.TrackInfo` +.. |semicolon_space| replace:: :literal:`; \ ` """ # -- Options for HTML output ------------------------------------------------- diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 780042026..3734b57e7 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -116,17 +116,21 @@ Default .. conf:: append_style_genre :default: no - Appends the Discogs style (if found) to the genre tag. This can be useful if - you want more granular genres to categorize your music. For example, - a release in Discogs might have a genre of "Electronic" and a style of - "Techno": enabling this setting would set the genre to be "Electronic, - Techno" (assuming default separator of ``", "``) instead of just - "Electronic". + Appends the Discogs style (if found) to the ``genres`` tag. This can be + useful if you want more granular genres to categorize your music. For + example, a release in Discogs might have a genre of "Electronic" and a style + of "Techno": enabling this setting would append "Techno" to the ``genres`` + list. .. conf:: separator :default: ", " - How to join multiple genre and style values from Discogs into a string. + How to join multiple style values from Discogs into a string. + + .. versionchanged:: 2.7.0 + + This option now only applies to the ``style`` field as beets now only + handles lists of ``genres``. .. conf:: strip_disambiguation :default: yes diff --git a/docs/plugins/fish.rst b/docs/plugins/fish.rst index c1ae4f990..a26b06458 100644 --- a/docs/plugins/fish.rst +++ b/docs/plugins/fish.rst @@ -28,7 +28,7 @@ option flags available to you, which also applies to subcommands such as ``beet import -``. If you type ``beet ls`` followed by a space and then the and the ``TAB`` key, you will see a list of all the album/track fields that can be used in beets queries. For example, typing ``beet ls ge`` will complete to -``genre:`` and leave you ready to type the rest of your query. +``genres:`` and leave you ready to type the rest of your query. Options ------- @@ -42,7 +42,7 @@ commands and option flags. If you want generated completions to also contain album/track field *values* for the items in your library, you can use the ``-e`` or ``--extravalues`` option. For example: ``beet fish -e genre`` or ``beet fish -e genre -e albumartist`` In -the latter case, subsequently typing ``beet list genre: `` will display a +the latter case, subsequently typing ``beet list genres: `` will display a list of all the genres in your library and ``beet list albumartist: `` will show a list of the album artists in your library. Keep in mind that all of these values will be put into the generated completions file, so use this option with diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index 47e679dbd..6bb76d796 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -26,12 +26,12 @@ Here's an example: ihate: warn: - artist:rnb - - genre:soul + - genres:soul # Only warn about tribute albums in rock genre. - - genre:rock album:tribute + - genres:rock album:tribute skip: - - genre::russian\srock - - genre:polka + - genres::russian\srock + - genres:polka - artist:manowar - album:christmas diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 0a461b857..bb89dde3f 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -481,6 +481,10 @@ beets-filetote_ Helps bring non-music extra files, attachments, and artifacts during imports and CLI file manipulation actions (``beet move``, etc.). +beets-fillmissing_ + Interactively prompts you to fill in missing or incomplete metadata fields + for music tracks. + beets-follow_ Lets you check for new albums from artists you like. @@ -596,6 +600,8 @@ beets-youtube_ .. _beets-filetote: https://github.com/gtronset/beets-filetote +.. _beets-fillmissing: https://github.com/amiv1/beets-fillmissing + .. _beets-follow: https://github.com/nolsto/beets-follow .. _beets-goingrunning: https://pypi.org/project/beets-goingrunning diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index ace7caaf0..fa68ce9db 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -90,9 +90,8 @@ By default, the plugin chooses the most popular tag on Last.fm as a genre. If you prefer to use a *list* of popular genre tags, you can increase the number of the ``count`` config option. -Lists of up to *count* genres will then be used instead of single genres. The -genres are separated by commas by default, but you can change this with the -``separator`` config option. +Lists of up to *count* genres will be stored in the ``genres`` field as a list +and written to your media files as separate genre tags. Last.fm_ provides a popularity factor, a.k.a. *weight*, for each tag ranging from 100 for the most popular tag down to 0 for the least popular. The plugin @@ -192,7 +191,6 @@ file. The available options are: Default: ``no``. - **source**: Which entity to look up in Last.fm. Can be either ``artist``, ``album`` or ``track``. Default: ``album``. -- **separator**: A separator for multiple genres. Default: ``', '``. - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. - **title_case**: Convert the new tags to TitleCase before saving. Default: diff --git a/docs/plugins/lyrics.rst b/docs/plugins/lyrics.rst index 9cc63a8b7..33aa9b61e 100644 --- a/docs/plugins/lyrics.rst +++ b/docs/plugins/lyrics.rst @@ -174,6 +174,9 @@ We use Azure to optionally translate your lyrics. To set up the integration, follow these steps: 1. `Create a Translator resource`_ on Azure. + Make sure the region of the translator resource is set to Global. You + will get 401 unauthorized errors if not. The region of the resource group + does not matter. 2. `Obtain its API key`_. 3. Add the API key to your configuration as ``translate.api_key``. 4. Configure your target language using the ``translate.to_language`` option. diff --git a/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst index f227559a8..48060ea79 100644 --- a/docs/plugins/smartplaylist.rst +++ b/docs/plugins/smartplaylist.rst @@ -121,7 +121,7 @@ instance the following configuration exports the ``id`` and ``genre`` fields: output: extm3u fields: - id - - genre + - genres playlists: - name: all.m3u query: '' @@ -132,7 +132,7 @@ look as follows: :: #EXTM3U - #EXTINF:805 id="1931" genre="Progressive%20Rock",Led Zeppelin - Stairway to Heaven + #EXTINF:805 id="1931" genres="Rock%3B%20Pop",Led Zeppelin - Stairway to Heaven ../music/singles/Led Zeppelin/Stairway to Heaven.mp3 To give a usage example, the webm3u_ and Beetstream_ plugins read the exported diff --git a/docs/plugins/zero.rst b/docs/plugins/zero.rst index 2f5b0796c..fed048330 100644 --- a/docs/plugins/zero.rst +++ b/docs/plugins/zero.rst @@ -44,7 +44,6 @@ For example: zero: fields: month day genre genres comments comments: [EAC, LAME, from.+collection, 'ripped by'] - genre: [rnb, 'power metal'] genres: [rnb, 'power metal'] update_database: true diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index c0274553a..6f60d2232 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -143,9 +143,9 @@ Optional command flags: :ref:`set_fields` configuration dictionary. You can use the option multiple times on the command line, like so: - :: +.. code-block:: sh - beet import --set genre="Alternative Rock" --set mood="emotional" + beet import --set genres="Alternative Rock" --set mood="emotional" .. _py7zr: https://pypi.org/project/py7zr/ @@ -267,6 +267,9 @@ Values can also be *templates*, using the same syntax as :doc:`path formats artist sort name into the artist field for all your tracks, and ``beet modify title='$track $title'`` will add track numbers to their title metadata. +To adjust a multi-valued field, such as ``genres``, separate the values with +|semicolon_space|. For example, ``beet modify genres="rock; pop"``. + The ``-a`` option changes to querying album fields instead of track fields and also enables to operate on albums in addition to the individual tracks. Without this flag, the command will only change *track-level* data, even if all the diff --git a/docs/reference/config.rst b/docs/reference/config.rst index b654c118f..4326a80a1 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -853,11 +853,11 @@ set_fields A dictionary indicating fields to set to values for newly imported music. Here's an example: -:: +.. code-block:: yaml set_fields: - genre: 'To Listen' - collection: 'Unordered' + genres: To Listen + collection: Unordered Other field/value pairs supplied via the ``--set`` option on the command-line override any settings here for fields with the same name. @@ -913,6 +913,55 @@ and the next-best match is above the *gap* threshold, the importer will suggest that match but not automatically confirm it. Otherwise, you'll see a list of options to choose from. +.. _distance-weights: + +distance_weights +~~~~~~~~~~~~~~~~ + +The ``distance_weights`` option allows you to customize how much each field +contributes to the overall distance score when matching albums and tracks. +Higher weights mean that differences in that field are penalized more heavily, +making them more important in the matching decision. + +The defaults are: + +.. code-block:: yaml + + match: + distance_weights: + data_source: 2.0 + artist: 3.0 + album: 3.0 + media: 1.0 + mediums: 1.0 + year: 1.0 + country: 0.5 + label: 0.5 + catalognum: 0.5 + albumdisambig: 0.5 + album_id: 5.0 + tracks: 2.0 + missing_tracks: 0.9 + unmatched_tracks: 0.6 + track_title: 3.0 + track_artist: 2.0 + track_index: 1.0 + track_length: 2.0 + track_id: 5.0 + medium: 1.0 + +For example, if you don't care as much about matching the exact release year, +you can reduce its weight: + +.. code-block:: yaml + + match: + distance_weights: + year: 0.1 + +You only need to specify the fields you want to override; unspecified fields +keep their default weights. + .. _max_rec: max_rec @@ -1172,9 +1221,9 @@ Here's an example file: color: yes paths: - default: $genre/$albumartist/$album/$track $title + default: %first{$genres}/$albumartist/$album/$track $title singleton: Singletons/$artist - $title - comp: $genre/$album/$track $title + comp: %first{$genres}/$album/$track $title albumtype:soundtrack: Soundtracks/$album/$track $title .. only:: man diff --git a/docs/reference/pathformat.rst b/docs/reference/pathformat.rst index 10dd3ae05..aff48a7c6 100644 --- a/docs/reference/pathformat.rst +++ b/docs/reference/pathformat.rst @@ -75,11 +75,34 @@ These functions are built in to beets: - ``%time{date_time,format}``: Return the date and time in any format accepted by strftime_. For example, to get the year some music was added to your library, use ``%time{$added,%Y}``. -- ``%first{text}``: Returns the first item, separated by ``;`` (a semicolon - followed by a space). You can use ``%first{text,count,skip}``, where ``count`` - is the number of items (default 1) and ``skip`` is number to skip (default 0). - You can also use ``%first{text,count,skip,sep,join}`` where ``sep`` is the - separator, like ``;`` or ``/`` and join is the text to concatenate the items. +- ``%first{text,count,skip,sep,join}``: Extract a subset of items from a + delimited string. Splits ``text`` by ``sep``, skips the first ``skip`` items, + then returns the next ``count`` items joined by ``join``. + + This is especially useful for multi-valued fields like ``artists`` or + ``genres`` where you may only want the first artist or a limited number of + genres in a path. + + Defaults: + + .. + Comically, you need to follow |semicolon_space| with some punctuation to + make sure it gets rendered correctly as '; ' in the docs. + + - **count**: 1, + - **skip**: 0, + - **sep**: |semicolon_space|, + - **join**: |semicolon_space|. + + Examples: + + :: + + %first{$genres} → returns the first genre + %first{$genres,2} → returns the first two genres, joined by "; " + %first{$genres,2,1} → skips the first genre, returns the next two + %first{$genres,2,0, , -> } → splits by space, joins with " -> " + - ``%ifdef{field}``, ``%ifdef{field,truetext}`` or ``%ifdef{field,truetext,falsetext}``: Checks if an flexible attribute ``field`` is defined. If it exists, then return ``truetext`` or ``field`` diff --git a/docs/team.rst b/docs/team.rst index b23b125ce..f99fcae97 100644 --- a/docs/team.rst +++ b/docs/team.rst @@ -42,12 +42,11 @@ of what you can expect from these *knowledge owners*. ----- - The Discogs plugin -- Other metadata source plugins -- Generalization of source plugin logic (The MetaDataSourcePlugin abstract - class) - Good documentation throughout the project - The smartplaylist plugin -- Things around m3u and other playlist formats +- The lastgenre plugin +- Support for M3U and other playlist formats +- beets as a DJ companion tool (BPM, audio features, key) @RollingStar ------------ @@ -74,6 +73,17 @@ of what you can expect from these *knowledge owners*. - Code quality - Typing +@snejus +------- + +- Grug-minded approach: simple, obvious solutions over clever complexity +- MusicBrainz/autotagger internals and source-plugin behavior +- Query/path handling and SQL-backed lookup behavior +- Typing and tooling modernization (mypy, Ruff, annotations) +- Test architecture, CI reliability, and coverage improvements +- Release and packaging workflows (Poetry/pyproject, dependencies, changelog) +- Cross-plugin refactors (especially metadata and lyrics-related internals) + @wisp3rwind ----------- diff --git a/poetry.lock b/poetry.lock index 1a0515819..023ea7718 100644 --- a/poetry.lock +++ b/poetry.lock @@ -722,18 +722,21 @@ files = [ [[package]] name = "confuse" -version = "2.1.0" +version = "2.2.0" description = "Painless YAML config files" optional = false -python-versions = ">=3.9" +python-versions = ">=3.10" files = [ - {file = "confuse-2.1.0-py3-none-any.whl", hash = "sha256:502be1299aa6bf7c48f7719f56795720c073fb28550c0c7a37394366c9d30316"}, - {file = "confuse-2.1.0.tar.gz", hash = "sha256:abb9674a99c7a6efaef84e2fc84403ecd2dd304503073ff76ea18ed4176e218d"}, + {file = "confuse-2.2.0-py3-none-any.whl", hash = "sha256:470c6aa1a5008c8d740267f2ad574e3a715b6dd873c1e5f8778b7f7abb954722"}, + {file = "confuse-2.2.0.tar.gz", hash = "sha256:35c1b53e81be125f441bee535130559c935917b26aeaa61289010cd1f55c2b9e"}, ] [package.dependencies] pyyaml = "*" +[package.extras] +docs = ["sphinx (>=7.4.7)", "sphinx-rtd-theme (>=3.0.2)"] + [[package]] name = "coverage" version = "7.11.0" @@ -4558,4 +4561,4 @@ web = ["flask", "flask-cors"] [metadata] lock-version = "2.0" python-versions = ">=3.10,<4" -content-hash = "eefe427d3b3b9b871ca6bcd8405e3578a16d660afd7925c14793514f03c96ac6" +content-hash = "9cff39f63616b2654fbf44b006f7eedcae6c1846fbb9f04af82483891b7d77b9" diff --git a/pyproject.toml b/pyproject.toml index 2a0a1904d..0ce774d9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "beets" -version = "2.6.1" +version = "2.6.2" description = "music tagger and library organizer" authors = ["Adrian Sampson "] maintainers = ["Serene-Arc"] @@ -44,7 +44,7 @@ Changelog = "https://github.com/beetbox/beets/blob/master/docs/changelog.rst" python = ">=3.10,<4" colorama = { version = "*", markers = "sys_platform == 'win32'" } -confuse = ">=2.1.0" +confuse = ">=2.2.0" jellyfish = "*" lap = ">=0.5.12" mediafile = ">=0.12.0" diff --git a/test/autotag/test_hooks.py b/test/autotag/test_hooks.py new file mode 100644 index 000000000..e5de089e8 --- /dev/null +++ b/test/autotag/test_hooks.py @@ -0,0 +1,17 @@ +import pytest + +from beets.autotag.hooks import Info + + +@pytest.mark.parametrize( + "genre, expected_genres", + [ + ("Rock", ("Rock",)), + ("Rock; Alternative", ("Rock", "Alternative")), + ], +) +def test_genre_deprecation(genre, expected_genres): + with pytest.warns( + DeprecationWarning, match="The 'genre' parameter is deprecated" + ): + assert tuple(Info(genre=genre).genres) == expected_genres diff --git a/test/conftest.py b/test/conftest.py index 059526d2f..b35083641 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -5,6 +5,7 @@ import pytest from beets.autotag.distance import Distance from beets.dbcore.query import Query +from beets.test._common import DummyIO from beets.test.helper import ConfigMixin from beets.util import cached_classproperty @@ -60,3 +61,24 @@ def clear_cached_classproperty(): def config(): """Provide a fresh beets configuration for a module, when requested.""" return ConfigMixin().config + + +@pytest.fixture +def io( + request: pytest.FixtureRequest, + monkeypatch: pytest.MonkeyPatch, + capteesys: pytest.CaptureFixture[str], +) -> DummyIO: + """Fixture for tests that need controllable stdin and captured stdout. + + This fixture builds a per-test ``DummyIO`` helper and exposes it to the + test. When used on a test class, it attaches the helper as ``self.io`` + attribute to make it available to all test methods, including + ``unittest.TestCase``-based ones. + """ + io = DummyIO(monkeypatch, capteesys) + + if request.instance: + request.instance.io = io + + return io diff --git a/test/library/__init__.py b/test/library/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test/library/test_migrations.py b/test/library/test_migrations.py new file mode 100644 index 000000000..2c0dece8b --- /dev/null +++ b/test/library/test_migrations.py @@ -0,0 +1,72 @@ +import pytest + +from beets.dbcore import types +from beets.library.migrations import MultiGenreFieldMigration +from beets.library.models import Album, Item +from beets.test.helper import TestHelper + + +class TestMultiGenreFieldMigration: + @pytest.fixture + def helper(self, monkeypatch): + # do not apply migrations upon library initialization + monkeypatch.setattr("beets.library.library.Library._migrations", ()) + # add genre field to both models to make sure this column is created + monkeypatch.setattr( + "beets.library.models.Item._fields", + {**Item._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album._fields", + {**Album._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album.item_keys", + {*Album.item_keys, "genre"}, + ) + helper = TestHelper() + helper.setup_beets() + + # and now configure the migrations to be tested + monkeypatch.setattr( + "beets.library.library.Library._migrations", + ((MultiGenreFieldMigration, (Item, Album)),), + ) + yield helper + + helper.teardown_beets() + + def test_migrates_only_rows_with_missing_genres(self, helper: TestHelper): + helper.config["lastgenre"]["separator"] = " - " + + expected_item_genres = [] + for genre, initial_genres, expected_genres in [ + # already existing value is not overwritten + ("Item Rock", ("Ignored",), ("Ignored",)), + ("", (), ()), + ("Rock", (), ("Rock",)), + # multiple genres are split on one of default separators + ("Item Rock; Alternative", (), ("Item Rock", "Alternative")), + # multiple genres are split the first (lastgenre) separator ONLY + ("Item - Rock, Alternative", (), ("Item", "Rock, Alternative")), + ]: + helper.add_item(genre=genre, genres=initial_genres) + expected_item_genres.append(expected_genres) + + unmigrated_album = helper.add_album( + genre="Album Rock / Alternative", genres=[] + ) + expected_item_genres.append(("Album Rock", "Alternative")) + + helper.lib._migrate() + + actual_item_genres = [tuple(i.genres) for i in helper.lib.items()] + assert actual_item_genres == expected_item_genres + + unmigrated_album.load() + assert unmigrated_album.genres == ["Album Rock", "Alternative"] + + # remove cached initial db tables data + del helper.lib.db_tables + assert helper.lib.migration_exists("multi_genre_field", "items") + assert helper.lib.migration_exists("multi_genre_field", "albums") diff --git a/test/plugins/test_bareasc.py b/test/plugins/test_bareasc.py index e699a3dcf..a661ae7aa 100644 --- a/test/plugins/test_bareasc.py +++ b/test/plugins/test_bareasc.py @@ -4,10 +4,10 @@ """Tests for the 'bareasc' plugin.""" from beets import logging -from beets.test.helper import PluginTestCase, capture_stdout +from beets.test.helper import IOMixin, PluginTestCase -class BareascPluginTest(PluginTestCase): +class BareascPluginTest(IOMixin, PluginTestCase): """Test bare ASCII query matching.""" plugin = "bareasc" @@ -65,16 +65,12 @@ class BareascPluginTest(PluginTestCase): def test_bareasc_list_output(self): """Bare-ASCII version of list command - check output.""" - with capture_stdout() as output: - self.run_command("bareasc", "with accents") + self.run_command("bareasc", "with accents") - assert "Antonin Dvorak" in output.getvalue() + assert "Antonin Dvorak" in self.io.getoutput() def test_bareasc_format_output(self): """Bare-ASCII version of list -f command - check output.""" - with capture_stdout() as output: - self.run_command( - "bareasc", "with accents", "-f", "$artist:: $title" - ) + self.run_command("bareasc", "with accents", "-f", "$artist:: $title") - assert "Antonin Dvorak:: with accents\n" == output.getvalue() + assert "Antonin Dvorak:: with accents\n" == self.io.getoutput() diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index b92a3bf15..96386d8b6 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -474,7 +474,7 @@ class BeatportTest(BeetsTestCase): item.year = 2016 item.comp = False item.label_name = "Gravitas Recordings" - item.genre = "Glitch Hop" + item.genres = ["Glitch Hop", "Breaks"] item.year = 2016 item.month = 4 item.day = 11 @@ -583,7 +583,7 @@ class BeatportTest(BeetsTestCase): def test_genre_applied(self): for track, test_track in zip(self.tracks, self.test_tracks): - assert track.genre == test_track.genre + assert track.genres == test_track.genres class BeatportResponseEmptyTest(unittest.TestCase): @@ -634,7 +634,7 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["subGenres"] = [] - assert tracks[0].genre == self.test_tracks[0]["genres"][0]["name"] + assert tracks[0].genres == [self.test_tracks[0]["genres"][0]["name"]] def test_genre_empty(self): """No 'genre' is provided. Test if 'sub_genre' is applied.""" @@ -643,4 +643,4 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["genres"] = [] - assert tracks[0].genre == self.test_tracks[0]["subGenres"][0]["name"] + assert tracks[0].genres == [self.test_tracks[0]["subGenres"][0]["name"]] diff --git a/test/plugins/test_bpd.py b/test/plugins/test_bpd.py index 157569bbe..81e088067 100644 --- a/test/plugins/test_bpd.py +++ b/test/plugins/test_bpd.py @@ -32,7 +32,7 @@ import yaml from beets.test.helper import PluginTestCase from beets.util import bluelet -bpd = pytest.importorskip("beetsplug.bpd") +bpd = pytest.importorskip("beetsplug.bpd", exc_type=ImportError) class CommandParseTest(unittest.TestCase): diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 2a1a3b94d..13dbea084 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -29,9 +29,9 @@ from beets.test import _common from beets.test.helper import ( AsIsImporterMixin, ImportHelper, + IOMixin, PluginTestCase, capture_log, - control_stdin, ) from beetsplug import convert @@ -66,7 +66,7 @@ class ConvertMixin: return path.read_bytes().endswith(tag.encode("utf-8")) -class ConvertTestCase(ConvertMixin, PluginTestCase): +class ConvertTestCase(IOMixin, ConvertMixin, PluginTestCase): db_on_disk = True plugin = "convert" @@ -157,8 +157,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): } def test_convert(self): - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.converted_mp3, "mp3") def test_convert_with_auto_confirmation(self): @@ -166,22 +166,22 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): assert self.file_endswith(self.converted_mp3, "mp3") def test_reject_confirmation(self): - with control_stdin("n"): - self.run_convert() + self.io.addinput("n") + self.run_convert() assert not self.converted_mp3.exists() def test_convert_keep_new(self): assert os.path.splitext(self.item.path)[1] == b".ogg" - with control_stdin("y"): - self.run_convert("--keep-new") + self.io.addinput("y") + self.run_convert("--keep-new") self.item.load() assert os.path.splitext(self.item.path)[1] == b".mp3" def test_format_option(self): - with control_stdin("y"): - self.run_convert("--format", "opus") + self.io.addinput("y") + self.run_convert("--format", "opus") assert self.file_endswith(self.convert_dest / "converted.ops", "opus") def test_embed_album_art(self): @@ -192,8 +192,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): with open(os.path.join(image_path), "rb") as f: image_data = f.read() - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() mediafile = MediaFile(self.converted_mp3) assert mediafile.images[0].data == image_data @@ -215,26 +215,26 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_no_transcode_when_maxbr_set_high_and_different_formats(self): self.config["convert"]["max_bitrate"] = 5000 - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() 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() + self.io.addinput("y") + self.run_convert() 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() + self.io.addinput("y") + self.run_convert() 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() + self.io.addinput("y") + self.run_convert() assert not self.file_endswith( self.convert_dest / "converted.ogg", "ogg" ) @@ -243,8 +243,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): self.config["convert"]["max_bitrate"] = 5000 self.config["convert"]["format"] = "ogg" - with control_stdin("y"): - self.run_convert("--force") + self.io.addinput("y") + self.run_convert("--force") converted = self.convert_dest / "converted.ogg" assert self.file_endswith(converted, "ogg") @@ -252,21 +252,21 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): 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() + self.io.addinput("y") + self.run_convert() 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() + self.io.addinput("y") + self.run_convert() 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") + self.io.addinput("y") + self.run_convert("--playlist", "playlist.m3u8") assert (self.convert_dest / "playlist.m3u8").exists() def test_playlist_pretend(self): @@ -282,8 +282,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item, "--format", "opus", "--force") + self.io.addinput("y") + self.run_convert_path(item, "--format", "opus", "--force") converted = self.convert_dest / "converted.ops" assert self.file_endswith(converted, "opus") @@ -309,23 +309,23 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): def test_transcode_from_lossless(self): [item] = self.add_item_fixtures(ext="flac") - with control_stdin("y"): - self.run_convert_path(item) + self.io.addinput("y") + self.run_convert_path(item) 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) + self.io.addinput("y") + self.run_convert_path(item) 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) + self.io.addinput("y") + self.run_convert_path(item) converted = self.convert_dest / "converted.ogg" assert not self.file_endswith(converted, "mp3") @@ -336,8 +336,8 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): } [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item, "--format", "opus", "--force") + self.io.addinput("y") + self.run_convert_path(item, "--format", "opus", "--force") converted = self.convert_dest / "converted.ops" assert self.file_endswith(converted, "opus") diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 15d47db6c..cef84e3a9 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -362,7 +362,7 @@ class DGAlbumInfoTest(BeetsTestCase): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre(self): @@ -371,7 +371,7 @@ class DGAlbumInfoTest(BeetsTestCase): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2, STYLE1, STYLE2" + assert d.genres == ["GENRE1", "GENRE2", "STYLE1", "STYLE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre_no_style(self): @@ -381,7 +381,7 @@ class DGAlbumInfoTest(BeetsTestCase): release.data["styles"] = [] d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style is None def test_strip_disambiguation(self): diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 564b2ff1a..06c7cad74 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -17,15 +17,16 @@ from typing import ClassVar from unittest.mock import patch from beets.dbcore.query import TrueQuery +from beets.importer import Action from beets.library import Item from beets.test import _common from beets.test.helper import ( AutotagImportTestCase, AutotagStub, BeetsTestCase, + IOMixin, PluginMixin, TerminalImportMixin, - control_stdin, ) @@ -103,24 +104,26 @@ class EditMixin(PluginMixin): """ m = ModifyFileMocker(**modify_file_args) with patch("beetsplug.edit.edit", side_effect=m.action): - with control_stdin("\n".join(stdin)): - self.importer.run() + for char in stdin: + self.importer.add_choice(char) + self.importer.run() def run_mocked_command(self, modify_file_args={}, stdin=[], args=[]): """Run the edit command, with mocked stdin and yaml writing, and passing `args` to `run_command`.""" m = ModifyFileMocker(**modify_file_args) with patch("beetsplug.edit.edit", side_effect=m.action): - with control_stdin("\n".join(stdin)): - self.run_command("edit", *args) + for char in stdin: + self.io.addinput(char) + self.run_command("edit", *args) @_common.slow_test() @patch("beets.library.Item.write") -class EditCommandTest(EditMixin, BeetsTestCase): +class EditCommandTest(IOMixin, EditMixin, BeetsTestCase): """Black box tests for `beetsplug.edit`. Command line interaction is - simulated using `test.helper.control_stdin()`, and yaml editing via an - external editor is simulated using `ModifyFileMocker`. + simulated using mocked stdin, and yaml editing via an external editor is + simulated using `ModifyFileMocker`. """ ALBUM_COUNT = 1 @@ -412,7 +415,7 @@ class EditDuringImporterNonSingletonTest(EditDuringImporterTestCase): self.run_mocked_interpreter( {}, # 1, Apply changes. - ["1", "a"], + ["1", Action.APPLY], ) # Retag and edit track titles. On retag, the importer will reset items diff --git a/test/plugins/test_export.py b/test/plugins/test_export.py index f37a0d2a7..3c795b2dc 100644 --- a/test/plugins/test_export.py +++ b/test/plugins/test_export.py @@ -19,10 +19,10 @@ import re # used to test csv format from xml.etree import ElementTree from xml.etree.ElementTree import Element -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class ExportPluginTest(PluginTestCase): +class ExportPluginTest(IOMixin, PluginTestCase): plugin = "export" def setUp(self): diff --git a/test/plugins/test_fetchart.py b/test/plugins/test_fetchart.py index 96d882e9a..f347ed66a 100644 --- a/test/plugins/test_fetchart.py +++ b/test/plugins/test_fetchart.py @@ -18,10 +18,10 @@ import os import sys from beets import util -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class FetchartCliTest(PluginTestCase): +class FetchartCliTest(IOMixin, PluginTestCase): plugin = "fetchart" def setUp(self): diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index aff4dda18..2bba41ad0 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -324,6 +324,11 @@ def test_find_feat_part( ("Alice feat. Bob", ("Alice", "Bob")), ("Alice featuring Bob", ("Alice", "Bob")), ("Alice & Bob", ("Alice", "Bob")), + ("Alice, Bob & Charlie", ("Alice", "Bob & Charlie")), + ( + "Alice, Bob & Charlie feat. Xavier", + ("Alice, Bob & Charlie", "Xavier"), + ), ("Alice and Bob", ("Alice", "Bob")), ("Alice With Bob", ("Alice", "Bob")), ("Alice defeat Bob", ("Alice defeat Bob", None)), @@ -454,7 +459,9 @@ def test_custom_words( assert ftintitle.contains_feat(given, custom_words) is expected -def test_album_template_value(): +def test_album_template_value(config): + config["ftintitle"]["custom_words"] = [] + album = Album() album["albumartist"] = "Foo ft. Bar" assert ftintitle._album_artist_no_feat(album) == "Foo" diff --git a/test/plugins/test_ihate.py b/test/plugins/test_ihate.py index f941d566c..b64b8d91d 100644 --- a/test/plugins/test_ihate.py +++ b/test/plugins/test_ihate.py @@ -11,7 +11,7 @@ class IHatePluginTest(unittest.TestCase): def test_hate(self): match_pattern = {} test_item = Item( - genre="TestGenre", album="TestAlbum", artist="TestArtist" + genres=["TestGenre"], album="TestAlbum", artist="TestArtist" ) task = importer.SingletonImportTask(None, test_item) @@ -27,19 +27,19 @@ class IHatePluginTest(unittest.TestCase): assert IHatePlugin.do_i_hate_this(task, match_pattern) # Query is blocked by AND clause. - match_pattern = ["album:notthis genre:testgenre"] + match_pattern = ["album:notthis genres:testgenre"] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Both queries are blocked by AND clause with unmatched condition. match_pattern = [ - "album:notthis genre:testgenre", + "album:notthis genres:testgenre", "artist:testartist album:notthis", ] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Only one query should fire. match_pattern = [ - "album:testalbum genre:testgenre", + "album:testalbum genres:testgenre", "artist:testartist album:notthis", ] assert IHatePlugin.do_i_hate_this(task, match_pattern) diff --git a/test/plugins/test_importsource.py b/test/plugins/test_importsource.py index a4f498181..7306558a1 100644 --- a/test/plugins/test_importsource.py +++ b/test/plugins/test_importsource.py @@ -19,7 +19,7 @@ import os import time from beets import importer, plugins -from beets.test.helper import AutotagImportTestCase, PluginMixin, control_stdin +from beets.test.helper import AutotagImportTestCase, IOMixin, PluginMixin from beets.util import syspath from beetsplug.importsource import ImportSourcePlugin @@ -34,7 +34,7 @@ def preserve_plugin_listeners(): ImportSourcePlugin.listeners = _listeners -class ImportSourceTest(PluginMixin, AutotagImportTestCase): +class ImportSourceTest(IOMixin, PluginMixin, AutotagImportTestCase): plugin = "importsource" preload_plugin = False @@ -50,31 +50,29 @@ class ImportSourceTest(PluginMixin, AutotagImportTestCase): self.all_items = self.lib.albums().get().items() self.item_to_remove = self.all_items[0] - def interact(self, stdin_input: str): - with control_stdin(stdin_input): - self.run_command( - "remove", - f"path:{syspath(self.item_to_remove.path)}", - ) + def interact(self, stdin: list[str]): + for char in stdin: + self.io.addinput(char) + self.run_command("remove", f"path:{syspath(self.item_to_remove.path)}") def test_do_nothing(self): - self.interact("N") + self.interact(["N"]) assert os.path.exists(self.item_to_remove.source_path) def test_remove_single(self): - self.interact("y\nD") + self.interact(["y", "D"]) assert not os.path.exists(self.item_to_remove.source_path) def test_remove_all_from_single(self): - self.interact("y\nR\ny") + self.interact(["y", "R", "y"]) for item in self.all_items: assert not os.path.exists(item.source_path) def test_stop_suggesting(self): - self.interact("y\nS") + self.interact(["y", "S"]) for item in self.all_items: assert os.path.exists(item.source_path) diff --git a/test/plugins/test_info.py b/test/plugins/test_info.py index c1b3fc941..3ad4d0884 100644 --- a/test/plugins/test_info.py +++ b/test/plugins/test_info.py @@ -15,11 +15,11 @@ from mediafile import MediaFile -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase from beets.util import displayable_path -class InfoTest(PluginTestCase): +class InfoTest(IOMixin, PluginTestCase): plugin = "info" def test_path(self): diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 3de43d197..55524d3fc 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -19,11 +19,11 @@ from unittest.mock import Mock, patch import pytest from beets.test import _common -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase from beetsplug import lastgenre -class LastGenrePluginTest(PluginTestCase): +class LastGenrePluginTest(IOMixin, PluginTestCase): plugin = "lastgenre" def setUp(self): @@ -80,13 +80,15 @@ class LastGenrePluginTest(PluginTestCase): self._setup_config(canonical="", whitelist={"rock"}) assert self.plugin._resolve_genres(["delta blues"]) == [] - def test_format_and_stringify(self): - """Format genres list and return them as a separator-delimited string.""" + def test_format_genres(self): + """Format genres list.""" self._setup_config(count=2) - assert ( - self.plugin._format_and_stringify(["jazz", "pop", "rock", "blues"]) - == "Jazz, Pop, Rock, Blues" - ) + assert self.plugin._format_genres(["jazz", "pop", "rock", "blues"]) == [ + "Jazz", + "Pop", + "Rock", + "Blues", + ] def test_count_c14n(self): """Keep the n first genres, after having applied c14n when necessary""" @@ -144,7 +146,7 @@ class LastGenrePluginTest(PluginTestCase): albumartist="Pretend Artist", artist="Pretend Artist", title="Pretend Track", - genre="Original Genre", + genres=["Original Genre"], ) album = self.lib.add_album([item]) @@ -155,10 +157,10 @@ class LastGenrePluginTest(PluginTestCase): with patch("beetsplug.lastgenre.Item.store", unexpected_store): output = self.run_with_output("lastgenre", "--pretend") - assert "Mock Genre" in output + assert "genres:" in output album.load() - assert album.genre == "Original Genre" - assert album.items()[0].genre == "Original Genre" + assert album.genres == ["Original Genre"] + assert album.items()[0].genres == ["Original Genre"] def test_no_duplicate(self): """Remove duplicated genres.""" @@ -204,7 +206,7 @@ class LastGenrePluginTest(PluginTestCase): @pytest.mark.parametrize( "config_values, item_genre, mock_genres, expected_result", [ - # 0 - force and keep whitelisted + # force and keep whitelisted ( { "force": True, @@ -215,13 +217,13 @@ class LastGenrePluginTest(PluginTestCase): "prefer_specific": False, "count": 10, }, - "Blues", + ["Blues"], { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), - # 1 - force and keep whitelisted, unknown original + # force and keep whitelisted, unknown original ( { "force": True, @@ -231,13 +233,13 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), - # 2 - force and keep whitelisted on empty tag + # force and keep whitelisted on empty tag ( { "force": True, @@ -247,13 +249,13 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazz"], }, - ("Jazz", "album, whitelist"), + (["Jazz"], "album, whitelist"), ), - # 3 force and keep, artist configured + # force and keep, artist configured ( { "force": True, @@ -263,14 +265,14 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], "artist": ["Pop"], }, - ("Blues, Pop", "keep + artist, whitelist"), + (["Blues", "Pop"], "keep + artist, whitelist"), ), - # 4 - don't force, disabled whitelist + # don't force, disabled whitelist ( { "force": False, @@ -280,13 +282,13 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "any genre", + ["any genre"], { "album": ["Jazz"], }, - ("any genre", "keep any, no-force"), + (["any genre"], "keep any, no-force"), ), - # 5 - don't force and empty is regular last.fm fetch; no whitelist too + # don't force and empty is regular last.fm fetch; no whitelist too ( { "force": False, @@ -296,13 +298,13 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazzin"], }, - ("Jazzin", "album, any"), + (["Jazzin"], "album, any"), ), - # 6 - fallback to next stages until found + # fallback to next stages until found ( { "force": True, @@ -312,15 +314,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "unknown genre", + ["unknown genre"], { "track": None, "album": None, "artist": ["Jazz"], }, - ("Unknown Genre, Jazz", "keep + artist, any"), + (["Unknown Genre", "Jazz"], "keep + artist, any"), ), - # 7 - Keep the original genre when force and keep_existing are on, and + # Keep the original genre when force and keep_existing are on, and # whitelist is disabled ( { @@ -332,15 +334,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "any existing", + ["any existing"], { "track": None, "album": None, "artist": None, }, - ("any existing", "original fallback"), + (["any existing"], "original fallback"), ), - # 7.1 - Keep the original genre when force and keep_existing are on, and + # Keep the original genre when force and keep_existing are on, and # whitelist is enabled, and genre is valid. ( { @@ -352,15 +354,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "album": None, "artist": None, }, - ("Jazz", "original fallback"), + (["Jazz"], "original fallback"), ), - # 7.2 - Return the configured fallback when force is on but + # Return the configured fallback when force is on but # keep_existing is not. ( { @@ -372,15 +374,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["fallback genre"], "fallback"), ), - # 8 - fallback to fallback if no original + # fallback to fallback if no original ( { "force": True, @@ -391,32 +393,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "track": None, "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["fallback genre"], "fallback"), ), - # 9 - null charachter as separator - ( - { - "force": True, - "keep_existing": True, - "source": "album", - "whitelist": True, - "separator": "\u0000", - "canonical": False, - "prefer_specific": False, - }, - "Blues", - { - "album": ["Jazz"], - }, - ("Blues\u0000Jazz", "keep + album, whitelist"), - ), - # 10 - limit a lot of results + # limit a lot of results ( { "force": True, @@ -426,31 +411,17 @@ class LastGenrePluginTest(PluginTestCase): "count": 5, "canonical": False, "prefer_specific": False, - "separator": ", ", }, - "original unknown, Blues, Rock, Folk, Metal", + ["original unknown", "Blues", "Rock", "Folk", "Metal"], { "album": ["Jazz", "Bebop", "Hardbop"], }, - ("Blues, Rock, Metal, Jazz, Bebop", "keep + album, whitelist"), + ( + ["Blues", "Rock", "Metal", "Jazz", "Bebop"], + "keep + album, whitelist", + ), ), - # 11 - force off does not rely on configured separator - ( - { - "force": False, - "keep_existing": False, - "source": "album", - "whitelist": True, - "count": 2, - "separator": ", ", - }, - "not ; configured | separator", - { - "album": ["Jazz", "Bebop"], - }, - ("not ; configured | separator", "keep any, no-force"), - ), - # 12 - fallback to next stage (artist) if no allowed original present + # fallback to next stage (artist) if no allowed original present # and no album genre were fetched. ( { @@ -462,15 +433,15 @@ class LastGenrePluginTest(PluginTestCase): "canonical": False, "prefer_specific": False, }, - "not whitelisted original", + ["not whitelisted original"], { "track": None, "album": None, "artist": ["Jazz"], }, - ("Jazz", "keep + artist, whitelist"), + (["Jazz"], "keep + artist, whitelist"), ), - # 13 - canonicalization transforms non-whitelisted genres to canonical forms + # canonicalization transforms non-whitelisted genres to canonical forms # # "Acid Techno" is not in the default whitelist, thus gets resolved "up" in the # tree to "Techno" and "Electronic". @@ -484,13 +455,13 @@ class LastGenrePluginTest(PluginTestCase): "prefer_specific": False, "count": 10, }, - "", + [], { "album": ["acid techno"], }, - ("Techno, Electronic", "album, whitelist"), + (["Techno", "Electronic"], "album, whitelist"), ), - # 14 - canonicalization transforms whitelisted genres to canonical forms and + # canonicalization transforms whitelisted genres to canonical forms and # includes originals # # "Detroit Techno" is in the default whitelist, thus it stays and and also gets @@ -507,16 +478,22 @@ class LastGenrePluginTest(PluginTestCase): "count": 10, "extended_debug": True, }, - "detroit techno", + ["detroit techno"], { "album": ["acid house"], }, ( - "Detroit Techno, Techno, Electronic, Acid House, House", + [ + "Detroit Techno", + "Techno", + "Electronic", + "Acid House", + "House", + ], "keep + album, whitelist", ), ), - # 15 - canonicalization transforms non-whitelisted original genres to canonical + # canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -532,16 +509,16 @@ class LastGenrePluginTest(PluginTestCase): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": ["Detroit Techno"], }, ( - "Disco, Electronic, Detroit Techno, Techno", + ["Disco", "Electronic", "Detroit Techno", "Techno"], "keep + album, whitelist", ), ), - # 16 - canonicalization transforms non-whitelisted original genres to canonical + # canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works, **even** when no new genres are found online. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -556,13 +533,13 @@ class LastGenrePluginTest(PluginTestCase): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": [], "artist": [], }, ( - "Disco, Electronic", + ["Disco", "Electronic"], "keep + original fallback, whitelist", ), ), @@ -592,9 +569,9 @@ def test_get_genre(config_values, item_genre, mock_genres, expected_result): # Configure plugin.config.set(config_values) plugin.setup() # Loads default whitelist and canonicalization tree + item = _common.item() - item.genre = item_genre + item.genres = item_genre # Run - res = plugin._get_genre(item) - assert res == expected_result + assert plugin._get_genre(item) == expected_result diff --git a/test/plugins/test_limit.py b/test/plugins/test_limit.py index d77e47ca8..8f227d41e 100644 --- a/test/plugins/test_limit.py +++ b/test/plugins/test_limit.py @@ -13,10 +13,10 @@ """Tests for the 'limit' plugin.""" -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class LimitPluginTest(PluginTestCase): +class LimitPluginTest(IOMixin, PluginTestCase): """Unit tests for LimitPlugin Note: query prefix tests do not work correctly with `run_with_output`. diff --git a/test/plugins/test_mbsubmit.py b/test/plugins/test_mbsubmit.py index 712c90866..48426fd7d 100644 --- a/test/plugins/test_mbsubmit.py +++ b/test/plugins/test_mbsubmit.py @@ -17,8 +17,6 @@ from beets.test.helper import ( AutotagImportTestCase, PluginMixin, TerminalImportMixin, - capture_stdout, - control_stdin, ) @@ -34,10 +32,10 @@ class MBSubmitPluginTest( def test_print_tracks_output(self): """Test the output of the "print tracks" choice.""" - with capture_stdout() as output: - with control_stdin("\n".join(["p", "s"])): - # Print tracks; Skip - self.importer.run() + self.io.addinput("p") + self.io.addinput("s") + # Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( @@ -45,17 +43,19 @@ class MBSubmitPluginTest( "01. Tag Track 1 - Tag Artist (0:01)\n" "02. Tag Track 2 - Tag Artist (0:01)" ) - assert tracklist in output.getvalue() + assert tracklist in self.io.getoutput() def test_print_tracks_output_as_tracks(self): """Test the output of the "print tracks" choice, as singletons.""" - with capture_stdout() as output: - with control_stdin("\n".join(["t", "s", "p", "s"])): - # as Tracks; Skip; Print tracks; Skip - self.importer.run() + self.io.addinput("t") + self.io.addinput("s") + self.io.addinput("p") + self.io.addinput("s") + # as Tracks; Skip; Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( "Open files with Picard? 02. Tag Track 2 - Tag Artist (0:01)" ) - assert tracklist in output.getvalue() + assert tracklist in self.io.getoutput() diff --git a/test/plugins/test_missing.py b/test/plugins/test_missing.py index d12f2b4cf..812ed5fa3 100644 --- a/test/plugins/test_missing.py +++ b/test/plugins/test_missing.py @@ -3,20 +3,23 @@ import uuid import pytest from beets.library import Album -from beets.test.helper import PluginMixin, TestHelper +from beets.test.helper import IOMixin, PluginMixin, TestHelper @pytest.fixture -def helper(): +def helper(request): helper = TestHelper() helper.setup_beets() - yield helper + request.instance.lib = helper.lib + + yield helper.teardown_beets() -class TestMissingAlbums(PluginMixin): +@pytest.mark.usefixtures("helper") +class TestMissingAlbums(IOMixin, PluginMixin): plugin = "missing" album_in_lib = Album( album="Album", @@ -47,15 +50,13 @@ class TestMissingAlbums(PluginMixin): ], ) def test_missing_artist_albums( - self, requests_mock, helper, release_from_mb, expected_output + self, requests_mock, release_from_mb, expected_output ): - helper.lib.add(self.album_in_lib) + self.lib.add(self.album_in_lib) requests_mock.get( f"/ws/2/release-group?artist={self.album_in_lib.mb_albumartistid}", json={"release-groups": [release_from_mb]}, ) with self.configure_plugin({}): - assert ( - helper.run_with_output("missing", "--album") == expected_output - ) + assert self.run_with_output("missing", "--album") == expected_output diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 8d7c5a2f8..e000e16ec 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -526,20 +526,20 @@ class MBAlbumInfoTest(MusicBrainzTestCase): config["musicbrainz"]["genres_tag"] = "genre" release = self._make_release() d = self.mb.album_info(release) - assert d.genre == "GENRE" + assert d.genres == ["GENRE"] def test_tags(self): config["musicbrainz"]["genres"] = True config["musicbrainz"]["genres_tag"] = "tag" release = self._make_release() d = self.mb.album_info(release) - assert d.genre == "TAG" + assert d.genres == ["TAG"] def test_no_genres(self): config["musicbrainz"]["genres"] = False release = self._make_release() d = self.mb.album_info(release) - assert d.genre is None + assert d.genres == [] def test_ignored_media(self): config["match"]["ignored_media"] = ["IGNORED1", "IGNORED2"] diff --git a/test/plugins/test_play.py b/test/plugins/test_play.py index b184db63f..53de21233 100644 --- a/test/plugins/test_play.py +++ b/test/plugins/test_play.py @@ -21,14 +21,14 @@ from unittest.mock import ANY, patch import pytest -from beets.test.helper import CleanupModulesMixin, PluginTestCase, control_stdin +from beets.test.helper import CleanupModulesMixin, IOMixin, PluginTestCase from beets.ui import UserError from beets.util import open_anything from beetsplug.play import PlayPlugin @patch("beetsplug.play.util.interactive_open") -class PlayPluginTest(CleanupModulesMixin, PluginTestCase): +class PlayPluginTest(IOMixin, CleanupModulesMixin, PluginTestCase): modules = (PlayPlugin.__module__,) plugin = "play" @@ -127,8 +127,8 @@ class PlayPluginTest(CleanupModulesMixin, PluginTestCase): self.config["play"]["warning_threshold"] = 1 self.add_item(title="another NiceTitle") - with control_stdin("a"): - self.run_command("play", "nice") + self.io.addinput("a") + self.run_command("play", "nice") open_mock.assert_not_called() @@ -138,12 +138,12 @@ class PlayPluginTest(CleanupModulesMixin, PluginTestCase): expected_playlist = f"{self.item.filepath}\n{self.other_item.filepath}" - with control_stdin("a"): - self.run_and_assert( - open_mock, - ["-y", "NiceTitle"], - expected_playlist=expected_playlist, - ) + self.io.addinput("a") + self.run_and_assert( + open_mock, + ["-y", "NiceTitle"], + expected_playlist=expected_playlist, + ) def test_command_failed(self, open_mock): open_mock.side_effect = OSError("some reason") diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 094349b25..054b70a76 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -14,12 +14,11 @@ import unittest -from typing import ClassVar +from typing import Any, ClassVar import pytest from mediafile import MediaFile -from beets import config from beets.test.helper import ( AsIsImporterMixin, ImportTestCase, @@ -39,10 +38,15 @@ try: except (ImportError, ValueError): GST_AVAILABLE = False -if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]): - GAIN_PROG_AVAILABLE = True -else: - GAIN_PROG_AVAILABLE = False + +GAIN_PROG = next( + ( + cmd + for cmd in ["mp3gain", "mp3rgain", "aacgain"] + if has_program(cmd, ["-v"]) + ), + None, +) FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"]) @@ -63,14 +67,18 @@ class ReplayGainTestCase(PluginMixin, ImportTestCase): plugin = "replaygain" preload_plugin = False - backend: ClassVar[str] + plugin_config: ClassVar[dict[str, Any]] + + @property + def backend(self): + return self.plugin_config["backend"] def setUp(self): # Implemented by Mixins, see above. This may decide to skip the test. self.test_backend() super().setUp() - self.config["replaygain"]["backend"] = self.backend + self.config["replaygain"].set(self.plugin_config) self.load_plugins() @@ -81,8 +89,16 @@ class ThreadedImportMixin: self.config["threaded"] = True -class GstBackendMixin: - backend = "gstreamer" +class BackendMixin: + plugin_config: ClassVar[dict[str, Any]] + has_r128_support: bool + + def test_backend(self): + """Check whether the backend actually has all required functionality.""" + + +class GstBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "gstreamer"} has_r128_support = True def test_backend(self): @@ -90,30 +106,25 @@ class GstBackendMixin: try: # Check if required plugins can be loaded by instantiating a # GStreamerBackend (via its .__init__). - config["replaygain"]["targetlevel"] = 89 - GStreamerBackend(config["replaygain"], None) + self.config["replaygain"]["targetlevel"] = 89 + GStreamerBackend(self.config["replaygain"], None) except FatalGstreamerPluginReplayGainError as e: # Skip the test if plugins could not be loaded. self.skipTest(str(e)) -class CmdBackendMixin: - backend = "command" +class CmdBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = { + "backend": "command", + "command": GAIN_PROG, + } has_r128_support = False - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - -class FfmpegBackendMixin: - backend = "ffmpeg" +class FfmpegBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "ffmpeg"} has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class ReplayGainCliTest: FNAME: str @@ -332,7 +343,7 @@ class ReplayGainGstCliTest( FNAME = "full" # file contains only silence -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdCliTest( ReplayGainCliTest, ReplayGainTestCase, CmdBackendMixin ): @@ -369,7 +380,7 @@ class ReplayGainGstImportTest(ImportTest, ReplayGainTestCase, GstBackendMixin): pass -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdImportTest(ImportTest, ReplayGainTestCase, CmdBackendMixin): pass diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 8ec2c74ce..d1125158f 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -24,7 +24,7 @@ import pytest from beets import config 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.test.helper import BeetsTestCase, IOMixin, PluginTestCase from beets.ui import UserError from beets.util import CHAR_REPLACE, syspath from beetsplug.smartplaylist import SmartPlaylistPlugin @@ -76,11 +76,11 @@ class SmartPlaylistTest(BeetsTestCase): {"name": "one_non_empty_sort", "query": ["foo year+", "bar"]}, { "name": "multiple_sorts", - "query": ["foo year+", "bar genre-"], + "query": ["foo year+", "bar genres-"], }, { "name": "mixed", - "query": ["foo year+", "bar", "baz genre+ id-"], + "query": ["foo year+", "bar", "baz genres+ id-"], }, ] ) @@ -102,11 +102,11 @@ class SmartPlaylistTest(BeetsTestCase): # Multiple queries store individual sorts in the tuple assert all(isinstance(x, NullSort) for x in sorts["only_empty_sorts"]) assert sorts["one_non_empty_sort"] == [sort("year"), NullSort()] - assert sorts["multiple_sorts"] == [sort("year"), sort("genre", False)] + assert sorts["multiple_sorts"] == [sort("year"), sort("genres", False)] assert sorts["mixed"] == [ sort("year"), NullSort(), - MultipleSort([sort("genre"), sort("id", False)]), + MultipleSort([sort("genres"), sort("id", False)]), ] def test_matches(self): @@ -259,7 +259,7 @@ class SmartPlaylistTest(BeetsTestCase): type(i).title = PropertyMock(return_value="fake Title") type(i).length = PropertyMock(return_value=300.123) type(i).path = PropertyMock(return_value=b"/tagada.mp3") - a = {"id": 456, "genre": "Fake Genre"} + a = {"id": 456, "genres": ["Rock", "Pop"]} i.__getitem__.side_effect = a.__getitem__ i.evaluate_template.side_effect = lambda pl, _: pl.replace( b"$title", @@ -280,7 +280,7 @@ class SmartPlaylistTest(BeetsTestCase): config["smartplaylist"]["output"] = "extm3u" config["smartplaylist"]["relative_to"] = False config["smartplaylist"]["playlist_dir"] = str(dir) - config["smartplaylist"]["fields"] = ["id", "genre"] + config["smartplaylist"]["fields"] = ["id", "genres"] try: spl.update_playlists(lib) except Exception: @@ -297,7 +297,7 @@ class SmartPlaylistTest(BeetsTestCase): assert content == ( b"#EXTM3U\n" - b'#EXTINF:300 id="456" genre="Fake%20Genre",Fake Artist - fake Title\n' + b'#EXTINF:300 id="456" genres="Rock%3B%20Pop",Fake Artist - fake Title\n' b"/tagada.mp3\n" ) @@ -458,7 +458,7 @@ class SmartPlaylistTest(BeetsTestCase): assert content.count(b"/item2.mp3") == 1 -class SmartPlaylistCLITest(PluginTestCase): +class SmartPlaylistCLITest(IOMixin, PluginTestCase): plugin = "smartplaylist" def setUp(self): diff --git a/test/plugins/test_types_plugin.py b/test/plugins/test_types_plugin.py index 41807b80d..24fb577f7 100644 --- a/test/plugins/test_types_plugin.py +++ b/test/plugins/test_types_plugin.py @@ -19,10 +19,10 @@ from datetime import datetime import pytest from confuse import ConfigValueError -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class TypesPluginTest(PluginTestCase): +class TypesPluginTest(IOMixin, PluginTestCase): plugin = "types" def test_integer_modify_and_query(self): diff --git a/test/plugins/test_zero.py b/test/plugins/test_zero.py index f466a409e..16ceaf56d 100644 --- a/test/plugins/test_zero.py +++ b/test/plugins/test_zero.py @@ -3,12 +3,12 @@ from mediafile import MediaFile from beets.library import Item -from beets.test.helper import PluginTestCase, control_stdin +from beets.test.helper import IOMixin, PluginTestCase from beets.util import syspath from beetsplug.zero import ZeroPlugin -class ZeroPluginTest(PluginTestCase): +class ZeroPluginTest(IOMixin, PluginTestCase): plugin = "zero" preload_plugin = False @@ -102,12 +102,10 @@ class ZeroPluginTest(PluginTestCase): item.write() item_id = item.id - with ( - self.configure_plugin( - {"fields": ["comments"], "update_database": True, "auto": False} - ), - control_stdin("y"), + with self.configure_plugin( + {"fields": ["comments"], "update_database": True, "auto": False} ): + self.io.addinput("y") self.run_command("zero") mf = MediaFile(syspath(item.path)) @@ -125,16 +123,14 @@ class ZeroPluginTest(PluginTestCase): item.write() item_id = item.id - with ( - self.configure_plugin( - { - "fields": ["comments"], - "update_database": False, - "auto": False, - } - ), - control_stdin("y"), + with self.configure_plugin( + { + "fields": ["comments"], + "update_database": False, + "auto": False, + } ): + self.io.addinput("y") self.run_command("zero") mf = MediaFile(syspath(item.path)) @@ -187,7 +183,8 @@ class ZeroPluginTest(PluginTestCase): item_id = item.id - with self.configure_plugin({"fields": []}), control_stdin("y"): + with self.configure_plugin({"fields": []}): + self.io.addinput("y") self.run_command("zero") item = self.lib.get_item(item_id) @@ -203,12 +200,10 @@ class ZeroPluginTest(PluginTestCase): item_id = item.id - with ( - self.configure_plugin( - {"fields": ["year"], "keep_fields": ["comments"]} - ), - control_stdin("y"), + with self.configure_plugin( + {"fields": ["year"], "keep_fields": ["comments"]} ): + self.io.addinput("y") self.run_command("zero") item = self.lib.get_item(item_id) @@ -307,12 +302,10 @@ class ZeroPluginTest(PluginTestCase): ) item.write() item_id = item.id - with ( - self.configure_plugin( - {"fields": ["comments"], "update_database": True, "auto": False} - ), - control_stdin("n"), + with self.configure_plugin( + {"fields": ["comments"], "update_database": True, "auto": False} ): + self.io.addinput("n") self.run_command("zero") mf = MediaFile(syspath(item.path)) diff --git a/test/rsrc/unicode’d.mp3 b/test/rsrc/unicode’d.mp3 index f7e8b6285..2b306cc13 100644 Binary files a/test/rsrc/unicode’d.mp3 and b/test/rsrc/unicode’d.mp3 differ diff --git a/test/test_files.py b/test/test_files.py index d0d93987c..4ed2f8608 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -165,7 +165,7 @@ class MoveTest(BeetsTestCase): self.i.move(operation=MoveOperation.LINK) assert self.dest.exists() assert os.path.islink(syspath(self.dest)) - assert self.dest.resolve() == self.path + assert self.dest.resolve() == self.path.resolve() @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_does_not_depart(self): diff --git a/test/test_importer.py b/test/test_importer.py index 6ae7d562b..56327af06 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -131,7 +131,7 @@ class NonAutotaggedImportTest(PathsMixin, AsIsImporterMixin, ImportTestCase): assert self.track_lib_path.exists() assert self.track_lib_path.is_symlink() - assert self.track_lib_path.resolve() == self.track_import_path + assert self.track_lib_path.resolve() == self.track_import_path.resolve() @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_import_hardlink_arrives(self): @@ -304,15 +304,15 @@ class ImportSingletonTest(AutotagImportTestCase): assert len(self.lib.albums()) == 2 def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" disc = 0 config["import"]["set_fields"] = { + "genres": "; ".join(genres), "collection": collection, - "genre": genre, - "title": "$title - formatted", "disc": disc, + "title": "$title - formatted", } # As-is item import. @@ -322,7 +322,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() # TODO: Not sure this is necessary. - assert item.genre == genre + assert item.genres == genres assert item.collection == collection assert item.title == "Tag Track 1 - formatted" assert item.disc == disc @@ -337,7 +337,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() - assert item.genre == genre + assert item.genres == genres assert item.collection == collection assert item.title == "Applied Track 1 - formatted" assert item.disc == disc @@ -373,12 +373,12 @@ class ImportTest(PathsMixin, AutotagImportTestCase): config["import"]["from_scratch"] = True for mediafile in self.import_media: - mediafile.genre = "Tag Genre" + mediafile.genres = ["Tag Genre"] mediafile.save() self.importer.add_choice(importer.Action.APPLY) self.importer.run() - assert self.lib.items().get().genre == "" + assert not self.lib.items().get().genres def test_apply_from_scratch_keeps_format(self): config["import"]["from_scratch"] = True @@ -464,17 +464,17 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.lib.items().get().data_source def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" - comments = "managed by beets" disc = 0 + comments = "managed by beets" config["import"]["set_fields"] = { - "genre": genre, + "genres": "; ".join(genres), "collection": collection, + "disc": disc, "comments": comments, "album": "$album - formatted", - "disc": disc, } # As-is album import. @@ -483,11 +483,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() # TODO: Not sure this is necessary. - assert album.genre == genre + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( @@ -505,11 +504,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() - assert album.genre == genre + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( diff --git a/test/test_library.py b/test/test_library.py index 4acf34746..5af6f76d8 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -56,25 +56,18 @@ class LoadTest(ItemInDBTestCase): class StoreTest(ItemInDBTestCase): def test_store_changes_database_value(self): - self.i.year = 1987 + new_year = 1987 + self.i.year = new_year self.i.store() - new_year = ( - self.lib._connection() - .execute("select year from items where title = ?", (self.i.title,)) - .fetchone()["year"] - ) - assert new_year == 1987 + + assert self.lib.get_item(self.i.id).year == new_year def test_store_only_writes_dirty_fields(self): - original_genre = self.i.genre - self.i._values_fixed["genre"] = "beatboxing" # change w/o dirtying + new_year = 1987 + self.i._values_fixed["year"] = new_year # change w/o dirtying self.i.store() - new_genre = ( - self.lib._connection() - .execute("select genre from items where title = ?", (self.i.title,)) - .fetchone()["genre"] - ) - assert new_genre == original_genre + + assert self.lib.get_item(self.i.id).year != new_year def test_store_clears_dirty_flags(self): self.i.composer = "tvp" @@ -688,14 +681,14 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin): self._assert_dest(b"/base/not_played") def test_first(self): - self.i.genres = "Pop; Rock; Classical Crossover" - self._setf("%first{$genres}") - self._assert_dest(b"/base/Pop") + self.i.albumtypes = ["album", "compilation"] + self._setf("%first{$albumtypes}") + self._assert_dest(b"/base/album") def test_first_skip(self): - self.i.genres = "Pop; Rock; Classical Crossover" - self._setf("%first{$genres,1,2}") - self._assert_dest(b"/base/Classical Crossover") + self.i.albumtype = "album; ep; compilation" + self._setf("%first{$albumtype,1,2}") + self._assert_dest(b"/base/compilation") def test_first_different_sep(self): self._setf("%first{Alice / Bob / Eve,2,0, / , & }") diff --git a/test/test_metasync.py b/test/test_metasync.py index 13c003a1c..aeb38545b 100644 --- a/test/test_metasync.py +++ b/test/test_metasync.py @@ -20,7 +20,7 @@ from datetime import datetime from beets.library import Item from beets.test import _common -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase def _parsetime(s): @@ -31,7 +31,7 @@ def _is_windows(): return platform.system() == "Windows" -class MetaSyncTest(PluginTestCase): +class MetaSyncTest(IOMixin, PluginTestCase): plugin = "metasync" itunes_library_unix = os.path.join(_common.RSRC, b"itunes_library_unix.xml") itunes_library_windows = os.path.join( diff --git a/test/test_plugins.py b/test/test_plugins.py index 53f24c13d..9622fb8db 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -38,6 +38,7 @@ from beets.test import helper from beets.test.helper import ( AutotagStub, ImportHelper, + IOMixin, PluginMixin, PluginTestCase, TerminalImportMixin, @@ -45,7 +46,7 @@ from beets.test.helper import ( from beets.util import PromptChoice, displayable_path, syspath -class TestPluginRegistration(PluginTestCase): +class TestPluginRegistration(IOMixin, PluginTestCase): class RatingPlugin(plugins.BeetsPlugin): item_types: ClassVar[dict[str, types.Type]] = { "rating": types.Float(), @@ -95,8 +96,7 @@ class TestPluginRegistration(PluginTestCase): item.add(self.lib) out = self.run_with_output("ls", "-f", "$multi_value") - delimiter = types.MULTI_VALUE_DSV.delimiter - assert out == f"one{delimiter}two{delimiter}three\n" + assert out == "one; two; three\n" class PluginImportTestCase(ImportHelper, PluginTestCase): @@ -429,8 +429,9 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase): # DummyPlugin.foo() should be called once with patch.object(DummyPlugin, "foo", autospec=True) as mock_foo: - with helper.control_stdin("\n".join(["f", "s"])): - self.importer.run() + self.io.addinput("f") + self.io.addinput("n") + self.importer.run() assert mock_foo.call_count == 1 # input_options should be called twice, as foo() returns None @@ -471,8 +472,8 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase): ) # DummyPlugin.foo() should be called once - with helper.control_stdin("f\n"): - self.importer.run() + self.io.addinput("f") + self.importer.run() # input_options should be called once, as foo() returns SKIP self.mock_input_options.assert_called_once_with( diff --git a/test/test_query.py b/test/test_query.py index 0ddf83e3a..81532c436 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -71,7 +71,7 @@ class TestGet: album="baz", year=2001, comp=True, - genre="rock", + genres=["rock"], ), helper.create_item( title="second", @@ -80,7 +80,7 @@ class TestGet: album="baz", year=2002, comp=True, - genre="Rock", + genres=["Rock"], ), ] album = helper.lib.add_album(album_items) @@ -94,7 +94,7 @@ class TestGet: album="foo", year=2003, comp=False, - genre="Hard Rock", + genres=["Hard Rock"], comments="caf\xe9", ) @@ -125,12 +125,12 @@ class TestGet: ("comments:caf\xe9", ["third"]), ("comp:true", ["first", "second"]), ("comp:false", ["third"]), - ("genre:=rock", ["first"]), - ("genre:=Rock", ["second"]), - ('genre:="Hard Rock"', ["third"]), - ('genre:=~"hard rock"', ["third"]), - ("genre:=~rock", ["first", "second"]), - ('genre:="hard rock"', []), + ("genres:=rock", ["first"]), + ("genres:=Rock", ["second"]), + ('genres:="Hard Rock"', ["third"]), + ('genres:=~"hard rock"', ["third"]), + ("genres:=~rock", ["first", "second"]), + ('genres:="hard rock"', []), ("popebear", []), ("pope:bear", []), ("singleton:true", ["third"]), @@ -243,13 +243,7 @@ class TestGet: class TestMatch: @pytest.fixture(scope="class") def item(self): - return _common.item( - album="the album", - disc=6, - genre="the genre", - year=1, - bitrate=128000, - ) + return _common.item(album="the album", disc=6, year=1, bitrate=128000) @pytest.mark.parametrize( "q, should_match", @@ -260,9 +254,9 @@ class TestMatch: (SubstringQuery("album", "album"), True), (SubstringQuery("album", "ablum"), False), (SubstringQuery("disc", "6"), True), - (StringQuery("genre", "the genre"), True), - (StringQuery("genre", "THE GENRE"), True), - (StringQuery("genre", "genre"), False), + (StringQuery("album", "the album"), True), + (StringQuery("album", "THE ALBUM"), True), + (StringQuery("album", "album"), False), (NumericQuery("year", "1"), True), (NumericQuery("year", "10"), False), (NumericQuery("bitrate", "100000..200000"), True), diff --git a/test/test_sort.py b/test/test_sort.py index 460aa07b8..d7d651de5 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -33,7 +33,7 @@ class DummyDataTestCase(BeetsTestCase): albums = [ Album( album="Album A", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-1", flex2="Flex2-A", @@ -41,7 +41,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album B", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-2", flex2="Flex2-A", @@ -49,7 +49,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album C", - genre="Jazz", + genres=["Jazz"], year=2005, flex1="Flex1-1", flex2="Flex2-B", @@ -236,19 +236,19 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): def test_sort_two_field_asc(self): q = "" - s1 = dbcore.query.FixedFieldSort("genre", True) + s1 = dbcore.query.FixedFieldSort("genres", True) s2 = dbcore.query.FixedFieldSort("album", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) results = self.lib.albums(q, sort) - assert results[0]["genre"] <= results[1]["genre"] - assert results[1]["genre"] <= results[2]["genre"] - assert results[1]["genre"] == "Rock" - assert results[2]["genre"] == "Rock" + assert results[0]["genres"] <= results[1]["genres"] + assert results[1]["genres"] <= results[2]["genres"] + assert results[1]["genres"] == ["Rock"] + assert results[2]["genres"] == ["Rock"] assert results[1]["album"] <= results[2]["album"] # same thing with query string - q = "genre+ album+" + q = "genres+ album+" results2 = self.lib.albums(q) for r1, r2 in zip(results, results2): assert r1.id == r2.id @@ -388,7 +388,7 @@ class CaseSensitivityTest(DummyDataTestCase): album = Album( album="album", - genre="alternative", + genres=["alternative"], year="2001", flex1="flex1", flex2="flex2-A", diff --git a/test/ui/commands/test_completion.py b/test/ui/commands/test_completion.py index ee2881a0e..992ed58c8 100644 --- a/test/ui/commands/test_completion.py +++ b/test/ui/commands/test_completion.py @@ -49,7 +49,6 @@ class CompletionTest(IOMixin, TestPluginTestCase): # Load completion script. self.run_command("completion", lib=None) completion_script = self.io.getoutput().encode("utf-8") - self.io.restore() tester.stdin.writelines(completion_script.splitlines(True)) # Load test suite. diff --git a/test/ui/commands/test_config.py b/test/ui/commands/test_config.py index c1215ef43..cd83b919f 100644 --- a/test/ui/commands/test_config.py +++ b/test/ui/commands/test_config.py @@ -5,10 +5,10 @@ import pytest import yaml from beets import config, ui -from beets.test.helper import BeetsTestCase +from beets.test.helper import BeetsTestCase, IOMixin -class ConfigCommandTest(BeetsTestCase): +class ConfigCommandTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() for k in ("VISUAL", "EDITOR"): diff --git a/test/ui/commands/test_fields.py b/test/ui/commands/test_fields.py index 0eaaa9ceb..98d4809c9 100644 --- a/test/ui/commands/test_fields.py +++ b/test/ui/commands/test_fields.py @@ -16,7 +16,7 @@ class FieldsTest(IOMixin, ItemInDBTestCase): items = library.Item.all_keys() albums = library.Album.all_keys() - output = self.io.stdout.get().split() + output = self.io.getoutput().split() self.remove_keys(items, output) self.remove_keys(albums, output) diff --git a/test/ui/commands/test_list.py b/test/ui/commands/test_list.py index a63a56ad1..0828980ca 100644 --- a/test/ui/commands/test_list.py +++ b/test/ui/commands/test_list.py @@ -1,9 +1,9 @@ from beets.test import _common -from beets.test.helper import BeetsTestCase, capture_stdout +from beets.test.helper import BeetsTestCase, IOMixin from beets.ui.commands.list import list_items -class ListTest(BeetsTestCase): +class ListTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() self.item = _common.item() @@ -12,13 +12,12 @@ class ListTest(BeetsTestCase): self.lib.add_album([self.item]) def _run_list(self, query="", album=False, path=False, fmt=""): - with capture_stdout() as stdout: - list_items(self.lib, query, album, fmt) - return stdout + list_items(self.lib, query, album, fmt) + return self.io.getoutput() def test_list_outputs_item(self): stdout = self._run_list() - assert "the title" in stdout.getvalue() + assert "the title" in stdout def test_list_unicode_query(self): self.item.title = "na\xefve" @@ -26,44 +25,44 @@ class ListTest(BeetsTestCase): self.lib._connection().commit() stdout = self._run_list(["na\xefve"]) - out = stdout.getvalue() + out = stdout assert "na\xefve" in out def test_list_item_path(self): stdout = self._run_list(fmt="$path") - assert stdout.getvalue().strip() == "xxx/yyy" + assert stdout.strip() == "xxx/yyy" def test_list_album_outputs_something(self): stdout = self._run_list(album=True) - assert len(stdout.getvalue()) > 0 + assert len(stdout) > 0 def test_list_album_path(self): stdout = self._run_list(album=True, fmt="$path") - assert stdout.getvalue().strip() == "xxx" + assert stdout.strip() == "xxx" def test_list_album_omits_title(self): stdout = self._run_list(album=True) - assert "the title" not in stdout.getvalue() + assert "the title" not in stdout def test_list_uses_track_artist(self): stdout = self._run_list() - assert "the artist" in stdout.getvalue() - assert "the album artist" not in stdout.getvalue() + assert "the artist" in stdout + assert "the album artist" not in stdout def test_list_album_uses_album_artist(self): stdout = self._run_list(album=True) - assert "the artist" not in stdout.getvalue() - assert "the album artist" in stdout.getvalue() + assert "the artist" not in stdout + assert "the album artist" in stdout def test_list_item_format_artist(self): stdout = self._run_list(fmt="$artist") - assert "the artist" in stdout.getvalue() + assert "the artist" in stdout def test_list_item_format_multiple(self): stdout = self._run_list(fmt="$artist - $album - $year") - assert "the artist - the album - 0001" == stdout.getvalue().strip() + assert "the artist - the album - 0001" == stdout.strip() def test_list_album_format(self): - stdout = self._run_list(album=True, fmt="$genre") - assert "the genre" in stdout.getvalue() - assert "the album" not in stdout.getvalue() + stdout = self._run_list(album=True, fmt="$genres") + assert "the genre" in stdout + assert "the album" not in stdout diff --git a/test/ui/commands/test_modify.py b/test/ui/commands/test_modify.py index 77d378032..3e7a63d90 100644 --- a/test/ui/commands/test_modify.py +++ b/test/ui/commands/test_modify.py @@ -2,23 +2,24 @@ import unittest from mediafile import MediaFile -from beets.test.helper import BeetsTestCase, control_stdin +from beets.test.helper import BeetsTestCase, IOMixin from beets.ui.commands.modify import modify_parse_args from beets.util import syspath -class ModifyTest(BeetsTestCase): +class ModifyTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() self.album = self.add_album_fixture() [self.item] = self.album.items() - def modify_inp(self, inp, *args): - with control_stdin(inp): - self.run_command("modify", *args) + def modify_inp(self, inp: list[str], *args): + for chat in inp: + self.io.addinput(chat) + self.run_command("modify", *args) def modify(self, *args): - self.modify_inp("y", *args) + self.modify_inp(["y"], *args) # Item tests @@ -30,14 +31,14 @@ class ModifyTest(BeetsTestCase): def test_modify_item_abort(self): item = self.lib.items().get() title = item.title - self.modify_inp("n", "title=newTitle") + self.modify_inp(["n"], "title=newTitle") item = self.lib.items().get() assert item.title == title def test_modify_item_no_change(self): title = "Tracktitle" item = self.add_item_fixture(title=title) - self.modify_inp("y", "title", f"title={title}") + self.modify_inp(["y"], "title", f"title={title}") item = self.lib.items(title).get() assert item.title == title @@ -96,7 +97,9 @@ class ModifyTest(BeetsTestCase): title=f"{title}{i}", artist=original_artist, album=album ) self.modify_inp( - "s\ny\ny\ny\nn\nn\ny\ny\ny\ny\nn", title, f"artist={new_artist}" + ["s", "y", "y", "y", "n", "n", "y", "y", "y", "y", "n"], + title, + f"artist={new_artist}", ) original_items = self.lib.items(f"artist:{original_artist}") new_items = self.lib.items(f"artist:{new_artist}") diff --git a/test/ui/commands/test_update.py b/test/ui/commands/test_update.py index 3fb687418..937ded10c 100644 --- a/test/ui/commands/test_update.py +++ b/test/ui/commands/test_update.py @@ -103,22 +103,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=False, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" not in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) @@ -141,22 +141,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["album"]) item = self.lib.items().get() assert b"differentAlbum" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_album_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() - self._update(move=True, fields=["genre"]) + self._update(move=True, fields=["genres"]) item = self.lib.items().get() assert b"differentAlbum" not in item.path - assert item.genre == "differentGenre" + assert item.genres == ["differentGenre"] def test_mtime_match_skips_update(self): mf = MediaFile(syspath(self.i.path)) diff --git a/test/ui/commands/test_write.py b/test/ui/commands/test_write.py index 312b51dd2..7197cbded 100644 --- a/test/ui/commands/test_write.py +++ b/test/ui/commands/test_write.py @@ -1,7 +1,7 @@ -from beets.test.helper import BeetsTestCase +from beets.test.helper import BeetsTestCase, IOMixin -class WriteTest(BeetsTestCase): +class WriteTest(IOMixin, BeetsTestCase): def write_cmd(self, *args): return self.run_with_output("write", *args) diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index 35f3c6ca7..24bac0123 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -1,3 +1,5 @@ +from textwrap import dedent + import pytest from beets.library import Item @@ -15,7 +17,7 @@ class TestFieldDiff: def patch_colorize(self, monkeypatch): """Patch to return a deterministic string format instead of ANSI codes.""" monkeypatch.setattr( - "beets.ui.colorize", + "beets.ui._colorize", lambda color_name, text: f"[{color_name}]{text}[/]", ) @@ -32,12 +34,25 @@ class TestFieldDiff: p({"title": "foo"}, {"title": "bar"}, "title", f"title: {diff_fmt('foo', 'bar')}", id="string_full_replace"), p({"title": "prefix foo"}, {"title": "prefix bar"}, "title", "title: prefix [text_diff_removed]foo[/] -> prefix [text_diff_added]bar[/]", id="string_partial_change"), p({"year": 2000}, {"year": 2001}, "year", f"year: {diff_fmt('2000', '2001')}", id="int_changed"), - p({}, {"genre": "Rock"}, "genre", "genre: -> [text_diff_added]Rock[/]", id="field_added"), - p({"genre": "Rock"}, {}, "genre", "genre: [text_diff_removed]Rock[/] -> ", id="field_removed"), + p({}, {"artist": "Artist"}, "artist", "artist: -> [text_diff_added]Artist[/]", id="field_added"), + p({"artist": "Artist"}, {}, "artist", "artist: [text_diff_removed]Artist[/] -> ", id="field_removed"), p({"track": 1}, {"track": 2}, "track", f"track: {diff_fmt('01', '02')}", id="formatted_value_changed"), p({"mb_trackid": None}, {"mb_trackid": "1234"}, "mb_trackid", "mb_trackid: -> [text_diff_added]1234[/]", id="none_to_value"), p({}, {"new_flex": "foo"}, "new_flex", "[text_diff_added]new_flex: foo[/]", id="flex_field_added"), p({"old_flex": "foo"}, {}, "old_flex", "[text_diff_removed]old_flex: foo[/]", id="flex_field_removed"), + p({"albumtypes": ["album", "ep"]}, {"albumtypes": ["ep", "album"]}, "albumtypes", None, id="multi_value_unchanged"), + p( + {"albumtypes": ["ep"]}, + {"albumtypes": ["album", "compilation"]}, + "albumtypes", + dedent(""" + albumtypes: + [text_diff_removed] - ep[/] + [text_diff_added] + album[/] + [text_diff_added] + compilation[/] + """).strip(), + id="multi_value_changed" + ), ], ) # fmt: skip @pytest.mark.parametrize("color", [True], ids=["color_enabled"]) diff --git a/test/ui/test_ui.py b/test/ui/test_ui.py index a0bf2e598..577954a85 100644 --- a/test/ui/test_ui.py +++ b/test/ui/test_ui.py @@ -71,11 +71,11 @@ class TestPluginTestCase(PluginTestCase): plugin = "test" def setUp(self): + self.config["pluginpath"] = [_common.PLUGINPATH] super().setUp() - config["pluginpath"] = [_common.PLUGINPATH] -class ConfigTest(TestPluginTestCase): +class ConfigTest(IOMixin, TestPluginTestCase): def setUp(self): super().setUp() @@ -162,6 +162,7 @@ class ConfigTest(TestPluginTestCase): with self.write_config_file() as config: config.write("library: /xxx/yyy/not/a/real/path") + self.io.addinput("n") with pytest.raises(ui.UserError): self.run_command("test", lib=None) @@ -397,7 +398,7 @@ class PluginTest(TestPluginTestCase): self.run_command("test", lib=None) -class CommonOptionsParserCliTest(BeetsTestCase): +class CommonOptionsParserCliTest(IOMixin, BeetsTestCase): """Test CommonOptionsParser and formatting LibModel formatting on 'list' command. """ diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index f6c9fe245..00e0a6fe5 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -22,7 +22,7 @@ from random import random from beets import config, ui from beets.test import _common -from beets.test.helper import BeetsTestCase, IOMixin, control_stdin +from beets.test.helper import BeetsTestCase, IOMixin class InputMethodsTest(IOMixin, unittest.TestCase): @@ -85,7 +85,7 @@ class InputMethodsTest(IOMixin, unittest.TestCase): assert items == ["1", "3"] -class ParentalDirCreation(BeetsTestCase): +class ParentalDirCreation(IOMixin, BeetsTestCase): def test_create_yes(self): non_exist_path = _common.os.fsdecode( os.path.join(self.temp_dir, b"nonexist", str(random()).encode()) @@ -94,8 +94,8 @@ class ParentalDirCreation(BeetsTestCase): # occur; wish I can use a golang defer here. test_config = deepcopy(config) test_config["library"] = non_exist_path - with control_stdin("y"): - lib = ui._open_library(test_config) + self.io.addinput("y") + lib = ui._open_library(test_config) lib._close() def test_create_no(self): @@ -108,14 +108,14 @@ class ParentalDirCreation(BeetsTestCase): test_config = deepcopy(config) test_config["library"] = non_exist_path - with control_stdin("n"): - try: - lib = ui._open_library(test_config) - except ui.UserError: - if os.path.exists(non_exist_path_parent): - shutil.rmtree(non_exist_path_parent) - raise OSError("Parent directories should not be created.") - else: - if lib: - lib._close() + self.io.addinput("n") + try: + lib = ui._open_library(test_config) + except ui.UserError: + if os.path.exists(non_exist_path_parent): + shutil.rmtree(non_exist_path_parent) raise OSError("Parent directories should not be created.") + else: + if lib: + lib._close() + raise OSError("Parent directories should not be created.")