diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 18ca9b9e4..daf377d94 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -347,7 +347,6 @@ others. See `unittest.mock`_ for more info. ``mock.patch``, as they require manual cleanup. Use the annotation or context manager forms instead. -.. _Python unittest: https://docs.python.org/2/library/unittest.html .. _Codecov: https://codecov.io/github/beetbox/beets .. _pytest-random: https://github.com/klrmn/pytest-random .. _tox: https://tox.readthedocs.io/en/latest/ @@ -358,10 +357,9 @@ others. See `unittest.mock`_ for more info. .. _`https://github.com/beetbox/beets/blob/master/setup.py#L99`: https://github.com/beetbox/beets/blob/master/setup.py#L99 .. _test: https://github.com/beetbox/beets/tree/master/test .. _`https://github.com/beetbox/beets/blob/master/test/test_template.py#L224`: https://github.com/beetbox/beets/blob/master/test/test_template.py#L224 -.. _unittest: https://docs.python.org/3.8/library/unittest.html +.. _unittest: https://docs.python.org/3/library/unittest.html .. _integration test: https://github.com/beetbox/beets/actions?query=workflow%3A%22integration+tests%22 .. _unittest.mock: https://docs.python.org/3/library/unittest.mock.html -.. _Python unittest: https://docs.python.org/2/library/unittest.html .. _documentation: https://beets.readthedocs.io/en/stable/ .. _pip: https://pip.pypa.io/en/stable/ .. _vim: https://www.vim.org/ diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 30904ff29..8bd87d84a 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -28,12 +28,6 @@ from unidecode import unidecode log = logging.getLogger('beets') -# The name of the type for patterns in re changed in Python 3.7. -try: - Pattern = re._pattern_type -except AttributeError: - Pattern = re.Pattern - # Classes used to represent candidate options. class AttrDict(dict): @@ -449,7 +443,7 @@ class Distance: be a compiled regular expression, in which case it will be matched against `value2`. """ - if isinstance(value1, Pattern): + if isinstance(value1, re.Pattern): return bool(value1.match(value2)) return value1 == value2 diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index b0c769790..a0d79da70 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -526,19 +526,6 @@ class FalseQuery(Query): # Time/date queries. -def _to_epoch_time(date): - """Convert a `datetime` object to an integer number of seconds since - the (local) Unix epoch. - """ - if hasattr(date, 'timestamp'): - # The `timestamp` method exists on Python 3.3+. - return int(date.timestamp()) - else: - epoch = datetime.fromtimestamp(0) - delta = date - epoch - return int(delta.total_seconds()) - - def _parse_periods(pattern): """Parse a string containing two dates separated by two dots (..). Return a pair of `Period` objects. @@ -724,13 +711,15 @@ class DateQuery(FieldQuery): clause_parts = [] subvals = [] + # Convert the `datetime` objects to an integer number of seconds since + # the (local) Unix epoch using `datetime.timestamp()`. if self.interval.start: clause_parts.append(self._clause_tmpl.format(self.field, ">=")) - subvals.append(_to_epoch_time(self.interval.start)) + subvals.append(int(self.interval.start.timestamp())) if self.interval.end: clause_parts.append(self._clause_tmpl.format(self.field, "<")) - subvals.append(_to_epoch_time(self.interval.end)) + subvals.append(int(self.interval.end.timestamp())) if clause_parts: # One- or two-sided interval. diff --git a/beets/library.py b/beets/library.py index d62403cbf..c05dcda18 100644 --- a/beets/library.py +++ b/beets/library.py @@ -51,6 +51,8 @@ class PathQuery(dbcore.FieldQuery): default, the behavior depends on the OS: case-insensitive on Windows and case-sensitive otherwise. """ + # For tests + force_implicit_query_detection = False def __init__(self, field, pattern, fast=True, case_sensitive=None): """Create a path query. @@ -62,21 +64,27 @@ class PathQuery(dbcore.FieldQuery): """ super().__init__(field, pattern, fast) + path = util.normpath(pattern) + # By default, the case sensitivity depends on the filesystem # that the query path is located on. if case_sensitive is None: - path = util.bytestring_path(util.normpath(pattern)) - case_sensitive = beets.util.case_sensitive(path) + case_sensitive = util.case_sensitive(path) self.case_sensitive = case_sensitive # Use a normalized-case pattern for case-insensitive matches. if not case_sensitive: - pattern = pattern.lower() + # We need to lowercase the entire path, not just the pattern. + # In particular, on Windows, the drive letter is otherwise not + # lowercased. + # This also ensures that the `match()` method below and the SQL + # from `col_clause()` do the same thing. + path = path.lower() # Match the path as a single file. - self.file_path = util.bytestring_path(util.normpath(pattern)) + self.file_path = path # As a directory (prefix). - self.dir_path = util.bytestring_path(os.path.join(self.file_path, b'')) + self.dir_path = os.path.join(path, b'') @classmethod def is_path_query(cls, query_part): @@ -90,11 +98,13 @@ class PathQuery(dbcore.FieldQuery): # Test both `sep` and `altsep` (i.e., both slash and backslash on # Windows). - return ( - (os.sep in query_part or - (os.altsep and os.altsep in query_part)) and - os.path.exists(syspath(normpath(query_part))) - ) + if not (os.sep in query_part + or (os.altsep and os.altsep in query_part)): + return False + + if cls.force_implicit_query_detection: + return True + return os.path.exists(syspath(normpath(query_part))) def match(self, item): path = item.path if self.case_sensitive else item.path.lower() @@ -300,34 +310,26 @@ class FileOperationError(Exception): self.path = path self.reason = reason - def text(self): + def __str__(self): """Get a string representing the error. - Describe both the underlying reason and the file path - in question. + Describe both the underlying reason and the file path in question. """ - return '{}: {}'.format( - util.displayable_path(self.path), - str(self.reason) - ) - - # define __str__ as text to avoid infinite loop on super() calls - # with @six.python_2_unicode_compatible - __str__ = text + return f"{util.displayable_path(self.path)}: {self.reason}" class ReadError(FileOperationError): """An error while reading a file (i.e. in `Item.read`).""" def __str__(self): - return 'error reading ' + super().text() + return 'error reading ' + str(super()) class WriteError(FileOperationError): """An error while writing a file (i.e. in `Item.write`).""" def __str__(self): - return 'error writing ' + super().text() + return 'error writing ' + str(super()) # Item and Album model classes. diff --git a/beets/logging.py b/beets/logging.py index 516528c05..05c22bd1c 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -12,63 +12,51 @@ # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. -"""A drop-in replacement for the standard-library `logging` module that -allows {}-style log formatting on Python 2 and 3. +"""A drop-in replacement for the standard-library `logging` module. -Provides everything the "logging" module does. The only difference is -that when getLogger(name) instantiates a logger that logger uses -{}-style formatting. +Provides everything the "logging" module does. In addition, beets' logger +(as obtained by `getLogger(name)`) supports thread-local levels, and messages +use {}-style formatting and can interpolate keywords arguments to the logging +calls (`debug`, `info`, etc). """ from copy import copy -import subprocess +import sys import threading import logging def logsafe(val): - """Coerce a potentially "problematic" value so it can be formatted - in a Unicode log string. + """Coerce `bytes` to `str` to avoid crashes solely due to logging. - This works around a number of pitfalls when logging objects in - Python 2: - - Logging path names, which must be byte strings, requires - conversion for output. - - Some objects, including some exceptions, will crash when you call - `unicode(v)` while `str(v)` works fine. CalledProcessError is an - example. + This is particularly relevant for bytestring paths. Much of our code + explicitly uses `displayable_path` for them, but better be safe and prevent + any crashes that are solely due to log formatting. """ - # Already Unicode. - if isinstance(val, str): - return val - - # Bytestring: needs decoding. - elif isinstance(val, bytes): + # Bytestring: Needs decoding to be safe for substitution in format strings. + if isinstance(val, bytes): # Blindly convert with UTF-8. Eventually, it would be nice to # (a) only do this for paths, if they can be given a distinct # type, and (b) warn the developer if they do this for other # bytestrings. return val.decode('utf-8', 'replace') - # A "problem" object: needs a workaround. - elif isinstance(val, subprocess.CalledProcessError): - try: - return str(val) - except UnicodeDecodeError: - # An object with a broken __unicode__ formatter. Use __str__ - # instead. - return str(val).decode('utf-8', 'replace') - # Other objects are used as-is so field access, etc., still works in - # the format string. - else: - return val + # the format string. Relies on a working __str__ implementation. + return val class StrFormatLogger(logging.Logger): """A version of `Logger` that uses `str.format`-style formatting - instead of %-style formatting. + instead of %-style formatting and supports keyword arguments. + + We cannot easily get rid of this even in the Python 3 era: This custom + formatting supports substitution from `kwargs` into the message, which the + default `logging.Logger._log()` implementation does not. + + Remark by @sampsyo: https://stackoverflow.com/a/24683360 might be a way to + achieve this with less code. """ class _LogMessage: @@ -82,10 +70,28 @@ class StrFormatLogger(logging.Logger): kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()} return self.msg.format(*args, **kwargs) - def _log(self, level, msg, args, exc_info=None, extra=None, **kwargs): + def _log(self, level, msg, args, exc_info=None, extra=None, + stack_info=False, **kwargs): """Log msg.format(*args, **kwargs)""" m = self._LogMessage(msg, args, kwargs) - return super()._log(level, m, (), exc_info, extra) + + stacklevel = kwargs.pop("stacklevel", 1) + if sys.version_info >= (3, 8): + stacklevel = {"stacklevel": stacklevel} + else: + # Simply ignore this when not supported by current Python version. + # Can be dropped when we remove support for Python 3.7. + stacklevel = {} + + return super()._log( + level, + m, + (), + exc_info=exc_info, + extra=extra, + stack_info=stack_info, + **stacklevel, + ) class ThreadLocalLevelLogger(logging.Logger): diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index ba058148d..cd9f4989e 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -790,9 +790,6 @@ def _store_dict(option, opt_str, value, parser): setattr(parser.values, dest, {}) option_values = getattr(parser.values, dest) - # Decode the argument using the platform's argument encoding. - value = util.text_string(value, util.arg_encoding()) - try: key, value = value.split('=', 1) if not (key and value): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 91cee4516..6c8e25b85 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1778,7 +1778,7 @@ def config_func(lib, opts, args): else: config_out = config.dump(full=opts.defaults, redact=opts.redact) if config_out.strip() != '{}': - print_(util.text_string(config_out)) + print_(config_out) else: print("Empty configuration") @@ -1852,7 +1852,7 @@ def completion_script(commands): """ base_script = os.path.join(os.path.dirname(__file__), 'completion_base.sh') with open(base_script) as base_script: - yield util.text_string(base_script.read()) + yield base_script.read() options = {} aliases = {} diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 06e02ee08..2319890a3 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -532,10 +532,6 @@ def link(path, dest, replace=False): raise FilesystemError('OS does not support symbolic links.' 'link', (path, dest), traceback.format_exc()) except OSError as exc: - # TODO: Windows version checks can be removed for python 3 - if hasattr('sys', 'getwindowsversion'): - if sys.getwindowsversion()[0] < 6: # is before Vista - exc = 'OS does not support symbolic links.' raise FilesystemError(exc, 'link', (path, dest), traceback.format_exc()) @@ -741,8 +737,7 @@ def legalize_path(path, replacements, length, extension, fragment): def py3_path(path): - """Convert a bytestring path to Unicode on Python 3 only. On Python - 2, return the bytestring path unchanged. + """Convert a bytestring path to Unicode. This helps deal with APIs on Python 3 that *only* accept Unicode (i.e., `str` objects). I philosophically disagree with this @@ -775,19 +770,6 @@ def as_string(value): return str(value) -def text_string(value, encoding='utf-8'): - """Convert a string, which can either be bytes or unicode, to - unicode. - - Text (unicode) is left untouched; bytes are decoded. This is useful - to convert from a "native string" (bytes on Python 2, str on Python - 3) to a consistently unicode value. - """ - if isinstance(value, bytes): - return value.decode(encoding) - return value - - def plurality(objs): """Given a sequence of hashble objects, returns the object that is most common in the set and the its number of appearance. The @@ -868,10 +850,7 @@ def command_output(cmd, shell=False): """ cmd = convert_command_args(cmd) - try: # python >= 3.3 - devnull = subprocess.DEVNULL - except AttributeError: - devnull = open(os.devnull, 'r+b') + devnull = subprocess.DEVNULL proc = subprocess.Popen( cmd, @@ -957,61 +936,52 @@ def interactive_open(targets, command): return os.execlp(*args) -def _windows_long_path_name(short_path): - """Use Windows' `GetLongPathNameW` via ctypes to get the canonical, - long path given a short filename. - """ - if not isinstance(short_path, str): - short_path = short_path.decode(_fsencoding()) - - import ctypes - buf = ctypes.create_unicode_buffer(260) - get_long_path_name_w = ctypes.windll.kernel32.GetLongPathNameW - return_value = get_long_path_name_w(short_path, buf, 260) - - if return_value == 0 or return_value > 260: - # An error occurred - return short_path - else: - long_path = buf.value - # GetLongPathNameW does not change the case of the drive - # letter. - if len(long_path) > 1 and long_path[1] == ':': - long_path = long_path[0].upper() + long_path[1:] - return long_path - - def case_sensitive(path): """Check whether the filesystem at the given path is case sensitive. To work best, the path should point to a file or a directory. If the path does not exist, assume a case sensitive file system on every platform except Windows. + + Currently only used for absolute paths by beets; may have a trailing + path separator. """ - # A fallback in case the path does not exist. - if not os.path.exists(syspath(path)): - # By default, the case sensitivity depends on the platform. - return platform.system() != 'Windows' + # Look at parent paths until we find a path that actually exists, or + # reach the root. + while True: + head, tail = os.path.split(path) + if head == path: + # We have reached the root of the file system. + # By default, the case sensitivity depends on the platform. + return platform.system() != 'Windows' - # If an upper-case version of the path exists but a lower-case - # version does not, then the filesystem must be case-sensitive. - # (Otherwise, we have more work to do.) - if not (os.path.exists(syspath(path.lower())) and - os.path.exists(syspath(path.upper()))): - return True + # Trailing path separator, or path does not exist. + if not tail or not os.path.exists(path): + path = head + continue - # Both versions of the path exist on the file system. Check whether - # they refer to different files by their inodes. Alas, - # `os.path.samefile` is only available on Unix systems on Python 2. - if platform.system() != 'Windows': - return not os.path.samefile(syspath(path.lower()), - syspath(path.upper())) + upper_tail = tail.upper() + lower_tail = tail.lower() - # On Windows, we check whether the canonical, long filenames for the - # files are the same. - lower = _windows_long_path_name(path.lower()) - upper = _windows_long_path_name(path.upper()) - return lower != upper + # In case we can't tell from the given path name, look at the + # parent directory. + if upper_tail == lower_tail: + path = head + continue + + upper_sys = syspath(os.path.join(head, upper_tail)) + lower_sys = syspath(os.path.join(head, lower_tail)) + + # If either the upper-cased or lower-cased path does not exist, the + # filesystem must be case-sensitive. + # (Otherwise, we have more work to do.) + if not os.path.exists(upper_sys) or not os.path.exists(lower_sys): + return True + + # Original and both upper- and lower-cased versions of the path + # exist on the file system. Check whether they refer to different + # files by their inodes (or an alternative method on Windows). + return not os.path.samefile(lower_sys, upper_sys) def raw_seconds_short(string): @@ -1054,8 +1024,7 @@ def asciify_path(path, sep_replace): def par_map(transform, items): """Apply the function `transform` to all the elements in the iterable `items`, like `map(transform, items)` but with no return - value. The map *might* happen in parallel: it's parallel on Python 3 - and sequential on Python 2. + value. The parallelism uses threads (not processes), so this is only useful for IO-bound `transform`s. diff --git a/beets/util/functemplate.py b/beets/util/functemplate.py index 289a436de..809207b9a 100644 --- a/beets/util/functemplate.py +++ b/beets/util/functemplate.py @@ -530,18 +530,7 @@ def _parse(template): return Expression(parts) -def cached(func): - """Like the `functools.lru_cache` decorator, but works (as a no-op) - on Python < 3.2. - """ - if hasattr(functools, 'lru_cache'): - return functools.lru_cache(maxsize=128)(func) - else: - # Do nothing when lru_cache is not available. - return func - - -@cached +@functools.lru_cache(maxsize=128) def template(fmt): return Template(fmt) diff --git a/beetsplug/hook.py b/beetsplug/hook.py index 0fe3bffc6..8c7211450 100644 --- a/beetsplug/hook.py +++ b/beetsplug/hook.py @@ -26,39 +26,20 @@ class CodingFormatter(string.Formatter): """A variant of `string.Formatter` that converts everything to `unicode` strings. - This is necessary on Python 2, where formatting otherwise occurs on - bytestrings. It intercepts two points in the formatting process to decode - the format string and all fields using the specified encoding. If decoding - fails, the values are used as-is. + This was necessary on Python 2, in needs to be kept for backwards + compatibility. """ def __init__(self, coding): """Creates a new coding formatter with the provided coding.""" self._coding = coding - def format(self, format_string, *args, **kwargs): - """Formats the provided string using the provided arguments and keyword - arguments. - - This method decodes the format string using the formatter's coding. - - See str.format and string.Formatter.format. - """ - if isinstance(format_string, bytes): - format_string = format_string.decode(self._coding) - - return super().format(format_string, *args, - **kwargs) - def convert_field(self, value, conversion): """Converts the provided value given a conversion type. This method decodes the converted value using the formatter's coding. - - See string.Formatter.convert_field. """ - converted = super().convert_field(value, - conversion) + converted = super().convert_field(value, conversion) if isinstance(converted, bytes): return converted.decode(self._coding) @@ -92,14 +73,13 @@ class HookPlugin(BeetsPlugin): self._log.error('invalid command "{0}"', command) return - # Use a string formatter that works on Unicode strings. + # For backwards compatibility, use a string formatter that decodes + # bytes (in particular, paths) to unicode strings. formatter = CodingFormatter(arg_encoding()) - - command_pieces = shlex.split(command) - - for i, piece in enumerate(command_pieces): - command_pieces[i] = formatter.format(piece, event=event, - **kwargs) + command_pieces = [ + formatter.format(piece, event=event, **kwargs) + for piece in shlex.split(command) + ] self._log.debug('running command "{0}" for event {1}', ' '.join(command_pieces), event) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index b695ab54e..412b7ddaa 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -67,12 +67,6 @@ class KeyFinderPlugin(BeetsPlugin): except (subprocess.CalledProcessError, OSError) as exc: self._log.error('execution failed: {0}', exc) continue - except UnicodeEncodeError: - # Workaround for Python 2 Windows bug. - # https://bugs.python.org/issue1759845 - self._log.error('execution failed for Unicode path: {0!r}', - item.path) - continue try: key_raw = output.rsplit(None, 1)[-1] @@ -83,7 +77,7 @@ class KeyFinderPlugin(BeetsPlugin): continue try: - key = util.text_string(key_raw) + key = key_raw.decode("utf-8") except UnicodeDecodeError: self._log.error('output is invalid UTF-8') continue diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index 5693353c0..2bed2542b 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -24,12 +24,7 @@ from beets.library import Item, Album, parse_query_string from beets.dbcore import OrQuery from beets.dbcore.query import MultipleSort, ParsingError import os - -try: - from urllib.request import pathname2url -except ImportError: - # python2 is a bit different - from urllib import pathname2url +from urllib.request import pathname2url class SmartPlaylistPlugin(BeetsPlugin): diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index b7baa93c1..7b04f3714 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -307,19 +307,24 @@ def item_file(item_id): else: item_path = util.py3_path(item.path) - try: - unicode_item_path = util.text_string(item.path) - except (UnicodeDecodeError, UnicodeEncodeError): - unicode_item_path = util.displayable_path(item.path) + base_filename = os.path.basename(item_path) + # FIXME: Arguably, this should just use `displayable_path`: The latter + # tries `_fsencoding()` first, but then falls back to `utf-8`, too. + if isinstance(base_filename, bytes): + try: + unicode_base_filename = base_filename.decode("utf-8") + except UnicodeError: + unicode_base_filename = util.displayable_path(base_filename) + else: + unicode_base_filename = base_filename - base_filename = os.path.basename(unicode_item_path) try: # Imitate http.server behaviour base_filename.encode("latin-1", "strict") - except UnicodeEncodeError: + except UnicodeError: safe_filename = unidecode(base_filename) else: - safe_filename = base_filename + safe_filename = unicode_base_filename response = flask.send_file( item_path, diff --git a/docs/changelog.rst b/docs/changelog.rst index 918508f7c..3fe976ee6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,9 @@ Changelog Changelog goes here! +With this release, beets now requires Python 3.7 or later (it removes support +for Python 3.6). + New features: * We now import the remixer field from Musicbrainz into the library. @@ -145,6 +148,7 @@ Bug fixes: For packagers: +* As noted above, the minimum Python version is now 3.7. * We fixed a version for the dependency on the `Confuse`_ library. :bug:`4167` * The minimum required version of :pypi:`mediafile` is now 0.9.0. diff --git a/docs/guides/main.rst b/docs/guides/main.rst index 2b573ac32..6169ded8c 100644 --- a/docs/guides/main.rst +++ b/docs/guides/main.rst @@ -10,7 +10,7 @@ Installing ---------- You will need Python. -Beets works on Python 3.6 or later. +Beets works on Python 3.7 or later. * **macOS** 11 (Big Sur) includes Python 3.8 out of the box. You can opt for a more recent Python installing it via `Homebrew`_ @@ -94,7 +94,7 @@ Installing on Windows Installing beets on Windows can be tricky. Following these steps might help you get it right: -1. If you don't have it, `install Python`_ (you want at least Python 3.6). The +1. If you don't have it, `install Python`_ (you want at least Python 3.7). The installer should give you the option to "add Python to PATH." Check this box. If you do that, you can skip the next step. @@ -105,7 +105,7 @@ get it right: should open the "System Properties" screen, then select the "Advanced" tab, then hit the "Environmental Variables..." button, and then look for the PATH variable in the table. Add the following to the end of the variable's value: - ``;C:\Python36;C:\Python36\Scripts``. You may need to adjust these paths to + ``;C:\Python37;C:\Python37\Scripts``. You may need to adjust these paths to point to your Python installation. 3. Now install beets by running: ``pip install beets`` diff --git a/setup.py b/setup.py index a6984ffd2..459969f8e 100755 --- a/setup.py +++ b/setup.py @@ -175,10 +175,10 @@ setup( 'Environment :: Web Environment', 'Programming Language :: Python', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: 3.9', + 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: Implementation :: CPython', ], ) diff --git a/test/helper.py b/test/helper.py index f7d37b654..b6e425c62 100644 --- a/test/helper.py +++ b/test/helper.py @@ -453,7 +453,7 @@ class TestHelper: def run_with_output(self, *args): with capture_stdout() as out: self.run_command(*args) - return util.text_string(out.getvalue()) + return out.getvalue() # Safe file operations diff --git a/test/test_query.py b/test/test_query.py index 3c6d6f70a..3d7b56781 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -15,8 +15,8 @@ """Various tests for querying the library database. """ +from contextlib import contextmanager from functools import partial -from unittest.mock import patch import os import sys import unittest @@ -454,23 +454,14 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): self.lib.add(i2) self.lib.add_album([i2]) + @contextmanager + def force_implicit_query_detection(self): # Unadorned path queries with path separators in them are considered # path queries only when the path in question actually exists. So we # mock the existence check to return true. - self.patcher_exists = patch('beets.library.os.path.exists') - self.patcher_exists.start().return_value = True - - # We have to create function samefile as it does not exist on - # Windows and python 2.7 - self.patcher_samefile = patch('beets.library.os.path.samefile', - create=True) - self.patcher_samefile.start().return_value = True - - def tearDown(self): - super().tearDown() - - self.patcher_samefile.stop() - self.patcher_exists.stop() + beets.library.PathQuery.force_implicit_query_detection = True + yield + beets.library.PathQuery.force_implicit_query_detection = False def test_path_exact_match(self): q = 'path:/a/b/c.mp3' @@ -526,31 +517,35 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_slashed_query_matches_path(self): - q = '/a/b' - results = self.lib.items(q) - self.assert_items_matched(results, ['path item']) + with self.force_implicit_query_detection(): + q = '/a/b' + results = self.lib.items(q) + self.assert_items_matched(results, ['path item']) - results = self.lib.albums(q) - self.assert_albums_matched(results, ['path album']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_path_query_in_or_query(self): - q = '/a/b , /a/b' - results = self.lib.items(q) - self.assert_items_matched(results, ['path item']) + with self.force_implicit_query_detection(): + q = '/a/b , /a/b' + results = self.lib.items(q) + self.assert_items_matched(results, ['path item']) def test_non_slashed_does_not_match_path(self): - q = 'c.mp3' - results = self.lib.items(q) - self.assert_items_matched(results, []) + with self.force_implicit_query_detection(): + q = 'c.mp3' + results = self.lib.items(q) + self.assert_items_matched(results, []) - results = self.lib.albums(q) - self.assert_albums_matched(results, []) + results = self.lib.albums(q) + self.assert_albums_matched(results, []) def test_slashes_in_explicit_field_does_not_match_path(self): - q = 'title:/a/b' - results = self.lib.items(q) - self.assert_items_matched(results, []) + with self.force_implicit_query_detection(): + q = 'title:/a/b' + results = self.lib.items(q) + self.assert_items_matched(results, []) def test_path_item_regex(self): q = 'path::c\\.mp3$' @@ -603,101 +598,67 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.items(makeq(case_sensitive=False)) self.assert_items_matched(results, ['path item', 'caps path']) - # Check for correct case sensitivity selection (this check - # only works on non-Windows OSes). - with _common.system_mock('Darwin'): - # exists = True and samefile = True => Case insensitive - q = makeq() - self.assertEqual(q.case_sensitive, False) + # FIXME: Also create a variant of this test for windows, which tests + # both os.sep and os.altsep + @unittest.skipIf(sys.platform == 'win32', 'win32') + def test_path_sep_detection(self): + is_path_query = beets.library.PathQuery.is_path_query - # exists = True and samefile = False => Case sensitive - self.patcher_samefile.stop() - self.patcher_samefile.start().return_value = False - try: - q = makeq() - self.assertEqual(q.case_sensitive, True) - finally: - self.patcher_samefile.stop() - self.patcher_samefile.start().return_value = True - - # Test platform-aware default sensitivity when the library path - # does not exist. For the duration of this check, we change the - # `os.path.exists` mock to return False. - self.patcher_exists.stop() - self.patcher_exists.start().return_value = False - try: - with _common.system_mock('Darwin'): - q = makeq() - self.assertEqual(q.case_sensitive, True) - - with _common.system_mock('Windows'): - q = makeq() - self.assertEqual(q.case_sensitive, False) - finally: - # Restore the `os.path.exists` mock to its original state. - self.patcher_exists.stop() - self.patcher_exists.start().return_value = True - - @patch('beets.library.os') - def test_path_sep_detection(self, mock_os): - mock_os.sep = '/' - mock_os.altsep = None - mock_os.path.exists = lambda p: True - is_path = beets.library.PathQuery.is_path_query - - self.assertTrue(is_path('/foo/bar')) - self.assertTrue(is_path('foo/bar')) - self.assertTrue(is_path('foo/')) - self.assertFalse(is_path('foo')) - self.assertTrue(is_path('foo/:bar')) - self.assertFalse(is_path('foo:bar/')) - self.assertFalse(is_path('foo:/bar')) + with self.force_implicit_query_detection(): + self.assertTrue(is_path_query('/foo/bar')) + self.assertTrue(is_path_query('foo/bar')) + self.assertTrue(is_path_query('foo/')) + self.assertFalse(is_path_query('foo')) + self.assertTrue(is_path_query('foo/:bar')) + self.assertFalse(is_path_query('foo:bar/')) + self.assertFalse(is_path_query('foo:/bar')) + # FIXME: shouldn't this also work on windows? @unittest.skipIf(sys.platform == 'win32', WIN32_NO_IMPLICIT_PATHS) def test_detect_absolute_path(self): - # Don't patch `os.path.exists`; we'll actually create a file when - # it exists. - self.patcher_exists.stop() - is_path = beets.library.PathQuery.is_path_query + """Test detection of implicit path queries based on whether or + not the path actually exists, when using an absolute path query. - try: - path = self.touch(os.path.join(b'foo', b'bar')) - path = path.decode('utf-8') + Thus, don't use the `force_implicit_query_detection()` + contextmanager which would disable the existence check. + """ + is_path_query = beets.library.PathQuery.is_path_query - # The file itself. - self.assertTrue(is_path(path)) + path = self.touch(os.path.join(b'foo', b'bar')) + self.assertTrue(os.path.isabs(util.syspath(path))) + path_str = path.decode('utf-8') - # The parent directory. - parent = os.path.dirname(path) - self.assertTrue(is_path(parent)) + # The file itself. + self.assertTrue(is_path_query(path_str)) - # Some non-existent path. - self.assertFalse(is_path(path + 'baz')) + # The parent directory. + parent = os.path.dirname(path_str) + self.assertTrue(is_path_query(parent)) - finally: - # Restart the `os.path.exists` patch. - self.patcher_exists.start() + # Some non-existent path. + self.assertFalse(is_path_query(path_str + 'baz')) def test_detect_relative_path(self): - self.patcher_exists.stop() - is_path = beets.library.PathQuery.is_path_query + """Test detection of implicit path queries based on whether or + not the path actually exists, when using a relative path query. + Thus, don't use the `force_implicit_query_detection()` + contextmanager which would disable the existence check. + """ + is_path_query = beets.library.PathQuery.is_path_query + + self.touch(os.path.join(b'foo', b'bar')) + + # Temporarily change directory so relative paths work. + cur_dir = os.getcwd() try: - self.touch(os.path.join(b'foo', b'bar')) - - # Temporarily change directory so relative paths work. - cur_dir = os.getcwd() - try: - os.chdir(self.temp_dir) - self.assertTrue(is_path('foo/')) - self.assertTrue(is_path('foo/bar')) - self.assertTrue(is_path('foo/bar:tagada')) - self.assertFalse(is_path('bar')) - finally: - os.chdir(cur_dir) - + os.chdir(self.temp_dir) + self.assertTrue(is_path_query('foo/')) + self.assertTrue(is_path_query('foo/bar')) + self.assertTrue(is_path_query('foo/bar:tagada')) + self.assertFalse(is_path_query('bar')) finally: - self.patcher_exists.start() + os.chdir(cur_dir) class IntQueryTest(unittest.TestCase, TestHelper): diff --git a/test/test_ui.py b/test/test_ui.py index dd393035b..86c40d204 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -1185,8 +1185,7 @@ class ShowChangeTest(_common.TestCase): cur_album, autotag.AlbumMatch(album_dist, info, mapping, set(), set()), ) - # FIXME decoding shouldn't be done here - return util.text_string(self.io.getoutput().lower()) + return self.io.getoutput().lower() def test_null_change(self): msg = self._show_change() diff --git a/test/test_util.py b/test/test_util.py index 14ac7f2b2..8c16243a5 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -14,10 +14,11 @@ """Tests for base utils from the beets.util package. """ -import sys -import re import os +import platform +import re import subprocess +import sys import unittest from unittest.mock import patch, Mock @@ -122,6 +123,28 @@ class UtilTest(unittest.TestCase): self.assertEqual(exc_context.exception.returncode, 1) self.assertEqual(exc_context.exception.cmd, 'taga \xc3\xa9') + def test_case_sensitive_default(self): + path = util.bytestring_path(util.normpath( + "/this/path/does/not/exist", + )) + + self.assertEqual( + util.case_sensitive(path), + platform.system() != 'Windows', + ) + + @unittest.skipIf(sys.platform == 'win32', 'fs is not case sensitive') + def test_case_sensitive_detects_sensitive(self): + # FIXME: Add tests for more code paths of case_sensitive() + # when the filesystem on the test runner is not case sensitive + pass + + @unittest.skipIf(sys.platform != 'win32', 'fs is case sensitive') + def test_case_sensitive_detects_insensitive(self): + # FIXME: Add tests for more code paths of case_sensitive() + # when the filesystem on the test runner is case sensitive + pass + class PathConversionTest(_common.TestCase): def test_syspath_windows_format(self):