tests: Cleanup some pylint errors and share common fixture

- Cleanup some of the pylint erros in the tests
- Reorganize some of the tests functions and fixtures
- Move the one common fixture to conftest.py for sharing

tests/conftest.py

@@ -0,0 +1,27 @@
+# encoding: utf-8
+import sys
+import os
+from tempfile import NamedTemporaryFile
+import pytest
+
+# Only import HOMETRASH on supported platforms
+if sys.platform != "win32":
+    from send2trash.plat_other import HOMETRASH
+
+
+@pytest.fixture(name="test_file")
+def fixture_test_file():
+    file = NamedTemporaryFile(dir=os.path.expanduser("~"), prefix="send2trash_test", delete=False)
+    file.close()
+    # Verify file was actually created
+    assert os.path.exists(file.name) is True
+    yield file.name
+    # Cleanup trash files on supported platforms
+    if sys.platform != "win32":
+        name = os.path.basename(file.name)
+        # Remove trash files if they exist
+        if os.path.exists(os.path.join(HOMETRASH, "files", name)):
+            os.remove(os.path.join(HOMETRASH, "files", name))
+            os.remove(os.path.join(HOMETRASH, "info", name + ".trashinfo"))
+    if os.path.exists(file.name):
+        os.remove(file.name)

tests/test_plat_other.py

@@ -1,82 +1,62 @@
 # encoding: utf-8
-import pytest
 import codecs
 import os
 import sys
 from os import path as op
-from send2trash import TrashPermissionError
-
-try:
-    from configparser import ConfigParser
-except ImportError:
-    # py2
-    from ConfigParser import ConfigParser  # noqa: F401
-
 from tempfile import mkdtemp, NamedTemporaryFile
 import shutil
 import stat
 import uuid
+from configparser import ConfigParser
+import pytest
+from send2trash import TrashPermissionError
 
-if sys.platform != "win32":
+if sys.platform == "win32":
+    pytest.skip("Skipping non-windows tests", allow_module_level=True)
+else:
     import send2trash.plat_other
     from send2trash.plat_other import send2trash as s2t
+    from send2trash.plat_other import is_parent
 
-    INFO_SUFFIX = send2trash.plat_other.INFO_SUFFIX.decode()
+INFO_SUFFIX = send2trash.plat_other.INFO_SUFFIX.decode()
-    HOMETRASH = send2trash.plat_other.HOMETRASH
+HOMETRASH = send2trash.plat_other.HOMETRASH
-else:
-    pytest.skip("Skipping non-windows tests", allow_module_level=True)
 
 
-@pytest.fixture
+@pytest.fixture(name="test_files")
-def testfile():
+def fixture_test_files():
-    file = NamedTemporaryFile(dir=op.expanduser("~"), prefix="send2trash_test", delete=False)
-    file.close()
-    assert op.exists(file.name) is True
-    yield file
-    # Cleanup trash files on supported platforms
-    if sys.platform != "win32":
-        name = op.basename(file.name)
-        # Remove trash files if they exist
-        if op.exists(op.join(HOMETRASH, "files", name)):
-            os.remove(op.join(HOMETRASH, "files", name))
-            os.remove(op.join(HOMETRASH, "info", name + INFO_SUFFIX))
-    if op.exists(file.name):
-        os.remove(file.name)
-
-
-@pytest.fixture
-def testfiles():
     files = list(
         map(
             lambda index: NamedTemporaryFile(
                 dir=op.expanduser("~"),
-                prefix="send2trash_test{}".format(index),
+                prefix=f"send2trash_test{index}",
                 delete=False,
             ),
             range(10),
         )
     )
-    [file.close() for file in files]
+    for file in files:
-    assert all([op.exists(file.name) for file in files]) is True
+        file.close()
+    assert all(op.exists(file.name) for file in files) is True
     yield files
     filenames = [op.basename(file.name) for file in files]
