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`
Create a centralised pytest fixture to provide controllable stdin and
captured stdout in all tests. Simplify DummyIO/DummyIn and remove the
custom DummyOut implementation and make use of pytest builtin fixtures.
Create a centralised pytest fixture to provide controllable stdin and
captured stdout that can be applied to any tests, regardless whether
they are based on pytest or unittest.
* `io` fixture can be used as a fixture in pytest-based tests
* `IOMixin` can be used to attach `io` attribute to any test class,
including `unittest.TestCase`
Use a case-insensitive sort key when ordering Markdown changelog bullet
points to produce stable ordering regardless of capitalization.
Diff for the last release:
diff --git a/before b/after
index 51303c65f..d88eda894 100644
--- a/before
+++ b/after
@@ -4,3 +4,2 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
-- Added support for Python 3.13.
- [Convert Plugin](https://beets.readthedocs.io/en/stable/plugins/convert.html): `force` can be passed to override checks like no_convert, never_convert_lossy_files, same format, and max_bitrate
@@ -23,2 +22,3 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
- [Titlecase Plugin](https://beets.readthedocs.io/en/stable/plugins/titlecase.html): Add the [Titlecase Plugin](https://beets.readthedocs.io/en/stable/plugins/titlecase.html) plugin to allow users to resolve differences in metadata source styles.
+- Added support for Python 3.13.
@@ -26,9 +26,2 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
-- Errors in metadata plugins during autotage process will now be logged but won't crash beets anymore. If you want to raise exceptions instead, set the new configuration option `raise_on_error` to `yes` 🐛 (#5903), 🐛 (#4789).
-- Fix a bug introduced in release 2.4.0 where import from any valid import-log-file always threw a "none of the paths are importable" error.
-- Handle potential OSError when unlinking temporary files in ArtResizer. 🐛 (#5615)
-- Running <span class="title-ref">beet --config \<mypath\> config -e</span> now edits <span class="title-ref">\<mypath\></span> rather than the default config path. 🐛 (#5652)
-- Sanitize log messages by removing control characters preventing terminal rendering issues.
-- When hardlinking from a symlink (e.g. importing a symlink with hardlinking enabled), dereference the symlink then hardlink, rather than creating a new (potentially broken) symlink 🐛 (#5676)
-- When using [FromFilename Plugin](https://beets.readthedocs.io/en/stable/plugins/fromfilename.html) together with [Edit Plugin](https://beets.readthedocs.io/en/stable/plugins/edit.html), temporary tags extracted from filenames are no longer lost when discarding or cancelling an edit session during import. 🐛 (#6104)
- [Command-Line Interface](https://beets.readthedocs.io/en/stable/reference/cli.html): Fix 'from_scratch' option for singleton imports: delete all (old) metadata when new metadata is applied. 🐛 (#3706)
@@ -44,4 +37,11 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
- [Spotify Plugin](https://beets.readthedocs.io/en/stable/plugins/spotify.html): Updated Spotify API credentials. 🐛 (#6270)
-- [Web Plugin](https://beets.readthedocs.io/en/stable/plugins/web.html): repair broken <span class="title-ref">/item/values/…</span> and <span class="title-ref">/albums/values/…</span> endpoints. Previously, due to single-quotes (ie. string literal) in the SQL query, the query eg. <span class="title-ref">GET /item/values/albumartist</span> would return the literal "albumartist" instead of a list of unique album artists.
- [update](https://beets.readthedocs.io/en/stable/reference/cli.html#update-cmd) [Edit Plugin](https://beets.readthedocs.io/en/stable/plugins/edit.html) fix display formatting of field changes to clearly show added and removed flexible fields.
+- [Web Plugin](https://beets.readthedocs.io/en/stable/plugins/web.html): repair broken <span class="title-ref">/item/values/…</span> and <span class="title-ref">/albums/values/…</span> endpoints. Previously, due to single-quotes (ie. string literal) in the SQL query, the query eg. <span class="title-ref">GET /item/values/albumartist</span> would return the literal "albumartist" instead of a list of unique album artists.
+- Errors in metadata plugins during autotage process will now be logged but won't crash beets anymore. If you want to raise exceptions instead, set the new configuration option `raise_on_error` to `yes` 🐛 (#5903), 🐛 (#4789).
+- Fix a bug introduced in release 2.4.0 where import from any valid import-log-file always threw a "none of the paths are importable" error.
+- Handle potential OSError when unlinking temporary files in ArtResizer. 🐛 (#5615)
+- Running <span class="title-ref">beet --config \<mypath\> config -e</span> now edits <span class="title-ref">\<mypath\></span> rather than the default config path. 🐛 (#5652)
+- Sanitize log messages by removing control characters preventing terminal rendering issues.
+- When hardlinking from a symlink (e.g. importing a symlink with hardlinking enabled), dereference the symlink then hardlink, rather than creating a new (potentially broken) symlink 🐛 (#5676)
+- When using [FromFilename Plugin](https://beets.readthedocs.io/en/stable/plugins/fromfilename.html) together with [Edit Plugin](https://beets.readthedocs.io/en/stable/plugins/edit.html), temporary tags extracted from filenames are no longer lost when discarding or cancelling an edit session during import. 🐛 (#6104)
@@ -71,2 +71,4 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
+- [BPD Plugin](https://beets.readthedocs.io/en/stable/plugins/bpd.html): Raise ImportError instead of ValueError when GStreamer is unavailable, enabling `importorskip` usage in pytest setup.
+- dbcore: Allow models to declare SQL indices; add an `items.album_id` index to speed up `album.items()` queries. 🐛 (#5809)
- Finally removed gmusic plugin and all related code/docs as the Google Play Music service was shut down in 2020.
@@ -76,3 +78 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
- Updated color documentation with `bright_*` and `bg_bright_*` entries.
-- [BPD Plugin](https://beets.readthedocs.io/en/stable/plugins/bpd.html): Raise ImportError instead of ValueError when GStreamer is unavailable, enabling `importorskip` usage in pytest setup.
-- dbcore: Allow models to declare SQL indices; add an `items.album_id` index to speed up `album.items()` queries. 🐛 (#5809)
This specifically fixes v2.6.0 changelog:
diff --git a/before b/after
index dc9a2ecd2..51303c65f 100644
--- a/before
+++ b/after
@@ -51,3 +51,3 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
-- Added a reusable requests handler which can be used by plugins to make HTTP requests with built-in retry and backoff logic. It uses beets user-agent and configures timeouts. See `~beetsplug._utils.requests.RequestHandler` for documentation.
+- Added a reusable requests handler which can be used by plugins to make HTTP requests with built-in retry and backoff logic. It uses beets user-agent and configures timeouts. See [beetsplug.\_utils.requests.RequestHandler](https://beets.readthedocs.io/en/stable/api/generated/beetsplug._utils.requests.RequestHandler.html#beetsplug._utils.requests.RequestHandler) for documentation.
@@ -62,3 +62,3 @@ Beets now requires Python 3.10 or later since support for EOL Python 3.9 has bee
- See `~beetsplug._utils.musicbrainz.MusicBrainzAPI` for documentation.
+ See [beetsplug.\_utils.musicbrainz.MusicBrainzAPI](https://beets.readthedocs.io/en/stable/api/generated/beetsplug._utils.musicbrainz.MusicBrainzAPI.html#beetsplug._utils.musicbrainz.MusicBrainzAPI) for documentation.
Fixes a bug where existing tags were set to None, if they weren't whitelisted, but an whitelisted canonicalized parent existed up the tree.
In all other cases, the original genres are canonicalized and considered for the final genre, except in the keep_existing logic branch.
This PR fixes the issue and results in the expected behavior for this combination of options.
For the bug to trigger several conditions had to be met:
- Canonicalization is enabled and a whitelist is specified.
- `force` and `keep_existing` are set. Meaning, that Lastfm is queried for a genre, but the existing genres are still left around when none are found online.
- A release with a non-whitelisted genre exists, but that genre has a whitelisted genre parent up the tree.
- That very release has no genre on lastfm.
This is rather convoluted, but stay with me :D
What would happen is the following:
- `keep_genres` is set to the existing genres, as `force` and `keep_existing` is set.
- Genres for `track`/`album`/`artist` aren't found for this release, as they don't exist in lastfm.
- Then the `keep_existing` logic is entered.
- The old logic only checks if the existing genres have an **exact** match for the whitelist. In contrast to all other code branches, we don't do the `_try_resolve_stage` in case there's no direct match, resulting in no match.
- We continue to the fallback logic, which returns the fallback (`None` in my case)
This patch results in one last try to resolve the existing genres when `keep_existing` is set, which includes canonicalization (if enabled).
