From 003f4fe93883ae728ff0b89f8656d952c3baddcc Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Wed, 31 Dec 2025 09:08:04 +0900 Subject: [PATCH 1/2] test: use pytest's tmp_path_factory fixture instead of tempfile --- test/t/conftest.py | 20 +++++---------- test/t/test_man.py | 4 +-- test/t/test_scp.py | 12 ++++----- test/t/test_slapt_get.py | 25 ++++++++++--------- test/t/test_slapt_src.py | 24 +++++++++--------- test/t/test_sshfs.py | 6 +++-- test/t/unit/test_unit_compgen_filedir.py | 10 +++----- test/t/unit/test_unit_load.py | 31 ++++++++++++++++-------- 8 files changed, 67 insertions(+), 65 deletions(-) diff --git a/test/t/conftest.py b/test/t/conftest.py index dc8d848304e..803d57cf69f 100644 --- a/test/t/conftest.py +++ b/test/t/conftest.py @@ -2,10 +2,8 @@ import os import re import shlex -import shutil import subprocess import sys -import tempfile import time from enum import Enum from pathlib import Path @@ -190,7 +188,7 @@ def get_testdir(): @pytest.fixture(scope="session") def test_session_tmpdir(tmp_path_factory) -> Path: - tmpdir = tmp_path_factory.mktemp("bash-completion-test_") + tmpdir = tmp_path_factory.mktemp("bash-completion.session.") user_dir_1 = tmpdir / "bash-completion" user_dir_2 = tmpdir / "bash-completion-fallback" @@ -231,9 +229,8 @@ def test_session_tmpdir(tmp_path_factory) -> Path: @pytest.fixture(scope="class") -def bash(request, test_session_tmpdir) -> pexpect.spawn: +def bash(request, test_session_tmpdir, tmp_path_factory) -> pexpect.spawn: logfile: Optional[TextIO] = None - tmpdir = None bash = None if os.environ.get("BASH_COMPLETION_TEST_LOGFILE"): @@ -268,10 +265,8 @@ def bash(request, test_session_tmpdir) -> pexpect.spawn: testdir, "fixtures", marker.kwargs.get("cwd") ) elif "temp_cwd" in marker.kwargs and marker.kwargs.get("temp_cwd"): - tmpdir = tempfile.TemporaryDirectory( - prefix="bash-completion-test_" - ) - cwd = tmpdir.name + tmpdir = tmp_path_factory.mktemp("bash-completion.bash.") + cwd = str(tmpdir) if cwd is None: cwd = os.path.join(testdir, "fixtures") os.chdir(cwd) @@ -371,8 +366,6 @@ def bash(request, test_session_tmpdir) -> pexpect.spawn: # Clean up if bash: bash.close() - if tmpdir: - tmpdir.cleanup() if logfile and logfile != sys.stdout: logfile.close() @@ -961,7 +954,7 @@ def in_container() -> bool: def prepare_fixture_dir( - request, files: Iterable[str], dirs: Iterable[str] + tmp_path_factory, files: Iterable[str], dirs: Iterable[str] ) -> Path: """ Fixture to prepare a test dir with dummy contents on the fly. @@ -970,8 +963,7 @@ def prepare_fixture_dir( prepare a dir on the fly rather than including their fixtures in git and the tarball. This is to work better with case insensitive file systems. """ - tempdir = Path(tempfile.mkdtemp(prefix="bash-completion-fixture-dir")) - request.addfinalizer(lambda: shutil.rmtree(str(tempdir))) + tempdir = tmp_path_factory.mktemp("bash-completion.fixture_dir.") old_cwd = os.getcwd() try: diff --git a/test/t/test_man.py b/test/t/test_man.py index d61a818121d..84d6651f6e3 100644 --- a/test/t/test_man.py +++ b/test/t/test_man.py @@ -16,7 +16,7 @@ class TestMan: assumed_present = "man" @pytest.fixture - def colonpath(self, request, bash): + def colonpath(self, bash, tmp_path_factory): try: assert_bash_exec(bash, "uname -s 2>&1 | grep -qiF cygwin") except AssertionError: @@ -25,7 +25,7 @@ def colonpath(self, request, bash): pytest.skip("Cygwin doesn't like paths with colons") tmpdir = prepare_fixture_dir( - request, + tmp_path_factory, files=["man/man3/Bash::Completion.3pm.gz"], dirs=["man", "man/man3"], ) diff --git a/test/t/test_scp.py b/test/t/test_scp.py index 88b1045f425..71527d97e60 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -189,12 +189,12 @@ def prefix_paths(prefix, paths): ) @pytest.fixture - def tmpdir_backslash(self, request, bash): + def tmpdir_backslash(self, bash, tmp_path_factory): if sys.platform.startswith("win"): pytest.skip("Filenames not allowed on Windows") tmpdir = prepare_fixture_dir( - request, files=["local_path-file\\"], dirs=[] + tmp_path_factory, files=["local_path-file\\"], dirs=[] ) return tmpdir @@ -211,11 +211,11 @@ def test_remote_path_ending_with_backslash(self, bash): assert completion.output == r"thetical\\\\ " @pytest.fixture - def tmpdir_mkfifo(self, request, bash): + def tmpdir_mkfifo(self, bash, tmp_path_factory): # We prepare two files: 1) a named pipe and 2) a regular file ending # with the same name but an extra special character "|". tmpdir = prepare_fixture_dir( - request, + tmp_path_factory, files=["local_path_2-pipe|"], dirs=[], ) @@ -257,12 +257,12 @@ def test_local_path_with_spaces_2(self, completion): assert completion == r"\ conf" @pytest.fixture - def tmpdir_backslash_2(self, request, bash): + def tmpdir_backslash_2(self, bash, tmp_path_factory): if sys.platform.startswith("win"): pytest.skip("Filenames not allowed on Windows") tmpdir = prepare_fixture_dir( - request, + tmp_path_factory, files=["backslash-a b.txt", r"backslash-a\ b.txt"], dirs=[], ) diff --git a/test/t/test_slapt_get.py b/test/t/test_slapt_get.py index 924497115f2..8bd93ee0350 100644 --- a/test/t/test_slapt_get.py +++ b/test/t/test_slapt_get.py @@ -1,5 +1,4 @@ import os.path -from tempfile import mkstemp import pytest @@ -9,17 +8,19 @@ @pytest.mark.bashcomp(cmd="slapt-get") class TestSlaptGet: @pytest.fixture(scope="class") - def slapt_getrc(self, request, bash): - fd, fname = mkstemp(prefix="slapt-getrc.", text=True) - request.addfinalizer(lambda: os.remove(fname)) - with os.fdopen(fd, "w") as f: - print( - "WORKINGDIR=%s/" - % os.path.join(bash.cwd, *"slackware var slapt-get".split()), - file=f, - ) - print("SOURCE=file:///home/", file=f) - return fname + def slapt_getrc(self, bash, tmp_path_factory): + working_dir = os.path.join( + bash.cwd, *"slackware var slapt-get".split() + ) + + tmpdir = tmp_path_factory.mktemp( + "bash-completion._comp_cmd_slapt_get." + ) + tmpfile = tmpdir / "slapt-getrc.0" + tmpfile.write_text( + f"WORKINGDIR={working_dir}/\nSOURCE=file:///home/\n" + ) + return str(tmpfile) @pytest.mark.complete("slapt-get -", require_cmd=True) def test_1(self, completion): diff --git a/test/t/test_slapt_src.py b/test/t/test_slapt_src.py index b55b722d302..26c1e33ee87 100644 --- a/test/t/test_slapt_src.py +++ b/test/t/test_slapt_src.py @@ -1,5 +1,4 @@ import os -from tempfile import mkstemp import pytest @@ -9,18 +8,17 @@ @pytest.mark.bashcomp(cmd="slapt-src") class TestSlaptSrc: @pytest.fixture(scope="class") - def slapt_srcrc(self, request, bash): - fd, fname = mkstemp(prefix="slapt-srcrc.", text=True) - request.addfinalizer(lambda: os.remove(fname)) - with os.fdopen(fd, "w") as f: - print( - "BUILDDIR=%s/" - % os.path.join( - bash.cwd, *"slackware usr src slapt-src".split() - ), - file=f, - ) - return fname + def slapt_srcrc(self, bash, tmp_path_factory): + build_dir = os.path.join( + bash.cwd, *"slackware usr src slapt-src".split() + ) + + tmpdir = tmp_path_factory.mktemp( + "bash-completion._comp_cmd_slapt_src." + ) + tmpfile = tmpdir / "slapt-srcrc.0" + tmpfile.write_text(f"BUILDDIR={build_dir}/\n") + return str(tmpfile) @pytest.mark.complete("slapt-src -", require_cmd=True) def test_1(self, completion): diff --git a/test/t/test_sshfs.py b/test/t/test_sshfs.py index 7b8f8009584..d0144290013 100644 --- a/test/t/test_sshfs.py +++ b/test/t/test_sshfs.py @@ -12,12 +12,14 @@ def test_1(self, completion): assert completion @pytest.fixture - def tmpdir_backslash(self, request, bash): + def tmpdir_backslash(self, bash, tmp_path_factory): if sys.platform.startswith("win"): pytest.skip("Filenames not allowed on Windows") tmpdir = prepare_fixture_dir( - request, files=["local_path-file\\"], dirs=["local_path-dir"] + tmp_path_factory, + files=["local_path-file\\"], + dirs=["local_path-dir"], ) return tmpdir diff --git a/test/t/unit/test_unit_compgen_filedir.py b/test/t/unit/test_unit_compgen_filedir.py index dd0814e0a70..56f9fbe0532 100644 --- a/test/t/unit/test_unit_compgen_filedir.py +++ b/test/t/unit/test_unit_compgen_filedir.py @@ -1,8 +1,5 @@ import os -import shutil import sys -import tempfile -from pathlib import Path import pytest @@ -42,11 +39,12 @@ def functions(self, request, bash): ) @pytest.fixture(scope="class") - def non_windows_testdir(self, request, bash): + def non_windows_testdir(self, bash, tmp_path_factory): if sys.platform.startswith("win"): pytest.skip("Filenames not allowed on Windows") - tempdir = Path(tempfile.mkdtemp(prefix="bash-completion_filedir")) - request.addfinalizer(lambda: shutil.rmtree(str(tempdir))) + tempdir = tmp_path_factory.mktemp( + "bash-completion._comp_compgen_filedir." + ) subdir = tempdir / 'a"b' subdir.mkdir() (subdir / "d").touch() diff --git a/test/t/unit/test_unit_load.py b/test/t/unit/test_unit_load.py index 9a556b65a42..e9b12e01d50 100644 --- a/test/t/unit/test_unit_load.py +++ b/test/t/unit/test_unit_load.py @@ -2,13 +2,13 @@ import pytest -from conftest import assert_bash_exec, bash_env_saved, prepare_fixture_dir +from conftest import assert_bash_exec, bash_env_saved @pytest.mark.bashcomp(cmd=None, cwd="_comp_load") class TestCompLoad: @pytest.fixture - def fixture_dir(self, request, bash): + def fixture_dir(self, bash, tmp_path_factory): """Construct the fixture directory in a temporary directory. Some of the tests use specific setups of symbolic links. However, if @@ -24,15 +24,26 @@ def fixture_dir(self, request, bash): set up symbolic links. """ - tmpdir = prepare_fixture_dir(request, files=[], dirs=[]) + tmpdir = tmp_path_factory.mktemp("bash-completion._comp_load.") + + # Note: I tried to use + # + # shutil.copytree(os.getcwd(), tmpdir, dirs_exist_ok=True) + # + # but it turned out that shutil.copytree of Python 3.7 (used in centos7 + # and debian10 in the docker images) does not provide the + # "dirs_exist_ok" option. We continue to use "cp -R" here, but we may + # update this line after we drop centos7 and debian10. assert_bash_exec(bash, "cp -R %s/* %s/" % (os.getcwd(), tmpdir)) - assert_bash_exec(bash, "mkdir -p %s/bin" % tmpdir) - assert_bash_exec( - bash, "ln -sf ../prefix1/bin/cmd1 %s/bin/cmd1" % tmpdir - ) - assert_bash_exec( - bash, "ln -sf ../prefix1/sbin/cmd2 %s/bin/cmd2" % tmpdir - ) + + bin_dir = tmpdir / "bin" + bin_dir.mkdir() + + cmd1 = bin_dir / "cmd1" + cmd2 = bin_dir / "cmd2" + cmd1.symlink_to("../prefix1/bin/cmd1") + cmd2.symlink_to("../prefix1/sbin/cmd2") + return str(tmpdir) def test_userdir_1(self, bash, fixture_dir): From e5013f6e910942cdb691c30fa6a9dd6c0244bcb9 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Wed, 31 Dec 2025 16:00:29 +0900 Subject: [PATCH 2/2] test(TestCompLoad.fixture_dir): specify the scope Currently, the fixture directory is created for every test because the scope of the fixture is not specified. Since the contents of the fixture directory is fixed and each test does not modify its contents, it should be safe to share the fixture directory. In this patch, we specify the scope of the fixture to be "class", so that the fixture directory is shared by tests. --- test/t/unit/test_unit_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/t/unit/test_unit_load.py b/test/t/unit/test_unit_load.py index e9b12e01d50..aacea13a6c8 100644 --- a/test/t/unit/test_unit_load.py +++ b/test/t/unit/test_unit_load.py @@ -7,7 +7,7 @@ @pytest.mark.bashcomp(cmd=None, cwd="_comp_load") class TestCompLoad: - @pytest.fixture + @pytest.fixture(scope="class") def fixture_dir(self, bash, tmp_path_factory): """Construct the fixture directory in a temporary directory.