-    [os.remove(op.join(HOMETRASH, "files", filename)) for filename in filenames]
+    for filename in filenames:
-    [os.remove(op.join(HOMETRASH, "info", filename + INFO_SUFFIX)) for filename in filenames]
+        os.remove(op.join(HOMETRASH, "files", filename))
+        os.remove(op.join(HOMETRASH, "info", filename + INFO_SUFFIX))
 
 
-def test_trash(testfile):
+def test_trash(test_file):
-    s2t(testfile.name)
+    s2t(test_file)
-    assert op.exists(testfile.name) is False
+    assert op.exists(test_file) is False
 
 
-def test_multitrash(testfiles):
+def test_multitrash(test_files):
-    filenames = [file.name for file in testfiles]
+    file_names = [file.name for file in test_files]
-    s2t(filenames)
+    s2t(file_names)
-    assert any([op.exists(filename) for filename in filenames]) is False
+    assert any(op.exists(filename) for filename in file_names) is False
 
 
 def touch(path):
-    with open(path, "a"):
+    with open(path, "a", encoding="utf-8"):
         os.utime(path, None)
 
 
@@ -86,8 +66,8 @@ def _filesys_enc():
     return codecs.lookup(enc).name
 
 
-@pytest.fixture
+@pytest.fixture(name="gen_unicode_file")
-def gen_unicode_file():
+def fixture_gen_unicode_file():
     name = "send2trash_tést1"
     file = op.join(op.expanduser(b"~"), name.encode("utf-8"))
     touch(file)
@@ -119,8 +99,6 @@ class ExtVol:
         self.trash_topdir_b = os.fsencode(self.trash_topdir)
 
         def s_getdev(path):
-            from send2trash.plat_other import is_parent
-
             st = os.lstat(path)
             if is_parent(self.trash_topdir, path):
                 return "dev"
@@ -145,8 +123,8 @@ class ExtVol:
         shutil.rmtree(self.trash_topdir)
 
 
-@pytest.fixture
+@pytest.fixture(name="gen_ext_vol")
-def gen_ext_vol():
+def fixture_gen_ext_vol():
     trash_topdir = mkdtemp(prefix="s2t")
     volume = ExtVol(trash_topdir)
     file_name = "test.txt"

tests/test_plat_win.py

@@ -2,56 +2,98 @@
 import os
 import shutil
 import sys
-import pytest
 from os import path as op
-
+import pytest
 from send2trash import send2trash as s2t
 
-# import the two versions as well as the "automatic" version
+s2t_modern = None
-if sys.platform == "win32":
+s2t_legacy = None
+
+if sys.platform != "win32":
+    pytest.skip("Skipping windows-only tests", allow_module_level=True)
+else:
+    # import the two versions as well as the "automatic" version
     from send2trash.win.modern import send2trash as s2t_modern
     from send2trash.win.legacy import send2trash as s2t_legacy
-else:
+
-    pytest.skip("Skipping windows-only tests", allow_module_level=True)
+if s2t_modern is None:
+    pytest.fail("Modern send2trash not available")
+
+if s2t_legacy is None:
+    pytest.fail("Legacy send2trash not available")
 
 
 def _create_tree(path):
-    dirname = op.dirname(path)
+    dir_name = op.dirname(path)
-    if not op.isdir(dirname):
+    if not op.isdir(dir_name):
-        os.makedirs(dirname)
+        os.makedirs(dir_name)
-    with open(path, "w") as writer:
+    with open(path, "w", encoding="utf-8") as writer:
         writer.write("send2trash test")
 
 
