From b80b898ba8f21002e94d7b78378e42453db2104a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 29 Jun 2024 18:19:54 +0100 Subject: [PATCH 01/11] test_bareasc: Add teardown which removes tmp files --- test/plugins/test_bareasc.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/plugins/test_bareasc.py b/test/plugins/test_bareasc.py index 66d8495e56..feb99953c1 100644 --- a/test/plugins/test_bareasc.py +++ b/test/plugins/test_bareasc.py @@ -27,6 +27,9 @@ def setUp(self): self.add_item(title="without umlaut or e", artist="Bruggen") self.add_item(title="without umlaut with e", artist="Brueggen") + def tearDown(self): + self.teardown_beets() + def test_bareasc_search(self): test_cases = [ ( From 56d9d9670f67679d04830019064e1a5339ba3bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 01:47:22 +0100 Subject: [PATCH 02/11] test_art, test_embedart: Define FetchImageHelper without _common.TestCase And move the definition to a shared module. The problem was that EmbedartCliTest ran `_common.TestCase.setUp` method which initialised temporary directory for the tests AND ran `helper.TestHelper.setup_beets` method which initialised another set of temporary directories. This meant that the first set of directories could not be tracked down for the cleanup. --- beets/test/helper.py | 27 +++++++++++++++++++++++++++ test/plugins/test_art.py | 32 +++++--------------------------- test/plugins/test_embedart.py | 5 ++--- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 39c933f183..ebfedb2aad 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -40,6 +40,7 @@ from io import StringIO from tempfile import mkdtemp, mkstemp +import responses from mediafile import Image, MediaFile import beets @@ -925,3 +926,29 @@ def _make_album_match(self, artist, album, tracks, distance=0, missing=0): albumtype="soundtrack", data_source="match_source", ) + + +class FetchImageHelper: + """Helper mixin for mocking requests when fetching images + with remote art sources. + """ + + @responses.activate + def run(self, *args, **kwargs): + super().run(*args, **kwargs) + + IMAGEHEADER = { + "image/jpeg": b"\x00" * 6 + b"JFIF", + "image/png": b"\211PNG\r\n\032\n", + } + + def mock_response(self, url, content_type="image/jpeg", file_type=None): + if file_type is None: + file_type = content_type + responses.add( + responses.GET, + url, + content_type=content_type, + # imghdr reads 32 bytes + body=self.IMAGEHEADER.get(file_type, b"").ljust(32, b"\x00"), + ) diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 6e877917d7..a8cbc15ae3 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -26,7 +26,7 @@ from beets import config, importer, library, logging, util from beets.autotag import AlbumInfo, AlbumMatch from beets.test import _common -from beets.test.helper import capture_log +from beets.test.helper import FetchImageHelper, capture_log from beets.util import syspath from beets.util.artresizer import ArtResizer from beetsplug import fetchart @@ -50,30 +50,8 @@ def setUp(self): self.plugin = fetchart.FetchArtPlugin() -class FetchImageHelper(_common.TestCase): - """Helper mixin for mocking requests when fetching images - with remote art sources. - """ - - @responses.activate - def run(self, *args, **kwargs): - super().run(*args, **kwargs) - - IMAGEHEADER = { - "image/jpeg": b"\x00" * 6 + b"JFIF", - "image/png": b"\211PNG\r\n\032\n", - } - - def mock_response(self, url, content_type="image/jpeg", file_type=None): - if file_type is None: - file_type = content_type - responses.add( - responses.GET, - url, - content_type=content_type, - # imghdr reads 32 bytes - body=self.IMAGEHEADER.get(file_type, b"").ljust(32, b"\x00"), - ) +class FetchImageTestCase(FetchImageHelper, UseThePlugin): + pass class CAAHelper: @@ -212,7 +190,7 @@ def mock_caa_response(self, url, json): ) -class FetchImageTest(FetchImageHelper, UseThePlugin): +class FetchImageTest(FetchImageTestCase): URL = "http://example.com/test.jpg" def setUp(self): @@ -293,7 +271,7 @@ def test_precedence_amongst_correct_files(self): self.assertEqual(candidates, paths) -class CombinedTest(FetchImageHelper, UseThePlugin, CAAHelper): +class CombinedTest(FetchImageTestCase, CAAHelper): ASIN = "xxxx" MBID = "releaseid" AMAZON_URL = "https://images.amazon.com/images/P/{}.01.LZZZZZZZ.jpg".format( diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 80d284d317..48a110295b 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -17,7 +17,6 @@ import shutil import tempfile import unittest -from test.plugins.test_art import FetchImageHelper from test.test_art_resize import DummyIMBackend from unittest.mock import MagicMock, patch @@ -25,7 +24,7 @@ from beets import art, config, logging, ui from beets.test import _common -from beets.test.helper import TestHelper +from beets.test.helper import FetchImageHelper, TestHelper from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer @@ -48,7 +47,7 @@ class EmbedartCliTest(TestHelper, FetchImageHelper): abbey_differentpath = os.path.join(_common.RSRC, b"abbey-different.jpg") def setUp(self): - super().setUp() + self.io = _common.DummyIO() self.io.install() self.setup_beets() # Converter is threaded self.load_plugins("embedart") From 1fda7b61119c6b560fa9bf6f31e0f05ecf21cfa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 02:00:15 +0100 Subject: [PATCH 03/11] fetchart, artresizer: Create art files in predictable directories This allows to clean them up in art (1) fetching, (2) resizing and (3) deinterlace tests. --- beets/test/helper.py | 19 +++++++++++++++- beets/util/__init__.py | 47 ++++++++++++++++++++++++++++++++++++++++ beets/util/artresizer.py | 27 ++++++++++------------- beetsplug/fetchart.py | 11 +++++----- test/plugins/test_art.py | 6 +++-- test/test_art_resize.py | 6 +++-- 6 files changed, 90 insertions(+), 26 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index ebfedb2aad..059500bbaa 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -29,6 +29,7 @@ - The `TestHelper` class encapsulates various fixtures that can be set up. """ +from __future__ import annotations import os import os.path @@ -39,6 +40,7 @@ from enum import Enum from io import StringIO from tempfile import mkdtemp, mkstemp +from typing import ClassVar import responses from mediafile import Image, MediaFile @@ -50,7 +52,12 @@ from beets.library import Album, Item, Library from beets.test import _common from beets.ui.commands import TerminalImportSession -from beets.util import MoveOperation, bytestring_path, syspath +from beets.util import ( + MoveOperation, + bytestring_path, + clean_module_tempdir, + syspath, +) class LogCapture(logging.Handler): @@ -952,3 +959,13 @@ def mock_response(self, url, content_type="image/jpeg", file_type=None): # imghdr reads 32 bytes body=self.IMAGEHEADER.get(file_type, b"").ljust(32, b"\x00"), ) + + +class CleanupModulesMixin: + modules: ClassVar[tuple[str, ...]] + + @classmethod + def tearDownClass(cls) -> None: + """Remove files created by the plugin.""" + for module in cls.modules: + clean_module_tempdir(module) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index cae081dd4f..9076bea30b 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -13,6 +13,7 @@ # included in all copies or substantial portions of the Software. """Miscellaneous utility functions.""" +from __future__ import annotations import errno import fnmatch @@ -26,9 +27,11 @@ import tempfile import traceback from collections import Counter, namedtuple +from contextlib import suppress from enum import Enum from logging import Logger from multiprocessing.pool import ThreadPool +from pathlib import Path from typing import ( Any, AnyStr, @@ -58,6 +61,7 @@ WINDOWS_MAGIC_PREFIX = "\\\\?\\" T = TypeVar("T") Bytes_or_String: TypeAlias = Union[str, bytes] +PathLike = Union[str, bytes, Path] class HumanReadableException(Exception): @@ -1076,3 +1080,46 @@ def __get__(self, instance, owner): self.cache[owner] = self.getter(owner) return self.cache[owner] + + +def get_module_tempdir(module: str) -> Path: + """Return the temporary directory for the given module. + + The directory is created within the `/tmp/beets/` directory on + Linux (or the equivalent temporary directory on other systems). + + Dots in the module name are replaced by underscores. + """ + module = module.replace("beets.", "").replace(".", "_") + return Path(tempfile.gettempdir()) / "beets" / module + + +def clean_module_tempdir(module: str) -> None: + """Clean the temporary directory for the given module.""" + tempdir = get_module_tempdir(module) + shutil.rmtree(tempdir, ignore_errors=True) + with suppress(OSError): + # remove parent (/tmp/beets) directory if it is empty + tempdir.parent.rmdir() + + +def get_temp_filename( + module: str, + prefix: str = "", + path: PathLike | None = None, + suffix: str = "", +) -> bytes: + """Return temporary filename for the given module and prefix. + + The filename starts with the given `prefix`. + If 'suffix' is given, it is used a the file extension. + If 'path' is given, we use the same suffix. + """ + if not suffix and path: + suffix = Path(os.fsdecode(path)).suffix + + tempdir = get_module_tempdir(module) + tempdir.mkdir(parents=True, exist_ok=True) + + _, filename = tempfile.mkstemp(dir=tempdir, prefix=prefix, suffix=suffix) + return bytestring_path(filename) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 84844fac17..09cc29e0df 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -22,11 +22,10 @@ import re import subprocess from itertools import chain -from tempfile import NamedTemporaryFile from urllib.parse import urlencode from beets import logging, util -from beets.util import bytestring_path, displayable_path, syspath +from beets.util import displayable_path, get_temp_filename, syspath PROXY_URL = "https://images.weserv.nl/" @@ -48,15 +47,6 @@ def resize_url(url, maxwidth, quality=0): return "{}?{}".format(PROXY_URL, urlencode(params)) -def temp_file_for(path): - """Return an unused filename with the same extension as the - specified path. - """ - ext = os.path.splitext(path)[1] - with NamedTemporaryFile(suffix=os.fsdecode(ext), delete=False) as f: - return bytestring_path(f.name) - - class LocalBackendNotAvailableError(Exception): pass @@ -141,7 +131,9 @@ def resize( Use the ``magick`` program or ``convert`` on older versions. Return the output path of resized image. """ - path_out = path_out or temp_file_for(path_in) + if not path_out: + path_out = get_temp_filename(__name__, "resize_IM_", path_in) + log.debug( "artresizer: ImageMagick resizing {0} to {1}", displayable_path(path_in), @@ -208,7 +200,8 @@ def get_size(self, path_in): return None def deinterlace(self, path_in, path_out=None): - path_out = path_out or temp_file_for(path_in) + if not path_out: + path_out = get_temp_filename(__name__, "deinterlace_IM_", path_in) cmd = self.convert_cmd + [ syspath(path_in, prefix=False), @@ -366,7 +359,9 @@ def resize( """Resize using Python Imaging Library (PIL). Return the output path of resized image. """ - path_out = path_out or temp_file_for(path_in) + if not path_out: + path_out = get_temp_filename(__name__, "resize_PIL_", path_in) + from PIL import Image log.debug( @@ -442,7 +437,9 @@ def get_size(self, path_in): return None def deinterlace(self, path_in, path_out=None): - path_out = path_out or temp_file_for(path_in) + if not path_out: + path_out = get_temp_filename(__name__, "deinterlace_PIL_", path_in) + from PIL import Image try: diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index a3bac19a1c..72aa3aa295 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -19,14 +19,13 @@ import re from collections import OrderedDict from contextlib import closing -from tempfile import NamedTemporaryFile import confuse import requests from mediafile import image_mime_type from beets import config, importer, plugins, ui, util -from beets.util import bytestring_path, sorted_walk, syspath +from beets.util import bytestring_path, get_temp_filename, sorted_walk, syspath from beets.util.artresizer import ArtResizer try: @@ -412,17 +411,17 @@ def fetch_image(self, candidate, plugin): ext, ) - suffix = os.fsdecode(ext) - with NamedTemporaryFile(suffix=suffix, delete=False) as fh: + filename = get_temp_filename(__name__, suffix=ext.decode()) + with open(filename, "wb") as fh: # write the first already loaded part of the image fh.write(header) # download the remaining part of the image for chunk in data: fh.write(chunk) self._log.debug( - "downloaded art to: {0}", util.displayable_path(fh.name) + "downloaded art to: {0}", util.displayable_path(filename) ) - candidate.path = util.bytestring_path(fh.name) + candidate.path = util.bytestring_path(filename) return except (OSError, requests.RequestException, TypeError) as exc: diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index a8cbc15ae3..8a2aa5870a 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -26,7 +26,7 @@ from beets import config, importer, library, logging, util from beets.autotag import AlbumInfo, AlbumMatch from beets.test import _common -from beets.test.helper import FetchImageHelper, capture_log +from beets.test.helper import CleanupModulesMixin, FetchImageHelper, capture_log from beets.util import syspath from beets.util.artresizer import ArtResizer from beetsplug import fetchart @@ -44,7 +44,9 @@ def __init__(self, **kwargs): setattr(self, k, v) -class UseThePlugin(_common.TestCase): +class UseThePlugin(CleanupModulesMixin, _common.TestCase): + modules = (fetchart.__name__, ArtResizer.__module__) + def setUp(self): super().setUp() self.plugin = fetchart.FetchArtPlugin() diff --git a/test/test_art_resize.py b/test/test_art_resize.py index 5cb1e7e697..ac9463cba4 100644 --- a/test/test_art_resize.py +++ b/test/test_art_resize.py @@ -20,7 +20,7 @@ from unittest.mock import patch from beets.test import _common -from beets.test.helper import TestHelper +from beets.test.helper import CleanupModulesMixin, TestHelper from beets.util import command_output, syspath from beets.util.artresizer import IMBackend, PILBackend @@ -48,9 +48,11 @@ def __init__(self): pass -class ArtResizerFileSizeTest(_common.TestCase, TestHelper): +class ArtResizerFileSizeTest(CleanupModulesMixin, _common.TestCase, TestHelper): """Unittest test case for Art Resizer to a specific filesize.""" + modules = (IMBackend.__module__,) + IMG_225x225 = os.path.join(_common.RSRC, b"abbey.jpg") IMG_225x225_SIZE = os.stat(syspath(IMG_225x225)).st_size From c9045215c14fbf2661e55f985cb8503a7bfc6112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 30 Jun 2024 15:52:02 +0100 Subject: [PATCH 04/11] TestHelper: Create mediafield fixtures within temp_dir This way they get automatically removed with removal of temp_dir. After this change `test/plugins/test_zero.py` does not any more leave any mediafield fixtures hanging around. --- beets/test/helper.py | 17 ++--------------- test/plugins/test_info.py | 3 --- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 059500bbaa..c9b30f619f 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -405,18 +405,14 @@ def add_album_fixture( return self.lib.add_album(items) def create_mediafile_fixture(self, ext="mp3", images=[]): - """Copies a fixture mediafile with the extension to a temporary - location and returns the path. - - It keeps track of the created locations and will delete the with - `remove_mediafile_fixtures()` + """Copy a fixture mediafile with the extension to `temp_dir`. `images` is a subset of 'png', 'jpg', and 'tiff'. For each specified extension a cover art image is added to the media file. """ src = os.path.join(_common.RSRC, util.bytestring_path("full." + ext)) - handle, path = mkstemp() + handle, path = mkstemp(dir=self.temp_dir) path = bytestring_path(path) os.close(handle) shutil.copyfile(syspath(src), syspath(path)) @@ -432,17 +428,8 @@ def create_mediafile_fixture(self, ext="mp3", images=[]): mediafile.images = imgs mediafile.save() - if not hasattr(self, "_mediafile_fixtures"): - self._mediafile_fixtures = [] - self._mediafile_fixtures.append(path) - return path - def remove_mediafile_fixtures(self): - if hasattr(self, "_mediafile_fixtures"): - for path in self._mediafile_fixtures: - os.remove(syspath(path)) - def _get_item_count(self): if not hasattr(self, "__item_count"): count = 0 diff --git a/test/plugins/test_info.py b/test/plugins/test_info.py index a14424799f..bfba739c5a 100644 --- a/test/plugins/test_info.py +++ b/test/plugins/test_info.py @@ -46,7 +46,6 @@ def test_path(self): self.assertIn("disctitle: DDD", out) self.assertIn("genres: a; b; c", out) self.assertNotIn("composer:", out) - self.remove_mediafile_fixtures() def test_item_query(self): item1, item2 = self.add_item_fixtures(count=2) @@ -88,7 +87,6 @@ def test_collect_item_and_path(self): self.assertIn("album: AAA", out) self.assertIn("tracktotal: 5", out) self.assertIn("title: [various]", out) - self.remove_mediafile_fixtures() def test_collect_item_and_path_with_multi_values(self): path = self.create_mediafile_fixture() @@ -116,7 +114,6 @@ def test_collect_item_and_path_with_multi_values(self): self.assertIn("title: [various]", out) self.assertIn("albumartists: [various]", out) self.assertIn("artists: Artist A; Artist Z", out) - self.remove_mediafile_fixtures() def test_custom_format(self): self.add_item_fixtures() From 0682d0d0301fd11f47031e2812ee1043cbeabb9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 02:08:31 +0100 Subject: [PATCH 05/11] test_play: Remove files generated by play plugin --- beetsplug/play.py | 16 ++++++++-------- test/plugins/test_play.py | 7 +++++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/beetsplug/play.py b/beetsplug/play.py index 4884bc8beb..3476e58243 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -18,12 +18,12 @@ import shlex import subprocess from os.path import relpath -from tempfile import NamedTemporaryFile from beets import config, ui, util from beets.plugins import BeetsPlugin from beets.ui import Subcommand from beets.ui.commands import PromptChoice +from beets.util import get_temp_filename # Indicate where arguments should be inserted into the command string. # If this is missing, they're placed at the end. @@ -194,15 +194,15 @@ def _exceeds_threshold( def _create_tmp_playlist(self, paths_list): """Create a temporary .m3u file. Return the filename.""" utf8_bom = config["play"]["bom"].get(bool) - m3u = NamedTemporaryFile("wb", suffix=".m3u", delete=False) + filename = get_temp_filename(__name__, suffix=".m3u") + with open(filename, "wb") as m3u: + if utf8_bom: + m3u.write(b"\xEF\xBB\xBF") - if utf8_bom: - m3u.write(b"\xEF\xBB\xBF") + for item in paths_list: + m3u.write(item + b"\n") - for item in paths_list: - m3u.write(item + b"\n") - m3u.close() - return m3u.name + return filename def before_choose_candidate_listener(self, session, task): """Append a "Play" choice to the interactive importer prompt.""" diff --git a/test/plugins/test_play.py b/test/plugins/test_play.py index cb99f6b433..ac60e8281e 100644 --- a/test/plugins/test_play.py +++ b/test/plugins/test_play.py @@ -20,13 +20,16 @@ import unittest from unittest.mock import ANY, patch -from beets.test.helper import TestHelper, control_stdin +from beets.test.helper import CleanupModulesMixin, TestHelper, control_stdin from beets.ui import UserError from beets.util import open_anything +from beetsplug.play import PlayPlugin @patch("beetsplug.play.util.interactive_open") -class PlayPluginTest(unittest.TestCase, TestHelper): +class PlayPluginTest(CleanupModulesMixin, unittest.TestCase, TestHelper): + modules = (PlayPlugin.__module__,) + def setUp(self): self.setup_beets() self.load_plugins("play") From 12730aa8d99d76b4dd3f8a33dc8d0b77deeb643b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 08:56:11 +0100 Subject: [PATCH 06/11] test_config_command: use TestHelper to manage temp dir Due to some weird race conditions the temporary directories were not getting torn down for four of the tests. I failed to figure out why, and I found that using TestHelper to manage the temporary directory somehow fixes this. --- test/test_config_command.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/test_config_command.py b/test/test_config_command.py index 553c985da3..0b122cf1cf 100644 --- a/test/test_config_command.py +++ b/test/test_config_command.py @@ -1,32 +1,29 @@ import os import unittest -from shutil import rmtree -from tempfile import mkdtemp from unittest.mock import patch import yaml from beets import config, ui -from beets.library import Library from beets.test.helper import TestHelper class ConfigCommandTest(unittest.TestCase, TestHelper): def setUp(self): - self.lib = Library(":memory:") - self.temp_dir = mkdtemp() + self.setup_beets() for k in ("VISUAL", "EDITOR"): if k in os.environ: del os.environ[k] - os.environ["BEETSDIR"] = self.temp_dir - self.config_path = os.path.join(self.temp_dir, "config.yaml") + temp_dir = self.temp_dir.decode() + + self.config_path = os.path.join(temp_dir, "config.yaml") with open(self.config_path, "w") as file: file.write("library: lib\n") file.write("option: value\n") file.write("password: password_value") - self.cli_config_path = os.path.join(self.temp_dir, "cli_config.yaml") + self.cli_config_path = os.path.join(temp_dir, "cli_config.yaml") with open(self.cli_config_path, "w") as file: file.write("option: cli overwrite") @@ -35,7 +32,7 @@ def setUp(self): config._materialized = False def tearDown(self): - rmtree(self.temp_dir) + self.teardown_beets() def _run_with_yaml_output(self, *args): output = self.run_with_output(*args) From 82b1a0d01f2dddb42fa7d8758124d9b78ed9afb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 09:19:54 +0100 Subject: [PATCH 07/11] test_plugins.py: dedupe setUp, tearDown usage And most importantly, remove a redudant invocation of `setup_beets` which left temporary directories hanging around without getting cleaned up. --- test/test_plugins.py | 52 +++++++++++--------------------------------- 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/test/test_plugins.py b/test/test_plugins.py index b13df9607b..707c7db31a 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -37,6 +37,7 @@ AutotagStub, ImportHelper, TerminalImportSessionSetup, + TestHelper, ) from beets.util import bytestring_path, displayable_path, syspath from beets.util.id_extractors import ( @@ -46,7 +47,7 @@ ) -class TestHelper(helper.TestHelper): +class PluginLoaderTestCase(unittest.TestCase, TestHelper): def setup_plugin_loader(self): # FIXME the mocking code is horrific, but this is the lowest and # earliest level of the plugin mechanism we can hook into. @@ -68,8 +69,6 @@ def teardown_plugin_loader(self): def register_plugin(self, plugin_class): self._plugin_classes.add(plugin_class) - -class ItemTypesTest(unittest.TestCase, TestHelper): def setUp(self): self.setup_plugin_loader() @@ -77,6 +76,8 @@ def tearDown(self): self.teardown_plugin_loader() self.teardown_beets() + +class ItemTypesTest(PluginLoaderTestCase): def test_flex_field_type(self): class RatingPlugin(plugins.BeetsPlugin): item_types = {"rating": types.Float()} @@ -102,10 +103,9 @@ class RatingPlugin(plugins.BeetsPlugin): self.assertNotIn("aaa", out) -class ItemWriteTest(unittest.TestCase, TestHelper): +class ItemWriteTest(PluginLoaderTestCase): def setUp(self): - self.setup_plugin_loader() - self.setup_beets() + super().setUp() class EventListenerPlugin(plugins.BeetsPlugin): pass @@ -113,10 +113,6 @@ class EventListenerPlugin(plugins.BeetsPlugin): self.event_listener_plugin = EventListenerPlugin() self.register_plugin(EventListenerPlugin) - def tearDown(self): - self.teardown_plugin_loader() - self.teardown_beets() - def test_change_tags(self): def on_write(item=None, path=None, tags=None): if tags["artist"] == "XXX": @@ -134,15 +130,7 @@ def register_listener(self, event, func): self.event_listener_plugin.register_listener(event, func) -class ItemTypeConflictTest(unittest.TestCase, TestHelper): - def setUp(self): - self.setup_plugin_loader() - self.setup_beets() - - def tearDown(self): - self.teardown_plugin_loader() - self.teardown_beets() - +class ItemTypeConflictTest(PluginLoaderTestCase): def test_mismatch(self): class EventListenerPlugin(plugins.BeetsPlugin): item_types = {"duplicate": types.INTEGER} @@ -170,17 +158,12 @@ class AdventListenerPlugin(plugins.BeetsPlugin): self.assertIsNotNone(plugins.types(Item)) -class EventsTest(unittest.TestCase, ImportHelper, TestHelper): +class EventsTest(ImportHelper, PluginLoaderTestCase): def setUp(self): - self.setup_plugin_loader() - self.setup_beets() + super().setUp() self.__create_import_dir(2) config["import"]["pretend"] = True - def tearDown(self): - self.teardown_plugin_loader() - self.teardown_beets() - def __copy_file(self, dest_path, metadata): # Copy files resource_path = os.path.join(RSRC, b"full.mp3") @@ -301,14 +284,7 @@ def test_sanitize_choices(self): ) -class ListenersTest(unittest.TestCase, TestHelper): - def setUp(self): - self.setup_plugin_loader() - - def tearDown(self): - self.teardown_plugin_loader() - self.teardown_beets() - +class ListenersTest(PluginLoaderTestCase): def test_register(self): class DummyPlugin(plugins.BeetsPlugin): def __init__(self): @@ -428,11 +404,10 @@ def dummy9(self, **kwargs): class PromptChoicesTest( - TerminalImportSessionSetup, unittest.TestCase, ImportHelper, TestHelper + TerminalImportSessionSetup, ImportHelper, PluginLoaderTestCase ): def setUp(self): - self.setup_plugin_loader() - self.setup_beets() + super().setUp() self._create_import_dir(3) self._setup_import_session() self.matcher = AutotagStub().install() @@ -443,9 +418,8 @@ def setUp(self): self.mock_input_options = self.input_options_patcher.start() def tearDown(self): + super().tearDown() self.input_options_patcher.stop() - self.teardown_plugin_loader() - self.teardown_beets() self.matcher.restore() def test_plugin_choices_in_ui_input_options_album(self): From 3177bc44baad4ca271288c391a08cd360ba251da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 09:36:32 +0100 Subject: [PATCH 08/11] test_importer.py: do not run _common.TestCase.setUp with setup_beets --- test/test_importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_importer.py b/test/test_importer.py index fdbcc8b142..fe41ad2f55 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1763,7 +1763,7 @@ def __init__(self, method_name="runTest"): self.matcher = None def setUp(self): - super().setUp() + self.io = _common.DummyIO() self.setup_beets() self.__create_import_dir() self.__create_empty_import_dir() From de704125d890480bc92d84602976aacd5ce8bbe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 09:43:07 +0100 Subject: [PATCH 09/11] test_ui_commands.py: make sure tearDown is run for FieldsTest --- test/test_ui_commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_ui_commands.py b/test/test_ui_commands.py index 1d9cae0487..f371a1ab11 100644 --- a/test/test_ui_commands.py +++ b/test/test_ui_commands.py @@ -94,6 +94,7 @@ def setUp(self): self.io.install() def tearDown(self): + super().tearDown() self.io.restore() def remove_keys(self, l, text): From f88bb4ed600f9008885d2c4ec819c2cf8f08776a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 2 Jul 2024 14:32:40 +0100 Subject: [PATCH 10/11] Add a shell command which tests for leftover temp files --- pyproject.toml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index be239a0ece..45e7afdc11 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -230,6 +230,21 @@ env.OPTS = """ --cov-context=test """ +[tool.poe.tasks.check-temp-files] +help = "Run each test module one by one and check for leftover temp files" +shell = """ +setopt nullglob +for file in test/**/*.py; do + print Temp files created by $file && poe test $file &>/dev/null + tempfiles=(/tmp/**/tmp* /tmp/beets/**/*) + if (( $#tempfiles )); then + print -l $'\t'$^tempfiles + rm -r --interactive=never $tempfiles &>/dev/null + fi +done +""" +interpreter = "zsh" + [tool.black] line-length = 80 target-version = ["py38", "py39", "py310", "py311"] From 83f101627e81bbbfa7833ccd5f58b88ce116a28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 17 Jul 2024 23:34:16 +0100 Subject: [PATCH 11/11] Use common temp_dir for playlist plugin directory --- test/plugins/test_playlist.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/plugins/test_playlist.py b/test/plugins/test_playlist.py index b4861dcafc..a4e6a91f93 100644 --- a/test/plugins/test_playlist.py +++ b/test/plugins/test_playlist.py @@ -14,8 +14,6 @@ import os -import shutil -import tempfile import unittest from shlex import quote @@ -72,7 +70,10 @@ def setUp(self): self.lib.add(i3) self.lib.add_album([i3]) - self.playlist_dir = tempfile.mkdtemp() + self.playlist_dir = os.path.join( + os.fsdecode(self.temp_dir), "playlists" + ) + os.makedirs(self.playlist_dir) self.config["directory"] = self.music_dir self.config["playlist"]["playlist_dir"] = self.playlist_dir @@ -84,7 +85,6 @@ def setup_test(self): def tearDown(self): self.unload_plugins() - shutil.rmtree(self.playlist_dir) self.teardown_beets()