From f6a93ae4a117e0ed27ccced1a8dba278e1179077 Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 16:38:27 +0200 Subject: [PATCH 1/6] Use tmp_path instead of tmpdir for end2end tests tmpdir uses a py.path object which will be removed at some point and is no longer actively maintained. --- tests/end2end/conftest.py | 82 +++++++++---------- .../command/test_expand_wildcards_bdd.py | 6 +- .../features/image/test_imageopen_bdd.py | 24 +++--- 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/tests/end2end/conftest.py b/tests/end2end/conftest.py index 12a7235a7..670c1c561 100644 --- a/tests/end2end/conftest.py +++ b/tests/end2end/conftest.py @@ -7,7 +7,6 @@ """Fixtures and bdd-given steps related to setup and cleanup for end2end testing.""" import logging -import os from PyQt5.QtGui import QPixmap @@ -33,8 +32,8 @@ def ensureqtbot(qtbot): @pytest.fixture(autouse=True) -def mock_directories(tmpdir): - directory = tmpdir.join(".vimiv-data") +def mock_directories(tmp_path): + directory = tmp_path / ".vimiv-data" utils.xdg.basedir = str(directory) yield utils.xdg.basedir = None @@ -79,13 +78,13 @@ def faster_wait_times(): ############################################################################### @bdd.given("I start vimiv") @bdd.given("I open any directory") -def start_vimiv(tmpdir): - start_directory(tmpdir) +def start_vimiv(tmp_path): + start_directory(tmp_path) @bdd.given(bdd.parsers.parse("I start vimiv with {args}")) -def start_vimiv_with_args(tmpdir, args): - start_directory(tmpdir, args=args.split()) +def start_vimiv_with_args(tmp_path, args): + start_directory(tmp_path, args=args.split()) @bdd.given("I run vimiv --version") @@ -95,51 +94,51 @@ def run_vimiv_version(): @bdd.given("I start vimiv with --log-level ") -def start_vimiv_log_level(tmpdir, level): - start_directory(tmpdir, args=["--log-level", level]) +def start_vimiv_log_level(tmp_path, level): + start_directory(tmp_path, args=["--log-level", level]) @bdd.given("I open a directory for which I do not have access permissions") -def start_directory_without_permission(tmpdir): - start_directory(tmpdir, permission=0o666) +def start_directory_without_permission(tmp_path): + start_directory(tmp_path, permission=0o666) @bdd.given(bdd.parsers.parse("I open a directory with {n_children:d} paths")) -def start_directory_with_n_paths(tmpdir, n_children): - start_directory(tmpdir, n_children=n_children) +def start_directory_with_n_paths(tmp_path, n_children): + start_directory(tmp_path, n_children=n_children) @bdd.given(bdd.parsers.parse("I open a directory with {n_images:d} images")) -def start_directory_with_n_images(tmpdir, n_images): - start_directory(tmpdir, n_images=n_images) +def start_directory_with_n_images(tmp_path, n_images): + start_directory(tmp_path, n_images=n_images) @bdd.given("I open any image") -def start_any_image(tmpdir): - start_image(tmpdir) +def start_any_image(tmp_path): + start_image(tmp_path) @bdd.given(bdd.parsers.parse("I open any image of size {size}")) -def start_any_image_of_size(tmpdir, size): +def start_any_image_of_size(tmp_path, size): size = [int(elem) for elem in size.split("x")] - start_image(tmpdir, size=size) + start_image(tmp_path, size=size) @bdd.given(bdd.parsers.parse("I open {n_images:d} images")) -def start_n_images(tmpdir, n_images): - start_image(tmpdir, n_images=n_images) +def start_n_images(tmp_path, n_images): + start_image(tmp_path, n_images=n_images) @bdd.given(bdd.parsers.parse("I open {n_images:d} images with {args}")) -def start_n_images_with_args(tmpdir, n_images, args): - start_image(tmpdir, n_images=n_images, args=args.split()) +def start_n_images_with_args(tmp_path, n_images, args): + start_image(tmp_path, n_images=n_images, args=args.split()) @bdd.given(bdd.parsers.parse("I open the image '{basename}'")) -def start_image_name(tmpdir, basename): - path = str(tmpdir.join(basename)) - create_image(path) - start([path]) +def start_image_name(tmp_path, basename): + filename = str(tmp_path / basename) + create_image(filename) + start([filename]) @bdd.given("I open an animated gif") @@ -173,45 +172,46 @@ def check_stderr(output, text): ############################################################################### # helpers # ############################################################################### -def start_directory(tmpdir, n_children=0, n_images=0, permission=0o777, args=None): +def start_directory(tmp_path, n_children=0, n_images=0, permission=0o777, args=None): """Run vimiv startup using one directory as the passed path.""" args = args if args is not None else [] - directory = tmpdir.mkdir("directory") - os.chmod(str(directory), permission) + directory = tmp_path / "directory" + directory.mkdir(mode=permission) for i in range(n_children): - directory.mkdir(f"child_{i + 1:02d}") + (directory / f"child_{i + 1:02d}").mkdir() create_n_images(directory, n_images) - start([str(directory)] + args) + start([directory] + args) -def start_image(tmpdir, n_images=1, size=(300, 300), args=None): +def start_image(tmp_path, n_images=1, size=(300, 300), args=None): """Run vimiv startup using n images as the passed paths.""" - paths = create_n_images(tmpdir, n_images, size=size) - args = paths if args is None else paths + args - start(args) + paths = create_n_images(tmp_path, n_images, size=size) + argv = paths if args is None else paths + args + start(argv) def start(argv): """Run vimiv startup passing argv as argument list.""" + argv = [str(arg) for arg in argv] args = startup.setup_pre_app(argv) startup.setup_post_app(args) -def create_n_images(tmpdir, number, size=(300, 300), imgformat="jpg"): +def create_n_images(directory, number, size=(300, 300), imgformat="jpg"): paths = [] for i in range(1, number + 1): basename = f"image.{imgformat}" if number == 1 else f"image_{i:02d}.{imgformat}" - path = str(tmpdir.join(basename)) - create_image(path, size=size) + path = directory / basename + create_image(str(path.resolve()), size=size) paths.append(path) return paths -def create_image(path: str, *, size=(300, 300)): - QPixmap(*size).save(path) +def create_image(filename: str, *, size=(300, 300)): + QPixmap(*size).save(filename) class Output: diff --git a/tests/end2end/features/command/test_expand_wildcards_bdd.py b/tests/end2end/features/command/test_expand_wildcards_bdd.py index 418e571e1..7825befee 100644 --- a/tests/end2end/features/command/test_expand_wildcards_bdd.py +++ b/tests/end2end/features/command/test_expand_wildcards_bdd.py @@ -14,9 +14,11 @@ @pytest.fixture(autouse=True) -def home_directory(tmpdir, mocker): +def home_directory(tmp_path, mocker): """Fixture to mock os.path.expanduser to return a different home directory.""" - new_home = str(tmpdir.mkdir("home")) + directory = tmp_path / "home" + directory.mkdir() + new_home = str(directory) def expand_user(path): return path.replace("~", new_home) diff --git a/tests/end2end/features/image/test_imageopen_bdd.py b/tests/end2end/features/image/test_imageopen_bdd.py index b7faf5555..eb5ed6062 100644 --- a/tests/end2end/features/image/test_imageopen_bdd.py +++ b/tests/end2end/features/image/test_imageopen_bdd.py @@ -15,18 +15,18 @@ @bdd.when("I open broken images") -def open_broken_images(tmpdir): - _open_file(tmpdir, b"\211PNG\r\n\032\n") # PNG - _open_file(tmpdir, b"000000JFIF") # JPG - _open_file(tmpdir, b"GIF89a") # GIF - _open_file(tmpdir, b"II") # TIFF - _open_file(tmpdir, b"BM") # BMP +def open_broken_images(tmp_path): + _open_file(tmp_path, b"\211PNG\r\n\032\n") # PNG + _open_file(tmp_path, b"000000JFIF") # JPG + _open_file(tmp_path, b"GIF89a") # GIF + _open_file(tmp_path, b"II") # TIFF + _open_file(tmp_path, b"BM") # BMP -def _open_file(tmpdir, data): +def _open_file(directory, data): """Open a file containing the bytes from data.""" - path = str(tmpdir.join("broken")) - with open(path, "wb") as f: - f.write(data) - assert imghdr.what(path) is not None, "Invalid magic bytes in test setup" - api.open_paths([path]) + path = directory / "broken" + path.write_bytes(data) + filename = str(path) + assert imghdr.what(filename) is not None, "Invalid magic bytes in test setup" + api.open_paths([filename]) From f3197612ce1cb1254ecd88b25c86bdbbfc9dba94 Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 18:00:00 +0200 Subject: [PATCH 2/6] Use tmp_path instead of tmpdir for unit tests See also f6a93ae4a117e0ed27ccced1a8dba278e1179077. --- tests/unit/api/test_mark.py | 5 +- tests/unit/commands/test_history.py | 8 +- tests/unit/config/test_config.py | 6 +- .../unit/config/test_external_configparser.py | 4 +- tests/unit/config/test_styles.py | 10 +-- tests/unit/utils/test_files.py | 88 ++++++++++++------- tests/unit/utils/test_migration.py | 6 +- tests/unit/utils/test_thumbnail_manager.py | 20 ++--- tests/unit/utils/test_trash_manager.py | 21 ++--- tests/unit/utils/test_utils.py | 8 +- tests/unit/utils/test_xdg.py | 4 +- 11 files changed, 106 insertions(+), 74 deletions(-) diff --git a/tests/unit/api/test_mark.py b/tests/unit/api/test_mark.py index 5f69efb1a..b163232d5 100644 --- a/tests/unit/api/test_mark.py +++ b/tests/unit/api/test_mark.py @@ -28,8 +28,9 @@ def mark(qtbot, mocker, monkeypatch): @pytest.fixture(autouse=True) -def tagdir(tmpdir, mocker): - tmp_tagdir = tmpdir.mkdir("tags") +def tagdir(tmp_path, mocker): + tmp_tagdir = tmp_path / "tags" + tmp_tagdir.mkdir() mocker.patch.object(Tag, "dirname", return_value=str(tmp_tagdir)) yield str(tmp_tagdir) diff --git a/tests/unit/commands/test_history.py b/tests/unit/commands/test_history.py index b06f70b68..642884ba4 100644 --- a/tests/unit/commands/test_history.py +++ b/tests/unit/commands/test_history.py @@ -41,10 +41,10 @@ def mixed_history(): @pytest.fixture() -def history_file(tmpdir, mocker): - path = str(tmpdir.join("history")) - mocker.patch.object(vimiv.commands.history, "filename", return_value=path) - yield path +def history_file(tmp_path, mocker): + filename = str(tmp_path / "history") + mocker.patch.object(vimiv.commands.history, "filename", return_value=filename) + yield filename def test_update_history(history): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index ae3422895..585abf4fa 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -26,7 +26,7 @@ ("[SECTION]\na=0\na=1\n", "duplicate key"), ], ) -def test_sysexit_on_broken_config(mocker, tmpdir, content, message): +def test_sysexit_on_broken_config(mocker, tmp_path, content, message): """Ensure SystemExit is correctly raised for various broken config files. Args: @@ -36,8 +36,8 @@ def test_sysexit_on_broken_config(mocker, tmpdir, content, message): print("Ensuring system exit with", message) mock_logger = mocker.Mock() parser = configparser.ConfigParser() - path = tmpdir.join("configfile") - path.write(content) + path = tmp_path / "configfile" + path.write_text(content) with pytest.raises(SystemExit, match=str(customtypes.Exit.err_config)): config.read_log_exception(parser, mock_logger, str(path)) mock_logger.critical.assert_called_once() diff --git a/tests/unit/config/test_external_configparser.py b/tests/unit/config/test_external_configparser.py index e12ba7204..80a1aad35 100644 --- a/tests/unit/config/test_external_configparser.py +++ b/tests/unit/config/test_external_configparser.py @@ -27,11 +27,11 @@ def parser(): @pytest.fixture() -def config(tmpdir): +def config(tmp_path): """Fixture to retrieve a written config file with external references.""" parser = configparser.ConfigParser() parser[SECTION_NAME][OPTION_NAME] = "${env:" + ENV_VARIABLE.name + "}" - path = tmpdir.join("config.ini") + path = tmp_path / "config.ini" with open(path, "w") as f: parser.write(f) yield str(path) diff --git a/tests/unit/config/test_styles.py b/tests/unit/config/test_styles.py index 7d668fc0b..b4af8b170 100644 --- a/tests/unit/config/test_styles.py +++ b/tests/unit/config/test_styles.py @@ -20,7 +20,7 @@ def new_style(request): @pytest.fixture -def style_file(tmpdir): +def style_file(tmp_path): """Fixture to create a style file with different properties.""" def create_style_file(color="#FFF", font=None, n_colors=16, header=True, **options): @@ -33,9 +33,9 @@ def create_style_file(color="#FFF", font=None, n_colors=16, header=True, **optio header. If False, omit the STYLE section header. options: Further style options passed. """ - path = str(tmpdir.join("style")) + filename = str(tmp_path / "style") if not header: - return path + return filename parser = configparser.ConfigParser() parser.add_section("STYLE") for i in range(n_colors): @@ -44,9 +44,9 @@ def create_style_file(color="#FFF", font=None, n_colors=16, header=True, **optio parser["STYLE"]["font"] = font for key, value in options.items(): parser["STYLE"][key] = value - with open(path, "w") as f: + with open(filename, "w") as f: parser.write(f) - return path + return filename return create_style_file diff --git a/tests/unit/utils/test_files.py b/tests/unit/utils/test_files.py index f26a03b46..cdf8eae64 100644 --- a/tests/unit/utils/test_files.py +++ b/tests/unit/utils/test_files.py @@ -6,6 +6,7 @@ """Tests for vimiv.utils.files.""" +import collections import imghdr import os import tarfile @@ -30,12 +31,48 @@ def mockimghdr(mocker): @pytest.fixture() -def tmpfile(tmpdir): - path = tmpdir.join("anything") - path.write("") +def tmpfile(tmp_path): + path = tmp_path / "anything" + path.touch() yield str(path) +@pytest.fixture() +def directory_tree(tmp_path): + """Fixture to create a directory tree. + + Tree structure: + . + ├── file0 + ├── sub0 + │   ├── file0 + │   └── file1 + ├── sub1 + │   └── file0 + └── sub2 + """ + + def create_subdirectory(*, index: int, n_children: int): + subdirectory = tmp_path / f"sub{index:d}" + subdirectory.mkdir() + for i in range(n_children): + subdirectory.joinpath(f"file{i:d}").touch() + + tmp_path.joinpath("file0").touch() # File in root directory + create_subdirectory(index=0, n_children=2) # Directory with 2 files + create_subdirectory(index=1, n_children=1) # Directory with 1 file + create_subdirectory(index=2, n_children=0) # Directory with 0 files + + files = [ + "file0", + os.path.join("sub0", "file0"), + os.path.join("sub0", "file1"), + os.path.join("sub1", "file0"), + ] + + return collections.namedtuple("directorytree", ("root", "files"))(tmp_path, files) + + def test_listdir_wrapper(mocker): mocker.patch("os.listdir", return_value=["a.txt", "b.txt"]) mocker.patch("os.path.abspath", return_value="") @@ -85,30 +122,30 @@ def test_images_supported(mocker): assert not directories -def test_tar_gz_not_an_image(tmpdir): +def test_tar_gz_not_an_image(tmp_path): """Test if is_image for a tar.gz returns False. The default implementation of QImageReader.canRead returns True which is not correct. """ - outfile = str(tmpdir.join("dummy.tar.gz")) - indirectory = str(tmpdir.mkdir("indir")) - with tarfile.open(outfile, mode="w:gz") as tar: - tar.add(indirectory, arcname=os.path.basename(indirectory)) - assert files.is_image(outfile) is False + tarpath = tmp_path / "dummy.tar.gz" + tarred_directory = tmp_path / "indir" + tarred_directory.mkdir() + with tarfile.open(tarpath, mode="w:gz") as tar: + tar.add(tarred_directory, arcname=tarred_directory.name) + assert files.is_image(str(tarpath)) is False -def test_is_image_on_error(tmpdir): - path = tmpdir.join("my_file") - path.write("") - path.chmod(0o00) - assert files.is_image(path) is False +def test_is_image_on_error(tmp_path): + path = tmp_path / "my_file" + path.touch(mode=0o000) + assert files.is_image(str(path)) is False -def test_is_image_on_fifo_file(qtbot, tmpdir): - path = tmpdir.join("my_file") +def test_is_image_on_fifo_file(qtbot, tmp_path): + path = tmp_path / "my_file" os.mkfifo(path) - assert files.is_image(path) is False + assert files.is_image(str(path)) is False @pytest.mark.parametrize( @@ -139,20 +176,9 @@ def test_get_size_with_permission_error(mocker): assert files.get_size("any") == "N/A" -def test_listfiles(tmpdir): - tmpdir.join("file0").open("w") # File in root directory - sub0 = tmpdir.mkdir("sub0") # Sub-directory with two files - sub0.join("file0").open("w") - sub0.join("file1").open("w") - tmpdir.mkdir("sub1").join("file0").open("w") # Sub-directory with one file - tmpdir.mkdir("sub2") # Sub-directory with no files - expected = [ - "file0", - os.path.join("sub0", "file0"), - os.path.join("sub0", "file1"), - os.path.join("sub1", "file0"), - ] - assert sorted(expected) == sorted(files.listfiles(str(tmpdir))) +def test_listfiles(directory_tree): + expected = sorted(directory_tree.files) + assert expected == sorted(files.listfiles(str(directory_tree.root))) @pytest.mark.parametrize("name", SUPPORTED_IMAGE_FORMATS) diff --git a/tests/unit/utils/test_migration.py b/tests/unit/utils/test_migration.py index 4fa9984eb..99a84ac30 100644 --- a/tests/unit/utils/test_migration.py +++ b/tests/unit/utils/test_migration.py @@ -17,10 +17,12 @@ @pytest.fixture -def mock_gtk_version(tmpdir, monkeypatch): +def mock_gtk_version(tmp_path, monkeypatch): """Fixture to mock the xdg directories and fill them with gtk-version-like files.""" for name in ("cache", "config", "data"): - monkeypatch.setenv(f"XDG_{name.upper()}_HOME", str(tmpdir.mkdir(name))) + directory = tmp_path / name + directory.mkdir() + monkeypatch.setenv(f"XDG_{name.upper()}_HOME", str(directory)) for directory in ( xdg.vimiv_config_dir(), diff --git a/tests/unit/utils/test_thumbnail_manager.py b/tests/unit/utils/test_thumbnail_manager.py index 4ccd6419e..c1486198d 100644 --- a/tests/unit/utils/test_thumbnail_manager.py +++ b/tests/unit/utils/test_thumbnail_manager.py @@ -14,22 +14,22 @@ @pytest.fixture -def manager(qtbot, tmpdir, mocker): +def manager(qtbot, tmp_path, mocker): """Fixture to create a thumbnail manager with relevant methods mocked.""" # Mock directory in which the thumbnails are created - tmp_cache_dir = tmpdir.join("cache") + tmp_cache_dir = tmp_path / "cache" mocker.patch("vimiv.utils.xdg.user_cache_dir", return_value=str(tmp_cache_dir)) # Create thumbnail manager and yield the instance yield thumbnail_manager.ThumbnailManager(None) @pytest.mark.parametrize("n_paths", (1, 5)) -def test_create_n_thumbnails(qtbot, tmpdir, manager, n_paths): +def test_create_n_thumbnails(qtbot, tmp_path, manager, n_paths): # Create images to create thumbnails of - paths = [str(tmpdir.join(f"image_{i}.jpg")) for i in range(n_paths)] - for path in paths: - QPixmap(300, 300).save(path, "jpg") - manager.create_thumbnails_async(paths) + filenames = [str(tmp_path / f"image_{i}.jpg") for i in range(n_paths)] + for filename in filenames: + QPixmap(300, 300).save(filename, "jpg") + manager.create_thumbnails_async(filenames) check_thumbails_created(qtbot, manager, n_paths) @@ -38,9 +38,9 @@ def test_create_thumbnails_for_non_existing_path(qtbot, manager): check_thumbails_created(qtbot, manager, 0) -def test_create_thumbnails_for_non_image_path(qtbot, tmpdir, manager): - path = str(tmpdir.join("image.jpg")) - manager.create_thumbnails_async([path]) +def test_create_thumbnails_for_non_image_path(qtbot, tmp_path, manager): + filename = str(tmp_path / "image.jpg") + manager.create_thumbnails_async([filename]) check_thumbails_created(qtbot, manager, 0) diff --git a/tests/unit/utils/test_trash_manager.py b/tests/unit/utils/test_trash_manager.py index ce5debae3..cb870d9d6 100644 --- a/tests/unit/utils/test_trash_manager.py +++ b/tests/unit/utils/test_trash_manager.py @@ -15,13 +15,13 @@ @pytest.fixture(autouse=True) -def trash(monkeypatch, tmpdir): +def trash(monkeypatch, tmp_path): """Initialize trash for testing as fixture. Returns: The path to the temporary trash directory. """ - xdg_data_home = tmpdir.mkdir("data") + xdg_data_home = tmp_path / "data" monkeypatch.setenv("XDG_DATA_HOME", str(xdg_data_home)) trash_manager.init() yield @@ -29,8 +29,8 @@ def trash(monkeypatch, tmpdir): @pytest.fixture() -def deleted_file(tmpdir): - original_filename = create_tmpfile(tmpdir, "file") +def deleted_file(tmp_path): + original_filename = create_tmpfile(tmp_path, "file") trash_filename = trash_manager.delete(original_filename) info_filename = trash_manager._get_info_filename(original_filename) paths = collections.namedtuple("DeletedFile", ("original", "trash", "info")) @@ -72,8 +72,9 @@ def test_fail_undelete_non_existing_file(): trash_manager.undelete(os.path.join("any", "random", "file")) -def test_fail_undelete_non_existing_original_directory(tmpdir): - directory = tmpdir.mkdir("directory") +def test_fail_undelete_non_existing_original_directory(tmp_path): + directory = tmp_path / "directory" + directory.mkdir() original_filename = create_tmpfile(directory, "file") trash_filename = trash_manager.delete(original_filename) os.rmdir(directory) @@ -81,8 +82,8 @@ def test_fail_undelete_non_existing_original_directory(tmpdir): trash_manager.undelete(os.path.basename(trash_filename)) -def create_tmpfile(tmpdir, basename): - """Simple function to create a temporary file using the tmpdir fixture.""" - path = tmpdir.join(basename) - path.write("temporary") +def create_tmpfile(directory, basename): + """Simple function to create a temporary file using pathlib.""" + path = directory / basename + path.touch() return str(path) diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 8956dddac..70552bd99 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -264,9 +264,11 @@ def test_run_qprocess(): assert utils.run_qprocess("pwd") == os.getcwd() -def test_run_qprocess_in_other_dir(tmpdir): - directory = str(tmpdir.mkdir("directory")) - assert utils.run_qprocess("pwd", cwd=directory) == directory +def test_run_qprocess_in_other_dir(tmp_path): + directory = tmp_path / "directory" + directory.mkdir() + dirname = str(directory) + assert utils.run_qprocess("pwd", cwd=dirname) == dirname @pytest.mark.parametrize( diff --git a/tests/unit/utils/test_xdg.py b/tests/unit/utils/test_xdg.py index 5f31ca8f8..166846108 100644 --- a/tests/unit/utils/test_xdg.py +++ b/tests/unit/utils/test_xdg.py @@ -22,9 +22,9 @@ def unset_xdg_env(monkeypatch): @pytest.fixture -def mock_xdg(tmpdir, monkeypatch): +def mock_xdg(tmp_path, monkeypatch): """Set XDG_* directories to a temporary directory.""" - dirname = str(tmpdir.join("directory")) + dirname = str(tmp_path / "directory") monkeypatch.setenv("XDG_CACHE_HOME", dirname) monkeypatch.setenv("XDG_CONFIG_HOME", dirname) monkeypatch.setenv("XDG_DATA_HOME", dirname) From bed8252c5bac8815908d1f645212f426e190f3a3 Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 18:10:26 +0200 Subject: [PATCH 3/6] Use pathlib in trash_manager unit test This simplifies and clarifies some of the path logic here. --- tests/unit/utils/test_trash_manager.py | 36 ++++++++++++++------------ 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/unit/utils/test_trash_manager.py b/tests/unit/utils/test_trash_manager.py index cb870d9d6..3d2a7cd2d 100644 --- a/tests/unit/utils/test_trash_manager.py +++ b/tests/unit/utils/test_trash_manager.py @@ -8,6 +8,7 @@ import collections import os +import pathlib import pytest @@ -34,20 +35,23 @@ def deleted_file(tmp_path): trash_filename = trash_manager.delete(original_filename) info_filename = trash_manager._get_info_filename(original_filename) paths = collections.namedtuple("DeletedFile", ("original", "trash", "info")) - return paths(original_filename, trash_filename, info_filename) + return paths( + pathlib.Path(original_filename), + pathlib.Path(trash_filename), + pathlib.Path(info_filename), + ) def test_delete_file(deleted_file): - assert not os.path.exists(deleted_file.original), "Original file not deleted" - assert os.path.exists(deleted_file.trash), "File not moved to trash" + assert not deleted_file.original.exists(), "Original file not deleted" + assert deleted_file.trash.exists(), "File not moved to trash" def test_undelete_file(deleted_file): - trash_basename = os.path.basename(deleted_file.trash) - trash_manager.undelete(trash_basename) - assert os.path.exists(deleted_file.original), "Original file not restored" - assert not os.path.exists(deleted_file.trash), "File not removed from trash" - assert not os.path.exists(deleted_file.info), "File info not removed from trash" + trash_manager.undelete(deleted_file.trash.name) + assert deleted_file.original.exists(), "Original file not restored" + assert not deleted_file.trash.exists(), "File not removed from trash" + assert not deleted_file.info.exists(), "File info not removed from trash" def test_create_trashinfo(deleted_file): @@ -60,16 +64,16 @@ def test_create_trashinfo(deleted_file): def test_do_not_overwrite_trash_file(deleted_file): - with open(deleted_file.original, "w") as f: - f.write("temporary") + deleted_file.original.touch() trash_manager.delete(deleted_file.original) - assert os.path.exists(deleted_file.trash + ".2") - assert os.path.exists(deleted_file.info.replace(".trashinfo", ".2.trashinfo")) + assert os.path.exists(str(deleted_file.trash) + ".2") + assert os.path.exists(str(deleted_file.info).replace(".trashinfo", ".2.trashinfo")) -def test_fail_undelete_non_existing_file(): +def test_fail_undelete_non_existing_file(tmp_path): with pytest.raises(FileNotFoundError, match="File for"): - trash_manager.undelete(os.path.join("any", "random", "file")) + path = tmp_path / "any" / "random" / "file" + trash_manager.undelete(str(path)) def test_fail_undelete_non_existing_original_directory(tmp_path): @@ -77,9 +81,9 @@ def test_fail_undelete_non_existing_original_directory(tmp_path): directory.mkdir() original_filename = create_tmpfile(directory, "file") trash_filename = trash_manager.delete(original_filename) - os.rmdir(directory) + directory.rmdir() with pytest.raises(FileNotFoundError, match="Original directory"): - trash_manager.undelete(os.path.basename(trash_filename)) + trash_manager.undelete(pathlib.Path(trash_filename).name) def create_tmpfile(directory, basename): From 09c1d5dad6432afd85c2d87ca1bd3ed52ef31027 Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 18:20:04 +0200 Subject: [PATCH 4/6] Use touch in end2end tests to create empty file This is easier to read at least for me. --- tests/end2end/features/conftest.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/end2end/features/conftest.py b/tests/end2end/features/conftest.py index 87296a652..c57ce881c 100644 --- a/tests/end2end/features/conftest.py +++ b/tests/end2end/features/conftest.py @@ -7,6 +7,7 @@ """Fixtures and bdd-like steps for usage during end2end testing.""" import os +import pathlib from PyQt5.QtCore import Qt, QProcess, QTimer from PyQt5.QtGui import QFocusEvent @@ -253,17 +254,17 @@ def create_directory(name): @bdd.when(bdd.parsers.parse("I create the file '{name}'")) def create_file(name): - assert not os.path.exists(name), f"Not overriding existing file '{name}'" - with open(name, "w") as f: - f.write("") + path = pathlib.Path(name) + assert not path.exists(), f"Not overriding existing file '{name}'" + path.touch() @bdd.when(bdd.parsers.parse("I create the tag file '{name}'")) def create_tag_file(name): - os.makedirs(api.mark.tagdir, mode=0o700, exist_ok=True) - path = os.path.join(api.mark.tagdir, name) - with open(path, "w") as f: - f.write("") + directory = pathlib.Path(api.mark.tagdir) + directory.mkdir(mode=0o700, parents=True, exist_ok=True) + path = directory / name + path.touch() ############################################################################### From 1051210029b49766bcf15cd5dad8a0a2a19db44e Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 18:34:20 +0200 Subject: [PATCH 5/6] Use tmp_path fixture in integration tests There should now be no more calls to tmpdir :) --- tests/integration/conftest.py | 4 ++-- tests/integration/test_read_bindings.py | 2 +- tests/integration/test_read_settings.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 2a603e804..dc4c8bfee 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -24,12 +24,12 @@ def create_custom_parser(default_parser, **sections): @pytest.fixture() -def custom_configfile(tmpdir, custom_configparser): +def custom_configfile(tmp_path, custom_configparser): """Fixture to create a custom config file from a configparser.""" def create_custom_configfile(basename, read, default_parser, **sections): parser = custom_configparser(default_parser, **sections) - path = tmpdir.join(basename) + path = tmp_path / basename with open(path, "w") as f: parser.write(f) read(str(path)) diff --git a/tests/integration/test_read_bindings.py b/tests/integration/test_read_bindings.py index bd7de5b33..65499cda6 100644 --- a/tests/integration/test_read_bindings.py +++ b/tests/integration/test_read_bindings.py @@ -32,7 +32,7 @@ def reset_to_default(cleanup_helper): @pytest.fixture(scope="function") -def keyspath(tmpdir, custom_configfile, request): +def keyspath(custom_configfile, request): """Fixture to create a custom keybindings file for reading.""" yield custom_configfile( "keys.conf", keyfile.read, keyfile.get_default_parser, **request.param diff --git a/tests/integration/test_read_settings.py b/tests/integration/test_read_settings.py index 89343c6d7..2fcc1c5cc 100644 --- a/tests/integration/test_read_settings.py +++ b/tests/integration/test_read_settings.py @@ -61,7 +61,7 @@ def reset_to_default(cleanup_helper): @pytest.fixture(scope="function") -def configpath(tmpdir, custom_configfile, request): +def configpath(custom_configfile, request): yield custom_configfile( "vimiv.conf", configfile.read, configfile.get_default_parser, **request.param ) From 5b67565a6f2e0d8c0edad60017c5c71c9b2eccc9 Mon Sep 17 00:00:00 2001 From: karlch Date: Mon, 8 Jun 2020 18:40:14 +0200 Subject: [PATCH 6/6] Override tmpdir fixture to raise ValueError This ensures we do not accidentally use the tmpdir fixture instead of tmp_path in the future. --- tests/conftest.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 87abd6c56..0e7c7b317 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -134,3 +134,9 @@ def _retrieve_file_from_web(url: str, path: str) -> None: with open(path, "wb") as f: f.write(data) print("... success") + + +@pytest.fixture() +def tmpdir(): + """Override tmpdir to raise a ValueError.""" + raise ValueError("Use the 'tmp_path' fixture instead of 'tmpdir'")