-@pytest.fixture
+@pytest.fixture(name="test_dir")
-def testdir(tmp_path):
+def fixture_test_dir(tmp_path):
-    dirname = "\\\\?\\" + str(tmp_path)
+    dir_name = "\\\\?\\" + str(tmp_path)
-    assert op.exists(dirname) is True
+    assert op.exists(dir_name) is True
-    yield dirname
+    yield dir_name
-    shutil.rmtree(dirname, ignore_errors=True)
+    shutil.rmtree(dir_name, ignore_errors=True)
 
 
-@pytest.fixture
+@pytest.fixture(name="test_file")
-def testfile(testdir):
+def fixture_test_file(test_dir):
-    file = op.join(testdir, "testfile.txt")
+    file = op.join(test_dir, "testfile.txt")
     _create_tree(file)
     assert op.exists(file) is True
     yield file
     # Note dir will cleanup the file
 
 
-@pytest.fixture
+@pytest.fixture(name="test_files")
-def testfiles(testdir):
+def fixture_test_files(test_dir):
-    files = [op.join(testdir, "testfile{}.txt".format(index)) for index in range(10)]
+    files = [op.join(test_dir, f"testfile{index}.txt") for index in range(10)]
-    [_create_tree(file) for file in files]
+    for file in files:
-    assert all([op.exists(file) for file in files]) is True
+        _create_tree(file)
+    assert all(op.exists(file) for file in files) is True
     yield files
     # Note dir will cleanup the files
 
 
-def _trash_folder(dir, fcn):
+# Long path tests
-    fcn(dir)
+@pytest.fixture(name="long_dir")
-    assert op.exists(dir) is False
+def fixture_long_dir(tmp_path):
+    dir_name = "\\\\?\\" + str(tmp_path)
+    name = "A" * 100
+    yield op.join(dir_name, name, name, name)
+    try:
+        shutil.rmtree(dir_name, ignore_errors=True)
+    except TypeError:
+        pass
+
+
+@pytest.fixture(name="long_file")
+def fixture_long_file(long_dir):
+    name = "A" * 100
+    path = op.join(long_dir, name + "{}.txt")
+    file = path.format("")
+    _create_tree(file)
+    assert op.exists(file) is True
+    yield file
+
+
+@pytest.fixture(name="long_files")
+def fixture_long_files(long_dir):
+    name = "A" * 100
+    path = op.join(long_dir, name + "{}.txt")
+    files = [path.format(index) for index in range(10)]
+    for file in files:
+        _create_tree(file)
+    assert all(op.exists(file) for file in files) is True
+    yield files
+
+
+def _trash_folder(folder, fcn):
+    fcn(folder)
+    assert op.exists(folder) is False
 
 
 def _trash_file(file, fcn):
@@ -61,160 +103,113 @@ def _trash_file(file, fcn):
 
 def _trash_multifile(files, fcn):
     fcn(files)
-    assert any([op.exists(file) for file in files]) is False
+    assert any(op.exists(file) for file in files) is False
 
 
-def _file_not_found(dir, fcn):
+def _file_not_found(folder, fcn):
-    file = op.join(dir, "otherfile.txt")
+    file = op.join(folder, "otherfile.txt")
     pytest.raises(OSError, fcn, file)
 
 
-def _multi_byte_unicode(dir, fcn):
+def _multi_byte_unicode(folder, fcn):
-    single_file = op.join(dir, "😇.txt")
+    single_file = op.join(folder, "😇.txt")
     _create_tree(single_file)
     assert op.exists(single_file) is True
     fcn(single_file)
     assert op.exists(single_file) is False
-    files = [op.join(dir, "😇{}.txt".format(index)) for index in range(10)]
+    files = [op.join(folder, f"😇{index}.txt") for index in range(10)]
-    [_create_tree(file) for file in files]
+    for file in files:
-    assert all([op.exists(file) for file in files]) is True
+        _create_tree(file)
+    assert all(op.exists(file) for file in files) is True
     fcn(files)
