Skip to content

Commit

Permalink
add missing syspath conversions (1/3, tests)
Browse files Browse the repository at this point in the history
these are mostly in the tests, which didn't cause issues since the
affected directories usually have nice ASCII paths. For consistency, it
is nicer to always invoke syspath. That also avoids deprecation warnings
for the bytestring interfaces on Python <= 3.5. The bytestring
interfaces were undeprecated with PEP 529 in Python 3.6, such that we
didn't observe any actual failures.
  • Loading branch information
wisp3rwind committed Jun 24, 2023
1 parent 854fec2 commit 446dff0
Show file tree
Hide file tree
Showing 18 changed files with 220 additions and 180 deletions.
20 changes: 9 additions & 11 deletions test/_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from beets import importer, logging # noqa: E402
from beets.ui import commands # noqa: E402
from beets import util # noqa: E402
from beets.util import bytestring_path, syspath # noqa: E402
import beets # noqa: E402

# Make sure the development versions of the plugins are used
Expand Down Expand Up @@ -140,11 +141,11 @@ class Assertions:
"""A mixin with additional unit test assertions."""

def assertExists(self, path): # noqa
self.assertTrue(os.path.exists(util.syspath(path)),
self.assertTrue(os.path.exists(syspath(path)),
f'file does not exist: {path!r}')

def assertNotExists(self, path): # noqa
self.assertFalse(os.path.exists(util.syspath(path)),
self.assertFalse(os.path.exists(syspath(path)),
f'file exists: {path!r}')

def assert_equal_path(self, a, b):
Expand Down Expand Up @@ -186,8 +187,8 @@ def setUp(self):
self.io = DummyIO()

def tearDown(self):
if os.path.isdir(self.temp_dir):
shutil.rmtree(self.temp_dir)
if os.path.isdir(syspath(self.temp_dir)):
shutil.rmtree(syspath(self.temp_dir))
if self._old_home is None:
del os.environ['HOME']
else:
Expand Down Expand Up @@ -325,7 +326,7 @@ def restore(self):
# Utility.

def touch(path):
open(path, 'a').close()
open(syspath(path), 'a').close()


class Bag:
Expand All @@ -351,16 +352,13 @@ def create_temp_dir(self):
"""Create a temporary directory and assign it into `self.temp_dir`.
Call `remove_temp_dir` later to delete it.
"""
path = tempfile.mkdtemp()
if not isinstance(path, bytes):
path = path.encode('utf8')
self.temp_dir = path
self.temp_dir = bytestring_path(tempfile.mkdtemp())

def remove_temp_dir(self):
"""Delete the temporary directory created by `create_temp_dir`.
"""
if os.path.isdir(self.temp_dir):
shutil.rmtree(self.temp_dir)
if os.path.isdir(syspath(self.temp_dir)):
shutil.rmtree(syspath(self.temp_dir))


# Platform mocking.
Expand Down
29 changes: 15 additions & 14 deletions test/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
from beets.autotag.hooks import AlbumInfo, TrackInfo
from mediafile import MediaFile, Image
from beets import util
from beets.util import MoveOperation
from beets.util import MoveOperation, syspath, bytestring_path

# TODO Move AutotagMock here
from test import _common
Expand Down Expand Up @@ -181,7 +181,7 @@ def setup_beets(self, disk=False):
self.config['threaded'] = False

self.libdir = os.path.join(self.temp_dir, b'libdir')
os.mkdir(self.libdir)
os.mkdir(syspath(self.libdir))
self.config['directory'] = util.py3_path(self.libdir)

if disk:
Expand Down Expand Up @@ -242,17 +242,17 @@ def create_importer(self, item_count=1, album_count=1):
`self.temp_dir` and creates a `ImportSessionFixture` for this path.
"""
import_dir = os.path.join(self.temp_dir, b'import')
if not os.path.isdir(import_dir):
os.mkdir(import_dir)
if not os.path.isdir(syspath(import_dir)):
os.mkdir(syspath(import_dir))

album_no = 0
while album_count:
album = util.bytestring_path(f'album {album_no}')
album_dir = os.path.join(import_dir, album)
if os.path.exists(album_dir):
if os.path.exists(syspath(album_dir)):
album_no += 1
continue
os.mkdir(album_dir)
os.mkdir(syspath(album_dir))
album_count -= 1

track_no = 0
Expand All @@ -262,11 +262,11 @@ def create_importer(self, item_count=1, album_count=1):
src = os.path.join(_common.RSRC, b'full.mp3')
title_file = util.bytestring_path(f'{title}.mp3')
dest = os.path.join(album_dir, title_file)
if os.path.exists(dest):
if os.path.exists(syspath(dest)):
track_no += 1
continue
album_item_count -= 1
shutil.copy(src, dest)
shutil.copy(syspath(src), syspath(dest))
mediafile = MediaFile(dest)
mediafile.update({
'artist': 'artist',
Expand Down Expand Up @@ -405,8 +405,9 @@ def create_mediafile_fixture(self, ext='mp3', images=[]):
"""
src = os.path.join(_common.RSRC, util.bytestring_path('full.' + ext))
handle, path = mkstemp()
path = bytestring_path(path)
os.close(handle)
shutil.copyfile(src, path)
shutil.copyfile(syspath(src), syspath(path))

if images:
mediafile = MediaFile(path)
Expand All @@ -428,7 +429,7 @@ def create_mediafile_fixture(self, ext='mp3', images=[]):
def remove_mediafile_fixtures(self):
if hasattr(self, '_mediafile_fixtures'):
for path in self._mediafile_fixtures:
os.remove(path)
os.remove(syspath(path))

def _get_item_count(self):
if not hasattr(self, '__item_count'):
Expand Down Expand Up @@ -467,7 +468,7 @@ def create_temp_dir(self):
def remove_temp_dir(self):
"""Delete the temporary directory created by `create_temp_dir`.
"""
shutil.rmtree(self.temp_dir)
shutil.rmtree(syspath(self.temp_dir))

def touch(self, path, dir=None, content=''):
"""Create a file at `path` with given content.
Expand All @@ -483,10 +484,10 @@ def touch(self, path, dir=None, content=''):
path = os.path.join(self.temp_dir, path)

parent = os.path.dirname(path)
if not os.path.isdir(parent):
os.makedirs(util.syspath(parent))
if not os.path.isdir(syspath(parent)):
os.makedirs(syspath(parent))

with open(util.syspath(path), 'a+') as f:
with open(syspath(path), 'a+') as f:
f.write(content)
return path

Expand Down
16 changes: 10 additions & 6 deletions test/test_art.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from beets import importer
from beets import logging
from beets import util
from beets.util import syspath
from beets.util.artresizer import ArtResizer
import confuse

Expand Down Expand Up @@ -197,7 +198,7 @@ class FSArtTest(UseThePlugin):
def setUp(self):
super().setUp()
self.dpath = os.path.join(self.temp_dir, b'arttest')
os.mkdir(self.dpath)
os.mkdir(syspath(self.dpath))

self.source = fetchart.FileSystem(logger, self.plugin.config)
self.settings = Settings(cautious=False,
Expand Down Expand Up @@ -251,7 +252,7 @@ class CombinedTest(FetchImageHelper, UseThePlugin, CAAHelper):
def setUp(self):
super().setUp()
self.dpath = os.path.join(self.temp_dir, b'arttest')
os.mkdir(self.dpath)
os.mkdir(syspath(self.dpath))

def test_main_interface_returns_amazon_art(self):
self.mock_response(self.AMAZON_URL)
Expand Down Expand Up @@ -641,10 +642,13 @@ def art_for_album(i, p, local_only=False):
# Test library.
self.libpath = os.path.join(self.temp_dir, b'tmplib.blb')
self.libdir = os.path.join(self.temp_dir, b'tmplib')
os.mkdir(self.libdir)
os.mkdir(os.path.join(self.libdir, b'album'))
os.mkdir(syspath(self.libdir))
os.mkdir(syspath(os.path.join(self.libdir, b'album')))
itempath = os.path.join(self.libdir, b'album', b'test.mp3')
shutil.copyfile(os.path.join(_common.RSRC, b'full.mp3'), itempath)
shutil.copyfile(
syspath(os.path.join(_common.RSRC, b'full.mp3')),
syspath(itempath),
)
self.lib = library.Library(self.libpath)
self.i = _common.item()
self.i.path = itempath
Expand Down Expand Up @@ -716,7 +720,7 @@ def test_delete_original_file(self):

def test_do_not_delete_original_if_already_in_place(self):
artdest = os.path.join(os.path.dirname(self.i.path), b'cover.jpg')
shutil.copyfile(self.art_file, artdest)
shutil.copyfile(syspath(self.art_file), syspath(artdest))
self.afa_response = fetchart.Candidate(logger, path=artdest)
self._fetch_art(True)

Expand Down
21 changes: 11 additions & 10 deletions test/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from mediafile import MediaFile
from beets import util
from beets.util import bytestring_path, displayable_path, syspath


def shell_quote(text):
Expand Down Expand Up @@ -53,15 +54,15 @@ def assertFileTag(self, path, tag): # noqa
"""
display_tag = tag
tag = tag.encode('utf-8')
self.assertTrue(os.path.isfile(path),
self.assertTrue(os.path.isfile(syspath(path)),
'{} is not a file'.format(
util.displayable_path(path)))
displayable_path(path)))
with open(path, 'rb') as f:
f.seek(-len(display_tag), os.SEEK_END)
self.assertEqual(f.read(), tag,
'{} is not tagged with {}'
.format(
util.displayable_path(path),
displayable_path(path),
display_tag))

def assertNoFileTag(self, path, tag): # noqa
Expand All @@ -70,15 +71,15 @@ def assertNoFileTag(self, path, tag): # noqa
"""
display_tag = tag
tag = tag.encode('utf-8')
self.assertTrue(os.path.isfile(path),
self.assertTrue(os.path.isfile(syspath(path)),
'{} is not a file'.format(
util.displayable_path(path)))
displayable_path(path)))
with open(path, 'rb') as f:
f.seek(-len(tag), os.SEEK_END)
self.assertNotEqual(f.read(), tag,
'{} is unexpectedly tagged with {}'
.format(
util.displayable_path(path),
displayable_path(path),
display_tag))


Expand Down Expand Up @@ -117,7 +118,7 @@ def test_import_original_on_convert_error(self):

item = self.lib.items().get()
self.assertIsNotNone(item)
self.assertTrue(os.path.isfile(item.path))
self.assertTrue(os.path.isfile(syspath(item.path)))

def test_delete_originals(self):
self.config['convert']['delete_originals'] = True
Expand Down Expand Up @@ -166,7 +167,7 @@ def setUp(self):
self.item = self.album.items()[0]
self.load_plugins('convert')

self.convert_dest = util.bytestring_path(
self.convert_dest = bytestring_path(
os.path.join(self.temp_dir, b'convert_dest')
)
self.config['convert'] = {
Expand Down Expand Up @@ -202,7 +203,7 @@ def test_reject_confirmation(self):
with control_stdin('n'):
self.run_convert()
converted = os.path.join(self.convert_dest, b'converted.mp3')
self.assertFalse(os.path.isfile(converted))
self.assertFalse(os.path.isfile(syspath(converted)))

def test_convert_keep_new(self):
self.assertEqual(os.path.splitext(self.item.path)[1], b'.ogg')
Expand Down Expand Up @@ -243,7 +244,7 @@ def test_skip_existing(self):
def test_pretend(self):
self.run_convert('--pretend')
converted = os.path.join(self.convert_dest, b'converted.mp3')
self.assertFalse(os.path.exists(converted))
self.assertFalse(os.path.exists(syspath(converted)))

def test_empty_query(self):
with capture_log('beets.convert') as logs:
Expand Down
14 changes: 9 additions & 5 deletions test/test_embedart.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

from mediafile import MediaFile
from beets import config, logging, ui
from beets.util import syspath, displayable_path
from beets.util import bytestring_path, displayable_path, syspath
from beets.util.artresizer import ArtResizer
from beets import art
from test.test_art import FetchImageHelper
Expand Down Expand Up @@ -110,6 +110,7 @@ def test_embed_art_remove_art_file(self):
logging.getLogger('beets.embedart').setLevel(logging.DEBUG)

handle, tmp_path = tempfile.mkstemp()
tmp_path = bytestring_path(tmp_path)
os.write(handle, self.image_data)
os.close(handle)

Expand All @@ -119,9 +120,11 @@ def test_embed_art_remove_art_file(self):
config['embedart']['remove_art_file'] = True
self.run_command('embedart', '-y')

if os.path.isfile(tmp_path):
os.remove(tmp_path)
self.fail(f'Artwork file {tmp_path} was not deleted')
if os.path.isfile(syspath(tmp_path)):
os.remove(syspath(tmp_path))
self.fail('Artwork file {} was not deleted'.format(
displayable_path(tmp_path)
))

def test_art_file_missing(self):
self.add_album_fixture()
Expand All @@ -134,13 +137,14 @@ def test_embed_non_image_file(self):
logging.getLogger('beets.embedart').setLevel(logging.DEBUG)

handle, tmp_path = tempfile.mkstemp()
tmp_path = bytestring_path(tmp_path)
os.write(handle, b'I am not an image.')
os.close(handle)

try:
self.run_command('embedart', '-y', '-f', tmp_path)
finally:
os.remove(tmp_path)
os.remove(syspath(tmp_path))

mediafile = MediaFile(syspath(album.items()[0].path))
self.assertFalse(mediafile.images) # No image added.
Expand Down
12 changes: 6 additions & 6 deletions test/test_filefilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from test.test_importer import ImportHelper
from beets import config
from mediafile import MediaFile
from beets.util import displayable_path, bytestring_path
from beets.util import displayable_path, bytestring_path, syspath
from beetsplug.filefilter import FileFilterPlugin


Expand All @@ -42,7 +42,7 @@ def tearDown(self):
def __copy_file(self, dest_path, metadata):
# Copy files
resource_path = os.path.join(_common.RSRC, b'full.mp3')
shutil.copy(resource_path, dest_path)
shutil.copy(syspath(resource_path), syspath(dest_path))
medium = MediaFile(dest_path)
# Set metadata
for attr in metadata:
Expand All @@ -51,14 +51,14 @@ def __copy_file(self, dest_path, metadata):

def __create_import_dir(self, count):
self.import_dir = os.path.join(self.temp_dir, b'testsrcdir')
if os.path.isdir(self.import_dir):
shutil.rmtree(self.import_dir)
if os.path.isdir(syspath(self.import_dir)):
shutil.rmtree(syspath(self.import_dir))

self.artist_path = os.path.join(self.import_dir, b'artist')
self.album_path = os.path.join(self.artist_path, b'album')
self.misc_path = os.path.join(self.import_dir, b'misc')
os.makedirs(self.album_path)
os.makedirs(self.misc_path)
os.makedirs(syspath(self.album_path))
os.makedirs(syspath(self.misc_path))

metadata = {
'artist': 'Tag Artist',
Expand Down
Loading

0 comments on commit 446dff0

Please sign in to comment.