Move MusicBrainzPlugin to SearchApiMetadataSourcePlugin hooks.
Keep entity mapping and criteria in provider-specific hooks.
Update typing and tests for the candidate search path.
Move search orchestration into SearchApiMetadataSourcePlugin.
Migrate Deezer, Spotify, and Discogs to provider hooks.
Keep query handling, logging, and limits centralized.
Add force_ci kwarg to requires_import pytest marker to allow tests
to run unconditionally in CI (GitHub Actions), even if the module
is not detected locally. Refactor autobpm test to use this instead
of manual env-checking at module level.
Add a `requires_import` pytest marker and `is_importable` fixture to
conditionally skip or adjust assertions based on whether optional
dependencies are available. Apply this to `langdetect`-dependent
language detection tests in lyrics and migration test suites.
- Add `LyricsMetadataInFlexFieldsMigration` to extract legacy source
URLs and language metadata from lyrics text into flex attributes
- Add `Lyrics.from_legacy_text` to parse legacy lyrics format
- Move `with_row_factory` context manager up to base `Migration` class
- Rename `migrate_table` to `migrate_model` and pass model class
instead of table name string. This is so that the migration can access
both `_table` and `_flex_table` attributes.
- Make `langdetect` import optional in `Lyrics.__post_init__`: users may
not have have the dependency installed, and we do not want the
migration to fail because of that.
- Move `BACKEND_BY_NAME` to module level for use outside plugin class
* Introduce a `Lyrics` dataclass to carry text, source URL, and language
metadata through fetch, translation, and storage paths.
* Return `Lyrics` from backends and plugin lookup methods instead of raw
tuples/strings.
* Store backend name in `lyrics_source` derived from fetched URL root
domain.
* Simplify translator flow to operate on `Lyrics`, reuse line splitting,
append translations in-place, and record translation language
metadata.
When lyrics.synced is enabled, avoid replacing existing synced lyrics with
newly fetched unsynced lyrics, even with force enabled.
Allow replacement when the new lyrics are also synced, or when synced mode
is disabled.
Introduce a new lastgenre `cleanup_existing` flag.
It handles the case where canonicalization is desired on existing tags.
The new logic triggers if:
- `force`: False
- `cleanup_existing: True
Depending on whether `whitelist: True` or `canonical: True`, the genres
are then canonicalized and/or whitelisting is applied
Build playlist paths using the selected format (`--format`/config), and only
replace extensions when the destination file is actually transcoded.
Precompute playlist entries before conversion runs so `--keep-new` does not
pick up mutated item paths and produce mismatched extensions.
Add/expand convert CLI tests to cover:
- config format playlist extension
- `--format` override playlist extension
- no-transcode (`no_convert`) playlist extension
- `--keep-new` destination playlist path behavior
When importing with autotag=no, duplicate detection was skipped entirely
because the import_asis stage called _apply_choice() directly without
first calling _resolve_duplicates(). This meant the duplicate_keys and
duplicate_action config options were ignored for as-is imports.
This was a known limitation documented by a FIXME comment added in
commit 79d1203541 (Sep 2014): "We should also resolve duplicates when
not autotagging." The FIXME was later removed during a comment cleanup
(f145e3b18) but the issue was never addressed.
This commit adds the _resolve_duplicates() call to import_asis, ensuring
duplicate detection works consistently regardless of the autotag setting.
This applies to both album imports and singleton imports.
Test changes:
- Renamed test_no_autotag_keeps_duplicate_album to
test_no_autotag_removes_duplicate_album to verify the corrected behavior
- Added test_no_autotag_removes_duplicate_singleton to verify singleton
duplicate detection also works with autotag=no
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Consolidate multiple granular test methods in ApplyTest into a single
comprehensive test that validates all applied metadata at once. This
improves test maintainability and clarity by:
- Replacing ~20 individual test methods with one data-driven test
- Using expected data dictionaries to validate all fields together
- Removing ApplyCompilationTest class (covered by va=True in main test)
- Keeping focused tests for edge cases (artist_credit, date handling)
- Switching from BeetsTestCase to standard TestCase for speed
- Adding operator import for efficient data extraction
The new approach makes it easier to validate all applied metadata at once.
When `import.delete` or `import.move` is enabled, the `assign_art` method calls `task.prune(candidate.path)` unconditionally.
This incorrectly deletes the configured `fetchart.fallback` file.
Add explicit check to skip pruning when the candidate path matches the configured fallback.
* Move genre-to-genres migration into a dedicated Migration class and
wire it into Library._migrations for items and albums.
* Add batched SQL updates via mutate_many and share the multi-value
delimiter as a constant.
* Cover migration behavior with new tests.
I initially attempted to migrate using our model infrastructure
/ Model.store(), see the comparison below:
Durations migrating my library of ~9000 items and ~2300 albums:
1. Using our Python logic: 11 minutes
2. Using SQL directly: 4 seconds
That's why I've gone ahead with option 2.
- Add Library._make_table() override to automatically migrate genres when database schema is updated
- Migration splits comma/semicolon/slash-separated genre strings into genres list
- Writes changes to both database and media files with progress reporting
- Remove lazy migration from correct_list_fields() - now handled at database level
- Remove migration-specific tests (migration is now automatic, not lazy)
- Update changelog to reflect automatic migration behavior
Related PR review comment changes:
- Replace _is_valid with _filter_valid method in lastgenre plugin
- Use unique_list and remove genre field from Beatport plugin
- Simplify LastGenre tests - remove separator logic
- Document separator deprecation in lastgenre plugin
- Add deprecation warning for genre parameter in Info.__init__()
Simplify multi-genre implementation based on maintainer feedback (PR #6169).
Changes:
- Remove multi_value_genres and genre_separator config options
- Replace complex sync_genre_fields() with ensure_first_value('genre', 'genres')
- Update all plugins (Beatport, MusicBrainz, LastGenre) to always write genres as lists
- Add automatic migration for comma/semicolon/slash-separated genre strings
- Add 'beet migrate genres' command for explicit batch migration with --pretend flag
- Update all tests to reflect simplified approach (44 tests passing)
- Update documentation
Implementation aligns with maintainer vision of always using multi-value genres
internally with automatic backward-compatible sync to the genre field via
ensure_first_value(), eliminating configuration complexity.
Migration strategy avoids problems from #5540:
- Automatic lazy migration on item access (no reimport/mbsync needed)
- Optional batch migration command for user control
- No endless rewrite loops due to proper field synchronization
- Use '\␀' as the DB delimiter while formatting lists with '; ' for
templates.
- Update DelimitedString parsing to accept both separators:
* '\␀' for the values from the DB
* '; ' for the rest of parsed values (for example `beet modify genres="eletronic; jazz"`)
- Refresh %first docs and tests to reflect multi-value field behavior.
I got a little bit nerdsniped by the problems observed in #5027. In
short, my high-level diagnosis in
https://github.com/beetbox/beets/pull/5027#issuecomment-1857953929 seems
to have been correct: other tests were suppressing the legitimate
failure of a flaky test.
I found the problem by running other tests before the problem test, like
this:
```
$ pytest -k 'test_nonexistant_db or test_delete_removes_item' test/test_ui.py
```
When running `test_nonexistant_db` alone, it fails. When running it like
this with another test that goes first, it passes. That's the problem.
However, `test_delete_removes_item` is just one example that works to
make this problem happen. It appeared that _any_ test in a class that
used our `_common.TestCase` base class had this power. I tracked down
the issue to our `DummyIO` utility, which was having an unintentional
effect even when it was never actually used.
Here's the solution. Instead of restoring `sys.stdin` to
`sys.__stdin__`, we now restore it to whatever it was before we
installed out dummy I/O hooks. This is relevant in pytest, for example,
which installs its *own* `sys.stdin`, which we were then clobbering.
This was leading to the suppression of test failures observed in #5021
and addressed in #5027.
The CI will fail for this PR because it now (correctly) exposes a
failing test. Hopefully by combining this with the fixes in the works in
#5027, we'll be back to a passing test suite. 😃 @Phil305, could
you perhaps help validate that hypothesis?
Edit: @snejus:
I've now consolidated test I/O handling by removing the legacy
`control_stdin`/`capture_stdout` context managers and the custom
`DummyOut` stream, replacing them with a pytest-driven `io` fixture
that:
- provides controllable `stdin` via a lightweight `DummyIn`
- captures `stdout` via `capteesys`
- attaches a `DummyIO` helper to test classes as `self.io`