-    assert any([op.exists(file) for file in files]) is False
+    assert any(op.exists(file) for file in files) is False
 
 
-def test_trash_folder(testdir):
+def test_trash_folder(test_dir):
-    _trash_folder(testdir, s2t)
+    _trash_folder(test_dir, s2t)
 
 
-def test_trash_file(testfile):
+def test_trash_file(test_file):
-    _trash_file(testfile, s2t)
+    _trash_file(test_file, s2t)
 
 
-def test_trash_multifile(testfiles):
+def test_trash_multifile(test_files):
-    _trash_multifile(testfiles, s2t)
+    _trash_multifile(test_files, s2t)
 
 
-def test_file_not_found(testdir):
+def test_file_not_found(test_dir):
-    _file_not_found(testdir, s2t)
+    _file_not_found(test_dir, s2t)
 
 
-def test_trash_folder_modern(testdir):
+def test_trash_folder_modern(test_dir):
-    _trash_folder(testdir, s2t_modern)
+    _trash_folder(test_dir, s2t_modern)
 
 
-def test_trash_file_modern(testfile):
+def test_trash_file_modern(test_file):
-    _trash_file(testfile, s2t_modern)
+    _trash_file(test_file, s2t_modern)
 
 
-def test_trash_multifile_modern(testfiles):
+def test_trash_multifile_modern(test_files):
-    _trash_multifile(testfiles, s2t_modern)
+    _trash_multifile(test_files, s2t_modern)
 
 
-def test_file_not_found_modern(testdir):
+def test_file_not_found_modern(test_dir):
-    _file_not_found(testdir, s2t_modern)
+    _file_not_found(test_dir, s2t_modern)
 
 
-def test_multi_byte_unicode_modern(testdir):
+def test_multi_byte_unicode_modern(test_dir):
-    _multi_byte_unicode(testdir, s2t_modern)
+    _multi_byte_unicode(test_dir, s2t_modern)
-
-
-def test_trash_folder_legacy(testdir):
-    _trash_folder(testdir, s2t_legacy)
-
-
-def test_trash_file_legacy(testfile):
-    _trash_file(testfile, s2t_legacy)
-
-
-def test_trash_multifile_legacy(testfiles):
-    _trash_multifile(testfiles, s2t_legacy)
-
-
-def test_file_not_found_legacy(testdir):
-    _file_not_found(testdir, s2t_legacy)
-
-
-def test_multi_byte_unicode_legacy(testdir):
-    _multi_byte_unicode(testdir, s2t_legacy)
-
-
-# Long path tests
-@pytest.fixture
-def longdir(tmp_path):
-    dirname = "\\\\?\\" + str(tmp_path)
-    name = "A" * 100
-    yield op.join(dirname, name, name, name)
-    try:
-        shutil.rmtree(dirname, ignore_errors=True)
-    except TypeError:
-        pass
-
-
-@pytest.fixture
-def longfile(longdir):
-    name = "A" * 100
-    path = op.join(longdir, name + "{}.txt")
-    file = path.format("")
-    _create_tree(file)
-    assert op.exists(file) is True
-    yield file
-
-
-@pytest.fixture
-def longfiles(longdir):
-    name = "A" * 100
-    path = op.join(longdir, name + "{}.txt")
-    files = [path.format(index) for index in range(10)]
-    [_create_tree(file) for file in files]
-    assert all([op.exists(file) for file in files]) is True
-    yield files
 
 
 # NOTE: both legacy and modern test "pass" on windows, however sometimes with the same path
 # they do not actually recycle files but delete them.  Noticed this when testing with the
 # recycle bin open, noticed later tests actually worked, modern version can actually detect
 # when this happens but not stop it at this moment, and we need a way to verify it when testing.
