From 15a1c95c91dc65ee6f0d561ac26b5704a99832a1 Mon Sep 17 00:00:00 2001 From: delta87 Date: Thu, 26 Dec 2024 12:15:53 +0330 Subject: [PATCH 1/7] Fix scandir() crash by returning [] when directory is not found (#13083) --- AUTHORS | 1 + changelog/13083.bugfix.rst | 6 ++++++ src/_pytest/pathlib.py | 36 ++++++++++++++++++++---------------- testing/test_pathlib.py | 24 ++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 changelog/13083.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 9629e00bcfb..826556f1464 100644 --- a/AUTHORS +++ b/AUTHORS @@ -360,6 +360,7 @@ Ran Benita Raphael Castaneda Raphael Pierzina Rafal Semik +Reza Mousavi Raquel Alegre Ravi Chandra Reagan Lee diff --git a/changelog/13083.bugfix.rst b/changelog/13083.bugfix.rst new file mode 100644 index 00000000000..b9b6d29f178 --- /dev/null +++ b/changelog/13083.bugfix.rst @@ -0,0 +1,6 @@ +13083.bugfix.rst: + +Fix issue where scandir failed for empty or non-existent directories. + +- Issue: https://github.com/pytest-dev/pytest/issues/13083 +- Authors: Reza Mousavi diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 25dc69b6349..7f065297966 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -955,23 +955,27 @@ def scandir( The returned entries are sorted according to the given key. The default is to sort by name. + If the directory does not exist, return an empty list. """ - entries = [] - with os.scandir(path) as s: - # Skip entries with symlink loops and other brokenness, so the caller - # doesn't have to deal with it. - for entry in s: - try: - entry.is_file() - except OSError as err: - if _ignore_error(err): - continue - raise - entries.append(entry) - entries.sort(key=sort_key) # type: ignore[arg-type] - return entries - - + try: + entries = [] + with os.scandir(path) as s: + # Skip entries with symlink loops and other brokenness, so the caller + # doesn't have to deal with it. + for entry in s: + try: + entry.is_file() + except OSError as err: + if _ignore_error(err): + continue + raise + entries.append(entry) + entries.sort(key=sort_key) # type: ignore[arg-type] + return entries + except FileNotFoundError: + return [] + + def visit( path: str | os.PathLike[str], recurse: Callable[[os.DirEntry[str]], bool] ) -> Iterator[os.DirEntry[str]]: diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 5a13cd5a400..da3a9ad06a2 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -38,6 +38,7 @@ from _pytest.pathlib import resolve_package_path from _pytest.pathlib import resolve_pkg_root_and_module_name from _pytest.pathlib import safe_exists +from _pytest.pathlib import scandir from _pytest.pathlib import spec_matches_module_path from _pytest.pathlib import symlink_or_skip from _pytest.pathlib import visit @@ -569,6 +570,29 @@ def test_samefile_false_negatives(tmp_path: Path, monkeypatch: MonkeyPatch) -> N assert getattr(module, "foo")() == 42 +def test_scandir_with_non_existent_directory() -> None: + # Test with a directory that does not exist + non_existent_dir = "path_to_non_existent_dir" + result = scandir(non_existent_dir) + # Assert that the result is an empty list + assert result == [] + + +def test_scandir_handles_os_error(): + # Create a mock entry that will raise an OSError when is_file is called + mock_entry = unittest.mock.MagicMock() + mock_entry.is_file.side_effect = OSError("Permission denied") + # Mock os.scandir to return an iterator with our mock entry + with unittest.mock.patch("os.scandir") as mock_scandir: + mock_scandir.return_value.__enter__.return_value = [mock_entry] + # Call the scandir function with a path + # We expect an OSError to be raised here + with pytest.raises(OSError, match="Permission denied"): + scandir("/fake/path") + # Verify that the is_file method was called on the mock entry + mock_entry.is_file.assert_called_once() + + class TestImportLibMode: def test_importmode_importlib_with_dataclass( self, tmp_path: Path, ns_param: bool From 4e1ac2e09dfa12b986a30b6fb96fb2c889d2f664 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 26 Dec 2024 08:49:26 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/pathlib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 7f065297966..e8fc1a421b5 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -974,8 +974,8 @@ def scandir( return entries except FileNotFoundError: return [] - - + + def visit( path: str | os.PathLike[str], recurse: Callable[[os.DirEntry[str]], bool] ) -> Iterator[os.DirEntry[str]]: From 8b4aab6d09c1529250971580c450c359e3d12a61 Mon Sep 17 00:00:00 2001 From: delta87 <124760624+delta87@users.noreply.github.com> Date: Fri, 27 Dec 2024 01:51:59 +0330 Subject: [PATCH 3/7] Update changelog/13083.bugfix.rst Co-authored-by: Bruno Oliveira --- changelog/13083.bugfix.rst | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/changelog/13083.bugfix.rst b/changelog/13083.bugfix.rst index b9b6d29f178..fc4564755ba 100644 --- a/changelog/13083.bugfix.rst +++ b/changelog/13083.bugfix.rst @@ -1,6 +1 @@ -13083.bugfix.rst: - -Fix issue where scandir failed for empty or non-existent directories. - -- Issue: https://github.com/pytest-dev/pytest/issues/13083 -- Authors: Reza Mousavi +Fixed issue where pytest could crash if one of the collected directories got removed during collection. From a14e8f6510ba8057144ba1d831dcc7714e24d52d Mon Sep 17 00:00:00 2001 From: delta87 <124760624+delta87@users.noreply.github.com> Date: Fri, 27 Dec 2024 01:52:25 +0330 Subject: [PATCH 4/7] Update testing/test_pathlib.py Co-authored-by: Bruno Oliveira --- testing/test_pathlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index da3a9ad06a2..a3d263b7cc3 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -578,7 +578,7 @@ def test_scandir_with_non_existent_directory() -> None: assert result == [] -def test_scandir_handles_os_error(): +def test_scandir_handles_os_error() -> None: # Create a mock entry that will raise an OSError when is_file is called mock_entry = unittest.mock.MagicMock() mock_entry.is_file.side_effect = OSError("Permission denied") From cadb8ba00a8fc7d789c37d4910e0d8eed83b7aae Mon Sep 17 00:00:00 2001 From: delta87 Date: Fri, 27 Dec 2024 02:02:41 +0330 Subject: [PATCH 5/7] Refactor scandir to use narrower try/except blocks and improve clarity --- src/_pytest/pathlib.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index e8fc1a421b5..a3b68f7f5b1 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -957,23 +957,26 @@ def scandir( The default is to sort by name. If the directory does not exist, return an empty list. """ + entries = [] + # Attempt to create a scandir iterator for the given path. try: - entries = [] - with os.scandir(path) as s: - # Skip entries with symlink loops and other brokenness, so the caller - # doesn't have to deal with it. - for entry in s: - try: - entry.is_file() - except OSError as err: - if _ignore_error(err): - continue - raise - entries.append(entry) - entries.sort(key=sort_key) # type: ignore[arg-type] - return entries + scandir_iter = os.scandir(path) except FileNotFoundError: + # If the directory does not exist, return an empty list. return [] + # Use the scandir iterator in a context manager to ensure it is properly closed. + with scandir_iter as s: + for entry in s: + try: + entry.is_file() + except OSError as err: + if _ignore_error(err): + continue + # Reraise non-ignorable errors to avoid hiding issues. + raise + entries.append(entry) + entries.sort(key=sort_key) # type: ignore[arg-type] + return entries def visit( From 6b52229b94ace7ffa69ae09d682e20f56b34405f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 26 Dec 2024 22:33:54 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/pathlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index a3b68f7f5b1..7c06e2df962 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -957,7 +957,7 @@ def scandir( The default is to sort by name. If the directory does not exist, return an empty list. """ - entries = [] + entries = [] # Attempt to create a scandir iterator for the given path. try: scandir_iter = os.scandir(path) From 476e31a7fbb94f4f11bb292a80caec4b0e74b573 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 6 Jan 2025 08:11:23 -0300 Subject: [PATCH 7/7] Apply suggestions from code review --- testing/test_pathlib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index a3d263b7cc3..65a4117812f 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -581,13 +581,13 @@ def test_scandir_with_non_existent_directory() -> None: def test_scandir_handles_os_error() -> None: # Create a mock entry that will raise an OSError when is_file is called mock_entry = unittest.mock.MagicMock() - mock_entry.is_file.side_effect = OSError("Permission denied") + mock_entry.is_file.side_effect = OSError("some permission error") # Mock os.scandir to return an iterator with our mock entry with unittest.mock.patch("os.scandir") as mock_scandir: mock_scandir.return_value.__enter__.return_value = [mock_entry] # Call the scandir function with a path # We expect an OSError to be raised here - with pytest.raises(OSError, match="Permission denied"): + with pytest.raises(OSError, match="some permission error"): scandir("/fake/path") # Verify that the is_file method was called on the mock entry mock_entry.is_file.assert_called_once()