use pathlib in tests: migrate most tests to pathlib - #6865
Open
snejus wants to merge 5 commits into
Open
Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
❌ 6 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
There was a problem hiding this comment.
Pull request overview
PR move big chunk of test suite from mixed bytes/str paths toward pathlib.Path, so tests use one path model and prefer item.filepath / album.art_filepath. grug like less path soup, but grug see few spots where test logic break or helper removal break other tests.
Changes:
- Update many tests to build paths with
Pathops, and use higher-levelfilepath/art_filepathAPIs. - Update test helper(s) to return
Path(not bytes) for media fixtures; remove some old “touch” helpers. - Small prod util tweak:
beets.util.mkdirallnow acceptsPath.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/util/test_m3ufile.py | Switch temp playlist paths to Path and PathsMixin. |
| test/util/test_hidden.py | Stop forcing bytes paths when calling hidden.is_hidden. |
| test/ui/test_ui.py | Use Path for config file paths; use Path.open/write_text. |
| test/ui/commands/test_update.py | Use item.filepath.stat() for mtime check. |
| test/ui/commands/test_remove.py | Use filepath.exists() and filepath vars instead of byte path. |
| test/ui/commands/test_move.py | Use Path.unlink() and compare against filepath. |
| test/ui/commands/test_config.py | Write YAML config via Path.write_text. |
| test/ui/commands/test_completion.py | Use Path.exists() / Path.open() for completion script. |
| test/testall.py | Remove old path-hacking test runner helper. |
| test/test_util.py | Use Path for path construction in syspath tests. |
| test/test_library.py | Use Path in a few path-oriented tests; switch to filepath.parent, etc. |
| test/test_importer.py | Use Path.mkdir() / Path.write_bytes() in importer tests. |
| test/test_files.py | Migrate many filesystem ops to Path methods and filepath/art_filepath. |
| test/rsrc/convert_stub.py | Use Path.open() and os.fsencode for tag bytes. |
| test/plugins/test_web.py | Try to migrate stored Item/Album paths to Path and simplify expected strings. |
| test/plugins/test_thumbnails.py | Use temp_path and Path.read_text() for dolphin .directory check. |
| test/plugins/test_smartplaylist.py | Use Path.unlink() for playlist cleanup. |
| test/plugins/test_scrub.py | Pass item.filepath into MediaFile. |
| test/plugins/test_ipfs.py | Build expected IPFS paths with Path and compare via filepath. |
| test/plugins/test_info.py | Pass CLI file args as str(Path); assert output includes decoded path. |
| test/plugins/test_importsource.py | Use Path.stat() for mtime checks. |
| test/plugins/test_importfeeds.py | Build configured playlist names/paths via Path. |
| test/plugins/test_importadded.py | Type annotate helper, use Path.stat() / filepath in assertions. |
| test/plugins/test_hook.py | Use Path(...).is_file() for file existence assertion. |
| test/plugins/test_filefilter.py | Compare expected item paths using Item.filepath. |
| test/plugins/test_fetchart.py | Use album.filepath / album.art_filepath and Path.write_text() in tests. |
| test/plugins/test_embedart.py | Use Path temp files + Path.read_bytes(); use item.filepath.stat() for mtimes. |
| test/plugins/test_duplicates.py | Assert output contains str(item.filepath) instead of item.path.decode(). |
| test/plugins/test_convert.py | Use Path.write_text() / Path.read_text() for converted file check. |
| test/plugins/test_aura.py | Use item.filepath.stat() for size. |
| test/plugins/test_art.py | Small Path conversion in one test around cover art destination. |
| test/library/test_migrations.py | Use Path for relative path construction and directory iteration. |
| test/dbcore/test_db.py | Use Path for temp db file paths; use Path.unlink(). |
| beets/util/init.py | Allow mkdirall to accept a Path. |
| beets/test/helper.py | create_mediafile_fixture now returns Path; remove old touch helper. |
| beets/test/_common.py | Remove old _common.touch helper. |
Comments suppressed due to low confidence (2)
test/test_files.py:502
- grug see test end after
self.i.remove(True)but then callutil.mkdirall(self.i.filepath). that create dirs again, so test stop checking prune behavior. should assert album dir gone like other tests.
self.ai.set_art(artfile)
self.ai.store()
self.i.remove(True)
util.mkdirall(self.i.filepath)
test/plugins/test_art.py:937
- grug see fetchart.Candidate wants
path: bytes, but code pass Pathartdest. this break (bytes contract). issue also say this file excluded from pathlib refactor. keep bytes here: pass os.fsencode(artdest) (or bytestring_path).
def test_do_not_delete_original_if_already_in_place(self):
artdest = self.i.filepath.parent / "cover.jpg"
shutil.copyfile(self.art_file, syspath(artdest))
self.afa_response = fetchart.Candidate(
logger, source_name="test", path=artdest
)
snejus
force-pushed
the
make-rsrc-pathlib-path
branch
from
July 24, 2026 16:04
bfd3b7e to
02a2c57
Compare
snejus
force-pushed
the
migrate-tests-to-pathlib
branch
3 times, most recently
from
July 27, 2026 19:34
06d9ca6 to
29e1096
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
test/plugins/test_art.py:937
- grug see fetchart.Candidate want path: bytes (see fetchart.Candidate init). test pass Path, so later bytes ops blow up. keep bytes here (this file even in scope exception list).
artdest = self.i.filepath.parent / "cover.jpg"
shutil.copyfile(self.art_file, syspath(artdest))
self.afa_response = fetchart.Candidate(
logger, source_name="test", path=artdest
)
|
|
||
| self.i.remove(True) | ||
| assert not self.i.filepath.parent.exists() | ||
| util.mkdirall(self.i.filepath) |
| self.config["create_backup_before_migrations"] = config_value | ||
| self.add_item(lyrics="some lyrics") | ||
| db_path = self.lib.path | ||
| db_path = self.lib_path |
| out_f.write(tag) | ||
| with Path(out_file).open("wb") as out_f, Path(in_file).open("rb") as in_f: | ||
| out_f.write(in_f.read()) | ||
| out_f.write(os.fsencode(tag)) |
snejus
force-pushed
the
migrate-tests-to-pathlib
branch
from
July 28, 2026 01:50
29e1096 to
aa6c306
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR continues the test suite's move from mixed string/bytes path handling to
pathlib.Path-based filesystem usage.The main architectural change is that shared test helpers now produce
Pathobjects directly, especiallyTestHelper.create_mediafile_fixture, so tests can use one consistent path model end-to-end.Tests are updated to prefer higher-level path APIs like
item.filepathandalbum.art_filepathinstead of rawitem.pathbytes, reducing manual conversions and directos.pathusage.Supporting utilities were adjusted where needed, such as allowing
beets.util.mkdirallto acceptPathinputs, so existing filesystem helpers still work during the transition.High-level impact:
In practice, this is mostly a test infrastructure cleanup and API alignment change. It should not change product behavior, but it makes future path-related test updates simpler and safer.
Fixes #6807
The rest of tests require refactoring of associated source code.