From 0089fb430a011bee35f87d104df0799ce557de9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 13:17:21 +0100 Subject: [PATCH 01/12] Enable EmbedartCliTest --- test/plugins/test_embedart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 48a110295b..5d3e22c636 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -40,7 +40,7 @@ def wrapper(*args, **kwargs): return wrapper -class EmbedartCliTest(TestHelper, FetchImageHelper): +class EmbedartCliTest(TestHelper, FetchImageHelper, unittest.TestCase): small_artpath = os.path.join(_common.RSRC, b"image-2x3.jpg") abbey_artpath = os.path.join(_common.RSRC, b"abbey.jpg") abbey_similarpath = os.path.join(_common.RSRC, b"abbey-similar.jpg") From c71aceb1842ccf62e5bdc8882421ddab84f3d954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 13:17:49 +0100 Subject: [PATCH 02/12] Enable LRCLibIntegrationTest --- test/plugins/test_lyrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py index 39fb19a24e..2953c1e910 100644 --- a/test/plugins/test_lyrics.py +++ b/test/plugins/test_lyrics.py @@ -750,7 +750,7 @@ def test_fetch_exception(self, mock_get): self.assertIsNone(lyrics) -class LRCLibIntegrationTest(LyricsAssertions): +class LRCLibIntegrationTest(LyricsAssertions, unittest.TestCase): def setUp(self): self.plugin = lyrics.LyricsPlugin() lrclib.config = self.plugin.config From c2f5a6c19ca2bc0025c5b2af8a326f3f4bfdced0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 13:34:20 +0100 Subject: [PATCH 03/12] Replace has_program with shutil.which --- beets/test/helper.py | 19 ------------------- test/plugins/test_replaygain.py | 7 ++++--- test/test_importer.py | 10 ++-------- test/test_ui.py | 10 ++-------- 4 files changed, 8 insertions(+), 38 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index c9b30f619f..2ec45677f7 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -18,8 +18,6 @@ - The `control_stdin` and `capture_stdout` context managers allow one to interact with the user interface. -- `has_program` checks the presence of a command on the system. - - The `generate_album_info` and `generate_track_info` functions return fixtures to be used when mocking the autotagger. @@ -34,7 +32,6 @@ import os import os.path import shutil -import subprocess import sys from contextlib import contextmanager from enum import Enum @@ -126,22 +123,6 @@ def _convert_args(args): return args -def has_program(cmd, args=["--version"]): - """Returns `True` if `cmd` can be executed.""" - full_cmd = _convert_args([cmd] + args) - try: - with open(os.devnull, "wb") as devnull: - subprocess.check_call( - full_cmd, stderr=devnull, stdout=devnull, stdin=devnull - ) - except OSError: - return False - except subprocess.CalledProcessError: - return False - else: - return True - - class TestHelper: """Helper mixin for high-level cli and plugin tests. diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 92e3e5f650..d37a9a67b3 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -13,12 +13,13 @@ # included in all copies or substantial portions of the Software. +import shutil import unittest from mediafile import MediaFile from beets import config -from beets.test.helper import TestHelper, has_program +from beets.test.helper import TestHelper from beetsplug.replaygain import ( FatalGstreamerPluginReplayGainError, GStreamerBackend, @@ -32,12 +33,12 @@ except (ImportError, ValueError): GST_AVAILABLE = False -if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]): +if shutil.which("mp3gain") or shutil.which("aacgain"): GAIN_PROG_AVAILABLE = True else: GAIN_PROG_AVAILABLE = False -FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"]) +FFMPEG_AVAILABLE = shutil.which("ffmpeg") def reset_replaygain(item): diff --git a/test/test_importer.py b/test/test_importer.py index fe41ad2f55..9ceeaad323 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -34,13 +34,7 @@ from beets.autotag import AlbumInfo, AlbumMatch, TrackInfo from beets.importer import albums_in_dir from beets.test import _common -from beets.test.helper import ( - AutotagStub, - ImportHelper, - TestHelper, - capture_log, - has_program, -) +from beets.test.helper import AutotagStub, ImportHelper, TestHelper, capture_log from beets.util import bytestring_path, displayable_path, syspath @@ -327,7 +321,7 @@ def create_archive(self): return path -@unittest.skipIf(not has_program("unrar"), "unrar program not found") +@unittest.skipIf(not shutil.which("unrar"), "unrar program not found") class ImportRarTest(ImportZipTest): def create_archive(self): return os.path.join(_common.RSRC, b"archive.rar") diff --git a/test/test_ui.py b/test/test_ui.py index f7494bafb3..aed49fbe08 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -30,12 +30,7 @@ from beets import autotag, config, library, plugins, ui, util from beets.autotag.match import distance from beets.test import _common -from beets.test.helper import ( - TestHelper, - capture_stdout, - control_stdin, - has_program, -) +from beets.test.helper import TestHelper, capture_stdout, control_stdin from beets.ui import commands from beets.util import MoveOperation, syspath @@ -1416,6 +1411,7 @@ def test_plugin_command_from_pluginpath(self): @_common.slow_test() +@unittest.skipIf(not shutil.which("bash"), "bash not available") class CompletionTest(_common.TestCase, TestHelper): def test_completion(self): # Load plugin commands @@ -1430,8 +1426,6 @@ def test_completion(self): # Open a `bash` process to run the tests in. We'll pipe in bash # commands via stdin. cmd = os.environ.get("BEETS_TEST_SHELL", "/bin/bash --norc").split() - if not has_program(cmd[0]): - self.skipTest("bash not available") tester = subprocess.Popen( cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, env=env ) From d716475f3d172cd71d9575e562e18ad12b6522e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 15:42:29 +0100 Subject: [PATCH 04/12] Do not skip any replaygain tests This means that the dependencies must be installed for the tests to work. Otherwise, why do we have these tests that aren't being run? --- .github/workflows/ci.yaml | 7 +++- test/plugins/test_replaygain.py | 64 --------------------------------- 2 files changed, 6 insertions(+), 65 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8d0befeee0..f121aa3017 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -33,7 +33,12 @@ jobs: if: matrix.platform == 'ubuntu-latest' run: | sudo apt update - sudo apt install ffmpeg gobject-introspection libgirepository1.0-dev + sudo apt install \ + ffmpeg \ + gobject-introspection \ + gstreamer1.0-plugins-good \ + libgirepository1.0-dev \ + mp3gain poetry install --extras replaygain - name: Install Python dependencies diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index d37a9a67b3..5d9f363159 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -11,34 +11,11 @@ # # The above copyright notice and this permission notice shall be # included in all copies or substantial portions of the Software. - - -import shutil import unittest from mediafile import MediaFile -from beets import config from beets.test.helper import TestHelper -from beetsplug.replaygain import ( - FatalGstreamerPluginReplayGainError, - GStreamerBackend, -) - -try: - import gi - - gi.require_version("Gst", "1.0") - GST_AVAILABLE = True -except (ImportError, ValueError): - GST_AVAILABLE = False - -if shutil.which("mp3gain") or shutil.which("aacgain"): - GAIN_PROG_AVAILABLE = True -else: - GAIN_PROG_AVAILABLE = False - -FFMPEG_AVAILABLE = shutil.which("ffmpeg") def reset_replaygain(item): @@ -56,43 +33,21 @@ class GstBackendMixin: backend = "gstreamer" has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - try: - # Check if required plugins can be loaded by instantiating a - # GStreamerBackend (via its .__init__). - config["replaygain"]["targetlevel"] = 89 - GStreamerBackend(config["replaygain"], None) - except FatalGstreamerPluginReplayGainError as e: - # Skip the test if plugins could not be loaded. - self.skipTest(str(e)) - class CmdBackendMixin: backend = "command" has_r128_support = False - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class FfmpegBackendMixin: backend = "ffmpeg" has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class ReplayGainCliTestBase(TestHelper): FNAME: str def setUp(self): - # Implemented by Mixins, see above. This may decide to skip the test. - self.test_backend() - self.setup_beets(disk=True) self.config["replaygain"]["backend"] = self.backend @@ -126,14 +81,6 @@ def test_cli_saves_track_gain(self): self.run_command("replaygain") - # Skip the test if rg_track_peak and rg_track gain is None, assuming - # that it could only happen if the decoder plugins are missing. - if all( - i.rg_track_peak is None and i.rg_track_gain is None - for i in self.lib.items() - ): - self.skipTest("decoder plugins could not be loaded.") - for item in self.lib.items(): self.assertIsNotNone(item.rg_track_peak) self.assertIsNotNone(item.rg_track_gain) @@ -319,28 +266,24 @@ def test_per_disc(self): self.assertIsNotNone(item.rg_album_gain) -@unittest.skipIf(not GST_AVAILABLE, "gstreamer cannot be found") class ReplayGainGstCliTest( ReplayGainCliTestBase, unittest.TestCase, GstBackendMixin ): FNAME = "full" # file contains only silence -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") class ReplayGainCmdCliTest( ReplayGainCliTestBase, unittest.TestCase, CmdBackendMixin ): FNAME = "full" # file contains only silence -@unittest.skipIf(not FFMPEG_AVAILABLE, "ffmpeg cannot be found") class ReplayGainFfmpegCliTest( ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin ): FNAME = "full" # file contains only silence -@unittest.skipIf(not FFMPEG_AVAILABLE, "ffmpeg cannot be found") class ReplayGainFfmpegNoiseCliTest( ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin ): @@ -351,9 +294,6 @@ class ImportTest(TestHelper): threaded = False def setUp(self): - # Implemented by Mixins, see above. This may decide to skip the test. - self.test_backend() - self.setup_beets(disk=True) self.config["threaded"] = self.threaded self.config["replaygain"]["backend"] = self.backend @@ -380,24 +320,20 @@ def test_import_converted(self): self.assertIsNotNone(item.rg_album_gain) -@unittest.skipIf(not GST_AVAILABLE, "gstreamer cannot be found") class ReplayGainGstImportTest(ImportTest, unittest.TestCase, GstBackendMixin): pass -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") class ReplayGainCmdImportTest(ImportTest, unittest.TestCase, CmdBackendMixin): pass -@unittest.skipIf(not FFMPEG_AVAILABLE, "ffmpeg cannot be found") class ReplayGainFfmpegImportTest( ImportTest, unittest.TestCase, FfmpegBackendMixin ): pass -@unittest.skipIf(not FFMPEG_AVAILABLE, "ffmpeg cannot be found") class ReplayGainFfmpegThreadedImportTest( ImportTest, unittest.TestCase, FfmpegBackendMixin ): From 06cf9c4ea846f4ec5e608221a372b4454ed99119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 15:44:49 +0100 Subject: [PATCH 05/12] replaygain: separate out r128 tests into a mixin to clarify which backends are tested --- test/plugins/test_replaygain.py | 188 ++++++++++++++------------------ 1 file changed, 83 insertions(+), 105 deletions(-) diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 5d9f363159..7a8bfc15d2 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -31,17 +31,94 @@ def reset_replaygain(item): class GstBackendMixin: backend = "gstreamer" - has_r128_support = True class CmdBackendMixin: backend = "command" - has_r128_support = False class FfmpegBackendMixin: backend = "ffmpeg" - has_r128_support = True + + +class R128Test: + def test_cli_does_not_skip_wrong_tag_type(self): + """Check that items that have tags of the wrong type won't be skipped.""" + album_rg = self._add_album(1) + item_rg = album_rg.items()[0] + + album_r128 = self._add_album(1, ext="opus") + item_r128 = album_r128.items()[0] + + item_rg.r128_track_gain = 0.0 + item_rg.store() + + item_r128.rg_track_gain = 0.0 + item_r128.rg_track_peak = 42.0 + item_r128.store() + + self.run_command("replaygain") + item_rg.load() + item_r128.load() + + self.assertIsNotNone(item_rg.rg_track_gain) + self.assertIsNotNone(item_rg.rg_track_peak) + # FIXME: Should the plugin null this field? + # self.assertIsNone(item_rg.r128_track_gain) + + self.assertIsNotNone(item_r128.r128_track_gain) + # FIXME: Should the plugin null these fields? + # self.assertIsNone(item_r128.rg_track_gain) + # self.assertIsNone(item_r128.rg_track_peak) + + def test_cli_writes_only_r128_tags(self): + album = self._add_album(2, ext="opus") + + self.run_command("replaygain", "-a") + + for item in album.items(): + mediafile = MediaFile(item.path) + # does not write REPLAYGAIN_* tags + self.assertIsNone(mediafile.rg_track_gain) + self.assertIsNone(mediafile.rg_album_gain) + # writes R128_* tags + self.assertIsNotNone(mediafile.r128_track_gain) + self.assertIsNotNone(mediafile.r128_album_gain) + + def test_r128_targetlevel_has_effect(self): + album = self._add_album(1, ext="opus") + item = album.items()[0] + + def analyse(target_level): + self.config["replaygain"]["r128_targetlevel"] = target_level + self.run_command("replaygain", "-f") + item.load() + return item.r128_track_gain + + gain_relative_to_84 = analyse(84) + gain_relative_to_89 = analyse(89) + + self.assertNotEqual(gain_relative_to_84, gain_relative_to_89) + + def test_r128_cli_skips_calculated_tracks(self): + album_r128 = self._add_album(1, ext="opus") + item_r128 = album_r128.items()[0] + + self.run_command("replaygain") + + item_r128.load() + self.assertIsNotNone(item_r128.r128_track_gain) + self.assertIsNone(item_r128.rg_track_gain) + self.assertIsNone(item_r128.rg_track_peak) + + item_r128.r128_track_gain += 1.0 + item_r128.store() + r128_track_gain = item_r128.r128_track_gain + + self.run_command("replaygain") + + item_r128.load() + self.assertEqual(item_r128.r128_track_gain, r128_track_gain) class ReplayGainCliTestBase(TestHelper): @@ -96,10 +173,6 @@ def test_cli_skips_calculated_tracks(self): album_rg = self._add_album(1) item_rg = album_rg.items()[0] - if self.has_r128_support: - album_r128 = self._add_album(1, ext="opus") - item_r128 = album_r128.items()[0] - self.run_command("replaygain") item_rg.load() @@ -113,64 +186,12 @@ def test_cli_skips_calculated_tracks(self): rg_track_gain = item_rg.rg_track_gain rg_track_peak = item_rg.rg_track_peak - if self.has_r128_support: - item_r128.load() - self.assertIsNotNone(item_r128.r128_track_gain) - self.assertIsNone(item_r128.rg_track_gain) - self.assertIsNone(item_r128.rg_track_peak) - - item_r128.r128_track_gain += 1.0 - item_r128.store() - r128_track_gain = item_r128.r128_track_gain - self.run_command("replaygain") item_rg.load() self.assertEqual(item_rg.rg_track_gain, rg_track_gain) self.assertEqual(item_rg.rg_track_peak, rg_track_peak) - if self.has_r128_support: - item_r128.load() - self.assertEqual(item_r128.r128_track_gain, r128_track_gain) - - def test_cli_does_not_skip_wrong_tag_type(self): - """Check that items that have tags of the wrong type won't be skipped.""" - if not self.has_r128_support: - # This test is a lot less interesting if the backend cannot write - # both tag types. - self.skipTest( - "r128 tags for opus not supported on backend {}".format( - self.backend - ) - ) - - album_rg = self._add_album(1) - item_rg = album_rg.items()[0] - - album_r128 = self._add_album(1, ext="opus") - item_r128 = album_r128.items()[0] - - item_rg.r128_track_gain = 0.0 - item_rg.store() - - item_r128.rg_track_gain = 0.0 - item_r128.rg_track_peak = 42.0 - item_r128.store() - - self.run_command("replaygain") - item_rg.load() - item_r128.load() - - self.assertIsNotNone(item_rg.rg_track_gain) - self.assertIsNotNone(item_rg.rg_track_peak) - # FIXME: Should the plugin null this field? - # self.assertIsNone(item_rg.r128_track_gain) - - self.assertIsNotNone(item_r128.r128_track_gain) - # FIXME: Should the plugin null these fields? - # self.assertIsNone(item_r128.rg_track_gain) - # self.assertIsNone(item_r128.rg_track_peak) - def test_cli_saves_album_gain_to_file(self): self._add_album(2) @@ -195,27 +216,6 @@ def test_cli_saves_album_gain_to_file(self): self.assertNotEqual(max(gains), 0.0) self.assertNotEqual(max(peaks), 0.0) - def test_cli_writes_only_r128_tags(self): - if not self.has_r128_support: - self.skipTest( - "r128 tags for opus not supported on backend {}".format( - self.backend - ) - ) - - album = self._add_album(2, ext="opus") - - self.run_command("replaygain", "-a") - - for item in album.items(): - mediafile = MediaFile(item.path) - # does not write REPLAYGAIN_* tags - self.assertIsNone(mediafile.rg_track_gain) - self.assertIsNone(mediafile.rg_album_gain) - # writes R128_* tags - self.assertIsNotNone(mediafile.r128_track_gain) - self.assertIsNotNone(mediafile.r128_album_gain) - def test_targetlevel_has_effect(self): album = self._add_album(1) item = album.items()[0] @@ -231,28 +231,6 @@ def analyse(target_level): self.assertNotEqual(gain_relative_to_84, gain_relative_to_89) - def test_r128_targetlevel_has_effect(self): - if not self.has_r128_support: - self.skipTest( - "r128 tags for opus not supported on backend {}".format( - self.backend - ) - ) - - album = self._add_album(1, ext="opus") - item = album.items()[0] - - def analyse(target_level): - self.config["replaygain"]["r128_targetlevel"] = target_level - self.run_command("replaygain", "-f") - item.load() - return item.r128_track_gain - - gain_relative_to_84 = analyse(84) - gain_relative_to_89 = analyse(89) - - self.assertNotEqual(gain_relative_to_84, gain_relative_to_89) - def test_per_disc(self): # Use the per_disc option and add a little more concurrency. album = self._add_album(track_count=4, disc_count=3) @@ -267,7 +245,7 @@ def test_per_disc(self): class ReplayGainGstCliTest( - ReplayGainCliTestBase, unittest.TestCase, GstBackendMixin + R128Test, ReplayGainCliTestBase, unittest.TestCase, GstBackendMixin ): FNAME = "full" # file contains only silence @@ -279,13 +257,13 @@ class ReplayGainCmdCliTest( class ReplayGainFfmpegCliTest( - ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin + R128Test, ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin ): FNAME = "full" # file contains only silence class ReplayGainFfmpegNoiseCliTest( - ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin + R128Test, ReplayGainCliTestBase, unittest.TestCase, FfmpegBackendMixin ): FNAME = "whitenoise" From 1d6781d06a7f8c11930750cb78be337a9f8fd877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 15:56:43 +0100 Subject: [PATCH 06/12] artresizer: always run PILBackend tests --- .github/workflows/ci.yaml | 2 +- test/test_art_resize.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f121aa3017..48d1653ef3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -39,7 +39,7 @@ jobs: gstreamer1.0-plugins-good \ libgirepository1.0-dev \ mp3gain - poetry install --extras replaygain + poetry install --extras replaygain --extras embedart - name: Install Python dependencies run: poetry install --only=main,test diff --git a/test/test_art_resize.py b/test/test_art_resize.py index ac9463cba4..450a736bdb 100644 --- a/test/test_art_resize.py +++ b/test/test_art_resize.py @@ -110,7 +110,6 @@ def _test_img_resize(self, backend): os.stat(syspath(im_b)).st_size, os.stat(syspath(im_75_qual)).st_size ) - @unittest.skipUnless(PILBackend.available(), "PIL not available") def test_pil_file_resize(self): """Test PIL resize function is lowering file size.""" self._test_img_resize(PILBackend()) @@ -120,7 +119,6 @@ def test_im_file_resize(self): """Test IM resize function is lowering file size.""" self._test_img_resize(IMBackend()) - @unittest.skipUnless(PILBackend.available(), "PIL not available") def test_pil_file_deinterlace(self): """Test PIL deinterlace function. From 2a1acb3ba2dbdc76b8cb21ea8a6e372e38abfe88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 15 Jul 2024 18:18:41 +0100 Subject: [PATCH 07/12] artresizer: always run IMBackend tests --- test/test_art_resize.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_art_resize.py b/test/test_art_resize.py index 450a736bdb..ac571a0fb6 100644 --- a/test/test_art_resize.py +++ b/test/test_art_resize.py @@ -114,7 +114,6 @@ def test_pil_file_resize(self): """Test PIL resize function is lowering file size.""" self._test_img_resize(PILBackend()) - @unittest.skipUnless(IMBackend.available(), "ImageMagick not available") def test_im_file_resize(self): """Test IM resize function is lowering file size.""" self._test_img_resize(IMBackend()) @@ -131,7 +130,6 @@ def test_pil_file_deinterlace(self): with Image.open(path) as img: self.assertNotIn("progression", img.info) - @unittest.skipUnless(IMBackend.available(), "ImageMagick not available") def test_im_file_deinterlace(self): """Test ImageMagick deinterlace function. From ec948978b0a199dc1bff3058b20c7b6b6f00b20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 16 Jul 2024 13:20:00 +0100 Subject: [PATCH 08/12] ui: always run tests that need bash --- .github/workflows/ci.yaml | 1 + test/test_ui.py | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 48d1653ef3..5319bd7f7f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -34,6 +34,7 @@ jobs: run: | sudo apt update sudo apt install \ + bash-completion \ ffmpeg \ gobject-introspection \ gstreamer1.0-plugins-good \ diff --git a/test/test_ui.py b/test/test_ui.py index aed49fbe08..f3814c4c8b 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -1411,7 +1411,6 @@ def test_plugin_command_from_pluginpath(self): @_common.slow_test() -@unittest.skipIf(not shutil.which("bash"), "bash not available") class CompletionTest(_common.TestCase, TestHelper): def test_completion(self): # Load plugin commands @@ -1436,12 +1435,12 @@ def test_completion(self): bash_completion = path break else: - self.skipTest("bash-completion script not found") + self.fail("bash-completion script not found") try: with open(util.syspath(bash_completion), "rb") as f: tester.stdin.writelines(f) except OSError: - self.skipTest("could not read bash-completion script") + self.fail("could not read bash-completion script") # Load completion script. self.io.install() From 6f7174f8443c8e61f0dd591ce56c058ce4f895bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 16 Jul 2024 13:36:19 +0100 Subject: [PATCH 09/12] importer: always test .rar archives unrar is actually not needed here. --- test/test_importer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_importer.py b/test/test_importer.py index 9ceeaad323..b857cde6c1 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -321,7 +321,6 @@ def create_archive(self): return path -@unittest.skipIf(not shutil.which("unrar"), "unrar program not found") class ImportRarTest(ImportZipTest): def create_archive(self): return os.path.join(_common.RSRC, b"archive.rar") From 31fba03f26ddc7518761771271a4dd2fc4d26b12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 16 Jul 2024 13:57:53 +0100 Subject: [PATCH 10/12] lyrics: assume bs4 is always available --- .github/workflows/ci.yaml | 5 ++- test/plugins/test_lyrics.py | 78 +++++++------------------------------ 2 files changed, 19 insertions(+), 64 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5319bd7f7f..3dce51f34e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -40,7 +40,10 @@ jobs: gstreamer1.0-plugins-good \ libgirepository1.0-dev \ mp3gain - poetry install --extras replaygain --extras embedart + poetry install \ + --extras embedart \ + --extras lyrics \ + --extras replaygain - name: Install Python dependencies run: poetry install --only=main,test diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py index 2953c1e910..3fddcab311 100644 --- a/test/plugins/test_lyrics.py +++ b/test/plugins/test_lyrics.py @@ -249,16 +249,12 @@ def assertLyricsContentOk(self, title, text, msg=""): # noqa: N802 LYRICS_TEXTS = confuse.load_yaml(yaml_path) -class LyricsGoogleBaseTest(unittest.TestCase): +class LyricsTestCase(unittest.TestCase): def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") + self.plugin = lyrics.LyricsPlugin() -class LyricsPluginSourcesTest(LyricsGoogleBaseTest, LyricsAssertions): +class LyricsPluginSourcesTest(LyricsTestCase, LyricsAssertions): """Check that beets google custom search engine sources are correctly scraped. """ @@ -346,10 +342,6 @@ class LyricsPluginSourcesTest(LyricsGoogleBaseTest, LyricsAssertions): ), ] - def setUp(self): - LyricsGoogleBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - @unittest.skipUnless( os.environ.get("INTEGRATION_TEST", "0") == "1", "integration testing not enabled", @@ -383,7 +375,7 @@ def test_google_sources_ok(self): self.assertLyricsContentOk(s["title"], res, url) -class LyricsGooglePluginMachineryTest(LyricsGoogleBaseTest, LyricsAssertions): +class LyricsGooglePluginMachineryTest(LyricsTestCase, LyricsAssertions): """Test scraping heuristics on a fake html page.""" source = dict( @@ -393,11 +385,6 @@ class LyricsGooglePluginMachineryTest(LyricsGoogleBaseTest, LyricsAssertions): path="/lyrics/beetssong", ) - def setUp(self): - """Set up configuration""" - LyricsGoogleBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - @patch.object(lyrics.Backend, "fetch_url", MockFetchUrl()) def test_mocked_source_ok(self): """Test that lyrics of the mocked page are correctly scraped""" @@ -461,23 +448,9 @@ def test_is_page_candidate_special_chars(self): # test Genius backend -class GeniusBaseTest(unittest.TestCase): - def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") - - -class GeniusScrapeLyricsFromHtmlTest(GeniusBaseTest): +class GeniusScrapeLyricsFromHtmlTest(LyricsTestCase): """tests Genius._scrape_lyrics_from_html()""" - def setUp(self): - """Set up configuration""" - GeniusBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - def test_no_lyrics_div(self): """Ensure we don't crash when the scraping the html for a genius page doesn't contain
@@ -507,14 +480,9 @@ def test_good_lyrics_multiple_divs(self): # TODO: find an example of a lyrics page with multiple divs and test it -class GeniusFetchTest(GeniusBaseTest): +class GeniusFetchTest(LyricsTestCase): """tests Genius.fetch()""" - def setUp(self): - """Set up configuration""" - GeniusBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - @patch.object(lyrics.Genius, "_scrape_lyrics_from_html") @patch.object(lyrics.Backend, "fetch_url", return_value=True) def test_json(self, mock_fetch_url, mock_scrape): @@ -567,22 +535,12 @@ def test_json(self, mock_fetch_url, mock_scrape): # test Tekstowo -class TekstowoBaseTest(unittest.TestCase): - def setUp(self): - """Set up configuration.""" - try: - __import__("bs4") - except ImportError: - self.skipTest("Beautiful Soup 4 not available") - - -class TekstowoExtractLyricsTest(TekstowoBaseTest): +class TekstowoExtractLyricsTest(LyricsTestCase): """tests Tekstowo.extract_lyrics()""" def setUp(self): """Set up configuration""" - TekstowoBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() + super().setUp() tekstowo.config = self.plugin.config def test_good_lyrics(self): @@ -628,14 +586,9 @@ def test_song_no_match(self): ) -class TekstowoParseSearchResultsTest(TekstowoBaseTest): +class TekstowoParseSearchResultsTest(LyricsTestCase): """tests Tekstowo.parse_search_results()""" - def setUp(self): - """Set up configuration""" - TekstowoBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() - def test_multiple_results(self): """Ensure we are able to scrape a page with multiple search results""" url = ( @@ -659,13 +612,12 @@ def test_no_results(self): self.assertEqual(tekstowo.parse_search_results(mock(url)), None) -class TekstowoIntegrationTest(TekstowoBaseTest, LyricsAssertions): +class TekstowoIntegrationTest(LyricsTestCase, LyricsAssertions): """Tests Tekstowo lyric source with real requests""" def setUp(self): """Set up configuration""" - TekstowoBaseTest.setUp(self) - self.plugin = lyrics.LyricsPlugin() + super().setUp() tekstowo.config = self.plugin.config @unittest.skipUnless( @@ -693,9 +645,9 @@ def test_no_matching_results(self): # test LRCLib backend -class LRCLibLyricsTest(unittest.TestCase): +class LRCLibLyricsTest(LyricsTestCase): def setUp(self): - self.plugin = lyrics.LyricsPlugin() + super().setUp() lrclib.config = self.plugin.config @patch("beetsplug.lyrics.requests.get") @@ -750,9 +702,9 @@ def test_fetch_exception(self, mock_get): self.assertIsNone(lyrics) -class LRCLibIntegrationTest(LyricsAssertions, unittest.TestCase): +class LRCLibIntegrationTest(LyricsTestCase, LyricsAssertions): def setUp(self): - self.plugin = lyrics.LyricsPlugin() + super().setUp() lrclib.config = self.plugin.config @unittest.skipUnless( From 370ae436342f58c33c69098c2cc1b012a7a195d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 16 Jul 2024 14:00:06 +0100 Subject: [PATCH 11/12] art: assume local backend is available --- test/plugins/test_art.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 8a2aa5870a..1c5355eb06 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -884,21 +884,12 @@ def _assert_image_operated(self, image_file, operation, should_operate): self.plugin.art_for_album(self.album, [""], True) self.assertEqual(mock_operation.called, should_operate) - def _require_backend(self): - """Skip the test if the art resizer doesn't have ImageMagick or - PIL (so comparisons and measurements are unavailable). - """ - if not ArtResizer.shared.local: - self.skipTest("ArtResizer has no local imaging backend available") - def test_respect_minwidth(self): - self._require_backend() self.plugin.minwidth = 300 self._assertImageIsValidArt(self.IMG_225x225, False) self._assertImageIsValidArt(self.IMG_348x348, True) def test_respect_enforce_ratio_yes(self): - self._require_backend() self.plugin.enforce_ratio = True self._assertImageIsValidArt(self.IMG_500x490, False) self._assertImageIsValidArt(self.IMG_225x225, True) @@ -908,60 +899,50 @@ def test_respect_enforce_ratio_no(self): self._assertImageIsValidArt(self.IMG_500x490, True) def test_respect_enforce_ratio_px_above(self): - self._require_backend() self.plugin.enforce_ratio = True self.plugin.margin_px = 5 self._assertImageIsValidArt(self.IMG_500x490, False) def test_respect_enforce_ratio_px_below(self): - self._require_backend() self.plugin.enforce_ratio = True self.plugin.margin_px = 15 self._assertImageIsValidArt(self.IMG_500x490, True) def test_respect_enforce_ratio_percent_above(self): - self._require_backend() self.plugin.enforce_ratio = True self.plugin.margin_percent = (500 - 490) / 500 * 0.5 self._assertImageIsValidArt(self.IMG_500x490, False) def test_respect_enforce_ratio_percent_below(self): - self._require_backend() self.plugin.enforce_ratio = True self.plugin.margin_percent = (500 - 490) / 500 * 1.5 self._assertImageIsValidArt(self.IMG_500x490, True) def test_resize_if_necessary(self): - self._require_backend() self.plugin.maxwidth = 300 self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False) self._assert_image_operated(self.IMG_348x348, self.RESIZE_OP, True) def test_fileresize(self): - self._require_backend() self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) def test_fileresize_if_necessary(self): - self._require_backend() self.plugin.max_filesize = self.IMG_225x225_SIZE self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, False) self._assertImageIsValidArt(self.IMG_225x225, True) def test_fileresize_no_scale(self): - self._require_backend() self.plugin.maxwidth = 300 self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) def test_fileresize_and_scale(self): - self._require_backend() self.plugin.maxwidth = 200 self.plugin.max_filesize = self.IMG_225x225_SIZE // 2 self._assert_image_operated(self.IMG_225x225, self.RESIZE_OP, True) def test_deinterlace(self): - self._require_backend() self.plugin.deinterlace = True self._assert_image_operated(self.IMG_225x225, self.DEINTERLACE_OP, True) self.plugin.deinterlace = False @@ -970,7 +951,6 @@ def test_deinterlace(self): ) def test_deinterlace_and_resize(self): - self._require_backend() self.plugin.maxwidth = 300 self.plugin.deinterlace = True self._assert_image_operated(self.IMG_348x348, self.DEINTERLACE_OP, True) From c9702a4c24adb19257833c0ee783b4b2ea9329ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 16 Jul 2024 14:42:35 +0100 Subject: [PATCH 12/12] windows: do not skip test when hiding file fails --- test/plugins/test_fetchart.py | 9 +++++---- test/test_hidden.py | 9 +++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/test/plugins/test_fetchart.py b/test/plugins/test_fetchart.py index b3307472a8..0feaa19f76 100644 --- a/test/plugins/test_fetchart.py +++ b/test/plugins/test_fetchart.py @@ -42,11 +42,12 @@ def check_cover_is_stored(self): def hide_file_windows(self): hidden_mask = 2 - success = ctypes.windll.kernel32.SetFileAttributesW( - self.cover_path, hidden_mask + self.assertTrue( + ctypes.windll.kernel32.SetFileAttributesW( + self.cover_path, hidden_mask + ), + "Could not set file attributes", ) - if not success: - self.skipTest("unable to set file attributes") def test_set_art_from_folder(self): self.touch(b"c\xc3\xb6ver.jpg", dir=self.album.path, content="IMAGE") diff --git a/test/test_hidden.py b/test/test_hidden.py index f60f1f6e94..e6bdccba2b 100644 --- a/test/test_hidden.py +++ b/test/test_hidden.py @@ -57,13 +57,10 @@ def test_windows_hidden(self): with tempfile.NamedTemporaryFile() as f: # Hide the file using - success = ctypes.windll.kernel32.SetFileAttributesW( - f.name, hidden_mask + self.assertTrue( + ctypes.windll.kernel32.SetFileAttributesW(f.name, hidden_mask), + "Could not set file attributes", ) - - if not success: - self.skipTest("unable to set file attributes") - self.assertTrue(hidden.is_hidden(f.name)) def test_other_hidden(self):