From 9af9eb9b31b85fef5cdc666eae95c71b2d5cc1dc Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Mon, 27 Jul 2020 11:43:34 +0200 Subject: [PATCH] tests: make use of our custom filesystem assertions for conciseness this replaces assertions of the form self.assertTrue(os.path.exists(syspath(path))) by self.assertExists(path) which includes the syspath conversion and is much easier to read. Occurences where located using git grep -E 'assert(True|False).*(isdir|isfile|exist)' --- test/_common.py | 12 ++++++++++++ test/test_convert.py | 22 +++++++++------------- test/test_files.py | 32 ++++++++++++++------------------ test/test_importer.py | 4 +--- test/test_smartplaylist.py | 9 +++++---- test/test_ui.py | 8 ++++---- 6 files changed, 45 insertions(+), 42 deletions(-) diff --git a/test/_common.py b/test/_common.py index ccf6d2e59..065e36dbe 100644 --- a/test/_common.py +++ b/test/_common.py @@ -148,6 +148,18 @@ class Assertions: self.assertFalse(os.path.exists(syspath(path)), f'file exists: {path!r}') + def assertIsFile(self, path): # noqa + self.assertExists(path) + self.assertTrue(os.path.isfile(syspath(path)), + u'path exists, but is not a regular file: {!r}' + .format(path)) + + def assertIsDir(self, path): # noqa + self.assertExists(path) + self.assertTrue(os.path.isdir(syspath(path)), + u'path exists, but is not a directory: {!r}' + .format(path)) + def assert_equal_path(self, a, b): """Check that two paths are equal.""" self.assertEqual(util.normpath(a), util.normpath(b), diff --git a/test/test_convert.py b/test/test_convert.py index ff2eade87..ae15a3cc5 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -25,7 +25,7 @@ from test.helper import control_stdin, capture_log from mediafile import MediaFile from beets import util -from beets.util import bytestring_path, displayable_path, syspath +from beets.util import bytestring_path, displayable_path def shell_quote(text): @@ -54,9 +54,7 @@ class TestHelper(helper.TestHelper): """ display_tag = tag tag = tag.encode('utf-8') - self.assertTrue(os.path.isfile(syspath(path)), - '{} is not a file'.format( - displayable_path(path))) + self.assertIsFile(path) with open(path, 'rb') as f: f.seek(-len(display_tag), os.SEEK_END) self.assertEqual(f.read(), tag, @@ -71,9 +69,7 @@ class TestHelper(helper.TestHelper): """ display_tag = tag tag = tag.encode('utf-8') - self.assertTrue(os.path.isfile(syspath(path)), - '{} is not a file'.format( - displayable_path(path))) + self.assertIsFile(path) with open(path, 'rb') as f: f.seek(-len(tag), os.SEEK_END) self.assertNotEqual(f.read(), tag, @@ -84,7 +80,7 @@ class TestHelper(helper.TestHelper): @_common.slow_test() -class ImportConvertTest(unittest.TestCase, TestHelper): +class ImportConvertTest(_common.TestCase, TestHelper): def setUp(self): self.setup_beets(disk=True) # Converter is threaded @@ -118,7 +114,7 @@ class ImportConvertTest(unittest.TestCase, TestHelper): item = self.lib.items().get() self.assertIsNotNone(item) - self.assertTrue(os.path.isfile(syspath(item.path))) + self.assertIsFile(item.path) def test_delete_originals(self): self.config['convert']['delete_originals'] = True @@ -159,7 +155,7 @@ class ConvertCommand: @_common.slow_test() -class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand): +class ConvertCliTest(_common.TestCase, TestHelper, ConvertCommand): def setUp(self): self.setup_beets(disk=True) # Converter is threaded @@ -203,7 +199,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand): with control_stdin('n'): self.run_convert() converted = os.path.join(self.convert_dest, b'converted.mp3') - self.assertFalse(os.path.isfile(syspath(converted))) + self.assertNotExists(converted) def test_convert_keep_new(self): self.assertEqual(os.path.splitext(self.item.path)[1], b'.ogg') @@ -244,7 +240,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand): def test_pretend(self): self.run_convert('--pretend') converted = os.path.join(self.convert_dest, b'converted.mp3') - self.assertFalse(os.path.exists(syspath(converted))) + self.assertNotExists(converted) def test_empty_query(self): with capture_log('beets.convert') as logs: @@ -307,7 +303,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand): @_common.slow_test() -class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper, +class NeverConvertLossyFilesTest(_common.TestCase, TestHelper, ConvertCommand): """Test the effect of the `never_convert_lossy_files` option. """ diff --git a/test/test_files.py b/test/test_files.py index 52617aabc..c96791d68 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -277,8 +277,8 @@ class AlbumFileTest(_common.TestCase): self.ai.store() self.i.load() - self.assertFalse(os.path.exists(syspath(oldpath))) - self.assertTrue(os.path.exists(syspath(self.i.path))) + self.assertNotExists(oldpath) + self.assertExists(self.i.path) def test_albuminfo_move_copies_file(self): oldpath = self.i.path @@ -287,8 +287,8 @@ class AlbumFileTest(_common.TestCase): self.ai.store() self.i.load() - self.assertTrue(os.path.exists(syspath(oldpath))) - self.assertTrue(os.path.exists(syspath(self.i.path))) + self.assertExists(oldpath) + self.assertExists(self.i.path) @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") def test_albuminfo_move_reflinks_file(self): @@ -332,21 +332,21 @@ class ArtFileTest(_common.TestCase): self.otherdir = os.path.join(self.temp_dir, b'testotherdir') def test_art_deleted_when_items_deleted(self): - self.assertTrue(os.path.exists(syspath(self.art))) + self.assertExists(self.art) self.ai.remove(True) - self.assertFalse(os.path.exists(syspath(self.art))) + self.assertNotExists(self.art) def test_art_moves_with_album(self): - self.assertTrue(os.path.exists(syspath(self.art))) + self.assertExists(self.art) oldpath = self.i.path self.ai.album = 'newAlbum' self.ai.move() self.i.load() self.assertNotEqual(self.i.path, oldpath) - self.assertFalse(os.path.exists(syspath(self.art))) + self.assertNotExists(self.art) newart = self.lib.get_album(self.i).art_destination(self.art) - self.assertTrue(os.path.exists(syspath(newart))) + self.assertExists(newart) def test_art_moves_with_album_to_custom_dir(self): # Move the album to another directory. @@ -373,7 +373,7 @@ class ArtFileTest(_common.TestCase): self.assertEqual(ai.artpath, None) ai.set_art(newart) - self.assertTrue(os.path.exists(syspath(ai.artpath))) + self.assertExists(ai.artpath) def test_setart_to_existing_art_works(self): util.remove(self.art) @@ -390,7 +390,7 @@ class ArtFileTest(_common.TestCase): # Set the art again. ai.set_art(ai.artpath) - self.assertTrue(os.path.exists(syspath(ai.artpath))) + self.assertExists(ai.artpath) def test_setart_to_existing_but_unset_art_works(self): newart = os.path.join(self.libdir, b'newart.jpg') @@ -407,7 +407,7 @@ class ArtFileTest(_common.TestCase): # Set the art again. ai.set_art(artdest) - self.assertTrue(os.path.exists(syspath(ai.artpath))) + self.assertExists(ai.artpath) def test_setart_to_conflicting_file_gets_new_path(self): newart = os.path.join(self.libdir, b'newart.jpg') @@ -702,16 +702,12 @@ class MkDirAllTest(_common.TestCase): def test_parent_exists(self): path = os.path.join(self.temp_dir, b'foo', b'bar', b'baz', b'qux.mp3') util.mkdirall(path) - self.assertTrue(os.path.isdir(syspath( - os.path.join(self.temp_dir, b'foo', b'bar', b'baz'), - ))) + self.assertIsDir(os.path.join(self.temp_dir, b'foo', b'bar', b'baz')) def test_child_does_not_exist(self): path = os.path.join(self.temp_dir, b'foo', b'bar', b'baz', b'qux.mp3') util.mkdirall(path) - self.assertTrue(not os.path.exists(syspath( - os.path.join(self.temp_dir, b'foo', b'bar', b'baz', b'qux.mp3'), - ))) + self.assertNotExists(path) def suite(): diff --git a/test/test_importer.py b/test/test_importer.py index f3717048e..1dc6705b7 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -697,9 +697,7 @@ class ImportTest(_common.TestCase, ImportHelper): self.assertEqual(self.lib.items().get(), None) def test_skip_non_album_dirs(self): - self.assertTrue(os.path.isdir(syspath( - os.path.join(self.import_dir, b'the_album'), - ))) + self.assertIsDir(os.path.join(self.import_dir, b'the_album')) self.touch(b'cruft', dir=self.import_dir) self.importer.add_choice(importer.action.APPLY) self.importer.run() diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py index 5deb34c76..4f254bec5 100644 --- a/test/test_smartplaylist.py +++ b/test/test_smartplaylist.py @@ -28,10 +28,11 @@ from beets.util import syspath, bytestring_path, py3_path, CHAR_REPLACE from beets.ui import UserError from beets import config +from test import _common from test.helper import TestHelper -class SmartPlaylistTest(unittest.TestCase): +class SmartPlaylistTest(_common.TestCase): def test_build_queries(self): spl = SmartPlaylistPlugin() self.assertEqual(spl._matched_playlists, None) @@ -174,7 +175,7 @@ class SmartPlaylistTest(unittest.TestCase): lib.albums.assert_called_once_with(a_q, None) m3u_filepath = path.join(dir, b'ta_ga_da-my_playlist_.m3u') - self.assertTrue(path.exists(m3u_filepath)) + self.assertExists(m3u_filepath) with open(syspath(m3u_filepath), 'rb') as f: content = f.read() rmtree(syspath(dir)) @@ -182,7 +183,7 @@ class SmartPlaylistTest(unittest.TestCase): self.assertEqual(content, b'/tagada.mp3\n') -class SmartPlaylistCLITest(unittest.TestCase, TestHelper): +class SmartPlaylistCLITest(_common.TestCase, TestHelper): def setUp(self): self.setup_beets() @@ -206,7 +207,7 @@ class SmartPlaylistCLITest(unittest.TestCase, TestHelper): self.run_with_output('splupdate', 'my_playlist') m3u_path = path.join(self.temp_dir, b'my_playlist.m3u') - self.assertTrue(path.exists(m3u_path)) + self.assertExists(m3u_path) with open(syspath(m3u_path), 'rb') as f: self.assertEqual(f.read(), self.item.path + b"\n") remove(syspath(m3u_path)) diff --git a/test/test_ui.py b/test/test_ui.py index f66917dc2..9e0a67a0e 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -128,26 +128,26 @@ class RemoveTest(_common.TestCase, TestHelper): commands.remove_items(self.lib, '', False, False, False) items = self.lib.items() self.assertEqual(len(list(items)), 0) - self.assertTrue(os.path.exists(syspath(self.i.path))) + self.assertExists(self.i.path) def test_remove_items_with_delete(self): self.io.addinput('y') commands.remove_items(self.lib, '', False, True, False) items = self.lib.items() self.assertEqual(len(list(items)), 0) - self.assertFalse(os.path.exists(syspath(self.i.path))) + self.assertNotExists(self.i.path) def test_remove_items_with_force_no_delete(self): commands.remove_items(self.lib, '', False, False, True) items = self.lib.items() self.assertEqual(len(list(items)), 0) - self.assertTrue(os.path.exists(syspath(self.i.path))) + self.assertExists(self.i.path) def test_remove_items_with_force_delete(self): commands.remove_items(self.lib, '', False, True, True) items = self.lib.items() self.assertEqual(len(list(items)), 0) - self.assertFalse(os.path.exists(syspath(self.i.path))) + self.assertNotExists(self.i.path) def test_remove_items_select_with_delete(self): i2 = library.Item.from_path(self.item_path)