From ec066940976555335770431449b9d022b390004d Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 19:12:05 -0800 Subject: [PATCH 1/3] Makes the import converter respect the quiet and pretend flags. When the delete_originals was set, beets would print the following, regardless of the presence of the quiet parameter: convert: Removing original file /path/to/file.ext This commit ensures that the log is only printed when quiet is not present. --- beetsplug/convert.py | 25 +++++++++++++++---------- docs/changelog.rst | 4 ++++ test/test_convert.py | 27 +++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 6bc07c287..16b6e7075 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -514,17 +514,22 @@ class ConvertPlugin(BeetsPlugin): except subprocess.CalledProcessError: return - # Change the newly-imported database entry to point to the - # converted file. - source_path = item.path - item.path = dest - item.write() - item.read() # Load new audio information data. - item.store() + pretend = self.config['pretend'].get(bool) + quiet = self.config['quiet'].get(bool) - if self.config['delete_originals']: - self._log.info('Removing original file {0}', source_path) - util.remove(source_path, False) + if not pretend: + # Change the newly-imported database entry to point to the + # converted file. + source_path = item.path + item.path = dest + item.write() + item.read() # Load new audio information data. + item.store() + + if self.config['delete_originals']: + if not quiet: + self._log.info('Removing original file {0}', source_path) + util.remove(source_path, False) def _cleanup(self, task, session): for path in task.old_paths: diff --git a/docs/changelog.rst b/docs/changelog.rst index 190ed2558..d0b59245f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,10 @@ Bug fixes: ``r128_album_gain`` fields was changed from integer to float to fix loss of precision due to truncation. :bug:`4169` +* :doc:`/plugins/convert`: Files are no longer converted when running import in + ``--pretend`` mode. +* :doc:`/plugins/convert`: Deleting the original files during conversion no + longer logs output when the ``quiet`` flag is enabled. For packagers: diff --git a/test/test_convert.py b/test/test_convert.py index ce0750119..53c1563b8 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -122,9 +122,28 @@ class ImportConvertTest(unittest.TestCase, TestHelper): self.importer.run() for path in self.importer.paths: for root, dirnames, filenames in os.walk(path): - self.assertTrue(len(fnmatch.filter(filenames, '*.mp3')) == 0, - 'Non-empty import directory {}' - .format(util.displayable_path(path))) + self.assertEqual(len(fnmatch.filter(filenames, '*.mp3')), 0, + 'Non-empty import directory {}' + .format(util.displayable_path(path))) + + def test_delete_originals_keeps_originals_when_pretend_enabled(self): + import_file_count = self.get_count_of_import_files() + + self.config['convert']['delete_originals'] = True + self.config['convert']['pretend'] = True + self.importer.run() + + self.assertEqual(self.get_count_of_import_files(), import_file_count, + 'Count of files differs after running import') + + def get_count_of_import_files(self): + import_file_count = 0 + + for path in self.importer.paths: + for root, _, filenames in os.walk(path): + import_file_count += len(filenames) + + return import_file_count class ConvertCommand: @@ -264,7 +283,7 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper, self.unload_plugins() self.teardown_beets() - def test_transcode_from_lossles(self): + def test_transcode_from_lossless(self): [item] = self.add_item_fixtures(ext='flac') with control_stdin('y'): self.run_convert_path(item.path) From 0132067a29eb970a4cbd54df1fab6cf439e29b44 Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 19:35:40 -0800 Subject: [PATCH 2/3] Fix @unittest.skipIf annotations to ignore only win32 --- test/test_convert.py | 3 ++- test/test_hook.py | 15 ++++++++++----- test/test_play.py | 3 ++- test/test_query.py | 6 ++++-- test/test_ui.py | 3 ++- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/test/test_convert.py b/test/test_convert.py index 53c1563b8..493d4ecca 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -107,7 +107,8 @@ class ImportConvertTest(unittest.TestCase, TestHelper): item = self.lib.items().get() self.assertFileTag(item.path, 'convert') - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_import_original_on_convert_error(self): # `false` exits with non-zero code self.config['convert']['command'] = 'false' diff --git a/test/test_hook.py b/test/test_hook.py index 6ade06349..5049b5d24 100644 --- a/test/test_hook.py +++ b/test/test_hook.py @@ -63,7 +63,8 @@ class HookTest(_common.TestCase, TestHelper): self.assertIn('hook: invalid command ""', logs) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_hook_non_zero_exit(self): self._add_hook('test_event', 'sh -c "exit 1"') @@ -86,7 +87,8 @@ class HookTest(_common.TestCase, TestHelper): message.startswith("hook: hook for test_event failed: ") for message in logs)) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_hook_no_arguments(self): temporary_paths = [ get_temporary_path() for i in range(self.TEST_HOOK_COUNT) @@ -105,7 +107,8 @@ class HookTest(_common.TestCase, TestHelper): self.assertTrue(os.path.isfile(path)) os.remove(path) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_hook_event_substitution(self): temporary_directory = tempfile._get_default_tempdir() event_names = [f'test_event_event_{i}' for i in @@ -126,7 +129,8 @@ class HookTest(_common.TestCase, TestHelper): self.assertTrue(os.path.isfile(path)) os.remove(path) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_hook_argument_substitution(self): temporary_paths = [ get_temporary_path() for i in range(self.TEST_HOOK_COUNT) @@ -145,7 +149,8 @@ class HookTest(_common.TestCase, TestHelper): self.assertTrue(os.path.isfile(path)) os.remove(path) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_hook_bytes_interpolation(self): temporary_paths = [ get_temporary_path().encode('utf-8') diff --git a/test/test_play.py b/test/test_play.py index 2007686c7..8577aee70 100644 --- a/test/test_play.py +++ b/test/test_play.py @@ -72,7 +72,8 @@ class PlayPluginTest(unittest.TestCase, TestHelper): self.run_and_assert( open_mock, ['title:aNiceTitle'], 'echo other') - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_relative_to(self, open_mock): self.config['play']['command'] = 'echo' self.config['play']['relative_to'] = '/something' diff --git a/test/test_query.py b/test/test_query.py index 709f42bd5..14f3f082a 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -425,7 +425,8 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.albums(q) self.assert_albums_matched(results, []) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_parent_directory_no_slash(self): q = 'path:/a' results = self.lib.items(q) @@ -434,7 +435,8 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_parent_directory_with_slash(self): q = 'path:/a/' results = self.lib.items(q) diff --git a/test/test_ui.py b/test/test_ui.py index 9804b0a12..dd24fce1a 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -918,7 +918,8 @@ class ConfigTest(unittest.TestCase, TestHelper, _common.Assertions): # '--config', cli_overwrite_config_path, 'test') # self.assertEqual(config['anoption'].get(), 'cli overwrite') - @unittest.skipIf(sys.platform, 'win32') # FIXME: fails on windows + # FIXME: fails on windows + @unittest.skipIf(sys.platform == 'win32', 'win32') def test_cli_config_paths_resolve_relative_to_user_dir(self): cli_config_path = os.path.join(self.temp_dir, b'config.yaml') with open(cli_config_path, 'w') as file: From 438262844a1697a38a33db060489e272a639090a Mon Sep 17 00:00:00 2001 From: mousecloak Date: Fri, 7 Jan 2022 21:39:19 -0800 Subject: [PATCH 3/3] Fixed style violation --- beetsplug/convert.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 16b6e7075..82e62af62 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -528,7 +528,8 @@ class ConvertPlugin(BeetsPlugin): if self.config['delete_originals']: if not quiet: - self._log.info('Removing original file {0}', source_path) + self._log.info('Removing original file {0}', + source_path) util.remove(source_path, False) def _cleanup(self, task, session):