-def test_trash_long_file_modern(longfile):
+def test_trash_long_file_modern(long_file):
-    _trash_file(longfile, s2t_modern)
+    _trash_file(long_file, s2t_modern)
 
 
-def test_trash_long_multifile_modern(longfiles):
+def test_trash_long_multifile_modern(long_files):
-    _trash_multifile(longfiles, s2t_modern)
+    _trash_multifile(long_files, s2t_modern)
-
-
-#     @pytest.skipif(
-#         op.splitdrive(os.getcwd())[0] != op.splitdrive(gettempdir())[0],
-#         "Cannot trash long path from other drive",
-#     )
-#     def test_trash_long_folder_modern(self):
-#         self._trash_folder(s2t_modern)
-
-
-def test_trash_long_file_legacy(longfile):
-    _trash_file(longfile, s2t_legacy)
-
-
-def test_trash_long_multifile_legacy(longfiles):
-    _trash_multifile(longfiles, s2t_legacy)
-
-
-#     @pytest.skipif(
-#         op.splitdrive(os.getcwd())[0] != op.splitdrive(gettempdir())[0],
-#         "Cannot trash long path from other drive",
-#     )
-#     def test_trash_long_folder_legacy(self):
-#         self._trash_folder(s2t_legacy)
-
-
-def test_trash_nothing_legacy():
-    try:
-        s2t_legacy([])
-    except Exception as ex:
-        assert False, "Exception thrown when trashing nothing: {}".format(ex)
 
 
 def test_trash_nothing_modern():
     try:
         s2t_modern([])
     except Exception as ex:
-        assert False, "Exception thrown when trashing nothing: {}".format(ex)
+        assert False, f"Exception thrown when trashing nothing: {ex}"
+
+
+def test_trash_folder_legacy(test_dir):
+    _trash_folder(test_dir, s2t_legacy)
+
+
+def test_trash_file_legacy(test_file):
+    _trash_file(test_file, s2t_legacy)
+
+
+def test_trash_multifile_legacy(test_files):
+    _trash_multifile(test_files, s2t_legacy)
+
+
+def test_file_not_found_legacy(test_dir):
+    _file_not_found(test_dir, s2t_legacy)
+
+
+def test_multi_byte_unicode_legacy(test_dir):
+    _multi_byte_unicode(test_dir, s2t_legacy)
+
+
+def test_trash_long_file_legacy(long_file):
+    _trash_file(long_file, s2t_legacy)
+
+
+def test_trash_long_multifile_legacy(long_files):
+    _trash_multifile(long_files, s2t_legacy)
+
+
+def test_trash_nothing_legacy():
+    try:
+        s2t_legacy([])
+    except Exception as ex:
+        assert False, f"Exception thrown when trashing nothing: {ex}"

tests/test_script_main.py

@@ -1,41 +1,16 @@
 # encoding: utf-8
-import os
-import sys
-import pytest
-from tempfile import NamedTemporaryFile
 from os import path as op
+import pytest
 
 from send2trash.__main__ import main as trash_main
 
-# Only import HOMETRASH on supported platforms
+
-if sys.platform != "win32":
+def test_trash(test_file):
-    from send2trash.plat_other import HOMETRASH
+    trash_main(["-v", test_file])
+    assert op.exists(test_file) is False
 
 
-@pytest.fixture
+def test_no_args(test_file):
-def file():
-    file = NamedTemporaryFile(dir=op.expanduser("~"), prefix="send2trash_test", delete=False)
-    file.close()
-    # Verify file was actually created
-    assert op.exists(file.name) is True
-    yield file.name
-    # Cleanup trash files on supported platforms
-    if sys.platform != "win32":
-        name = op.basename(file.name)
-        # Remove trash files if they exist
-        if op.exists(op.join(HOMETRASH, "files", name)):
-            os.remove(op.join(HOMETRASH, "files", name))
-            os.remove(op.join(HOMETRASH, "info", name + ".trashinfo"))
-    if op.exists(file.name):
-        os.remove(file.name)
-
-
-def test_trash(file):
-    trash_main(["-v", file])
-    assert op.exists(file) is False
-
-
-def test_no_args(file):
     pytest.raises(SystemExit, trash_main, [])
     pytest.raises(SystemExit, trash_main, ["-v"])
-    assert op.exists(file) is True
+    assert op.exists(test_file) is True