From 2c58caae78c8c5981c5e9f6be26537e95e309f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Mon, 4 Aug 2025 14:19:38 -0400 Subject: [PATCH 1/8] fix: Improve file limit test to catch SystemExit or RuntimeError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Enhance the file limit test to accommodate different exception types. A RedHat user reported a RuntimeError in #2935 * The test now catches `SystemExit` or `RuntimeError` which can be raised by the CLI. Fixes #2935 Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- docs/changelog/2935.bugfix.rst | 1 + tests/unit/test_file_limit.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 docs/changelog/2935.bugfix.rst diff --git a/docs/changelog/2935.bugfix.rst b/docs/changelog/2935.bugfix.rst new file mode 100644 index 000000000..758553f8a --- /dev/null +++ b/docs/changelog/2935.bugfix.rst @@ -0,0 +1 @@ +Accept RuntimeError in `test_too_many_open_files`, by :user:`esafak` diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index df2a281e8..3e11add6b 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -2,6 +2,7 @@ import os import sys +import typing import pytest @@ -30,10 +31,15 @@ def test_too_many_open_files(tmp_path): try: fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) - with pytest.raises(SystemExit) as excinfo: + # Typically SystemExit, but RuntimeError was observed in RedHat + expected_exception = typing.Union[SystemExit, RuntimeError] + with pytest.raises(expected_exception) as too_many_open_files_exc: cli_run([str(tmp_path / "venv")]) - assert excinfo.value.code != 0 + if too_many_open_files_exc is SystemExit: + assert too_many_open_files_exc.value.code != 0 + else: + assert "Too many open files" in str(too_many_open_files_exc.value) finally: for fd in fds: From a231b18e5bfcb49203143415036ec13d2d4681f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Tue, 5 Aug 2025 08:54:12 -0400 Subject: [PATCH 2/8] Remove soft limit guard and add RuntimeError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need to skip if the soft limit is high; we're going to reduce it anyway. Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- tests/unit/test_file_limit.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index 3e11add6b..e889ae7bb 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -9,7 +9,6 @@ from virtualenv.run import cli_run -@pytest.mark.skipif(sys.platform == "win32", reason="resource module not available on Windows") def test_too_many_open_files(tmp_path): """ Test that we get a specific error message when we have too many open files. @@ -17,8 +16,6 @@ def test_too_many_open_files(tmp_path): import resource # noqa: PLC0415 soft_limit, hard_limit = resource.getrlimit(resource.RLIMIT_NOFILE) - if soft_limit > 1024: - pytest.skip("soft limit for open files is too high to reliably trigger the error") # Lower the soft limit to a small number to trigger the error try: @@ -31,13 +28,12 @@ def test_too_many_open_files(tmp_path): try: fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) - # Typically SystemExit, but RuntimeError was observed in RedHat - expected_exception = typing.Union[SystemExit, RuntimeError] - with pytest.raises(expected_exception) as too_many_open_files_exc: + expected_exceptions = SystemExit, OSError, RuntimeError + with pytest.raises(expected_exceptions) as too_many_open_files_exc: cli_run([str(tmp_path / "venv")]) - if too_many_open_files_exc is SystemExit: - assert too_many_open_files_exc.value.code != 0 + if isinstance(too_many_open_files_exc, SystemExit): + assert too_many_open_files_exc.code != 0 else: assert "Too many open files" in str(too_many_open_files_exc.value) From 10e7ba9083b2f58ddec3744d348ac75e80e50f77 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 5 Aug 2025 12:54:33 +0000 Subject: [PATCH 3/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/test_file_limit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index e889ae7bb..140065bef 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -1,8 +1,6 @@ from __future__ import annotations import os -import sys -import typing import pytest From feabc2dae9bdf4912cb24ceb2086120d5bea72bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Tue, 5 Aug 2025 08:57:11 -0400 Subject: [PATCH 4/8] Restore win32 skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- tests/unit/test_file_limit.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index 140065bef..f49030091 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -1,12 +1,14 @@ from __future__ import annotations import os +import sys import pytest from virtualenv.run import cli_run +@pytest.mark.skipif(sys.platform == "win32", reason="resource module not available on Windows") def test_too_many_open_files(tmp_path): """ Test that we get a specific error message when we have too many open files. From b9f29017677337ace6a39e4bdc87078df5ffb5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Tue, 5 Aug 2025 10:57:31 -0400 Subject: [PATCH 5/8] Enhance file limit test for JIT implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add exception handling for `resource.setrlimit` to skip tests on implementations that don't support it (e.g., PyPy, GraalPy). * Adjust the number of opened file descriptors to account for JIT implementations which may consume more initially. * Add specific checks for `OSError` with `errno.EMFILE` and the "Too many open files" message for JIT environments. Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- tests/unit/test_file_limit.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index f49030091..13241755f 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -1,9 +1,11 @@ from __future__ import annotations +import errno import os import sys import pytest +from virtualenv.info import IMPLEMENTATION from virtualenv.run import cli_run @@ -22,11 +24,19 @@ def test_too_many_open_files(tmp_path): resource.setrlimit(resource.RLIMIT_NOFILE, (32, hard_limit)) except ValueError: pytest.skip("could not lower the soft limit for open files") + except AttributeError as exc: # pypy, graalpy + if "module 'resource' has no attribute 'setrlimit'" in str(exc): + pytest.skip(f"{IMPLEMENTATION} does not support resource.setrlimit") # Keep some file descriptors open to make it easier to trigger the error fds = [] try: - fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) + # JIT implementations use more file descriptors up front so we can run out early + try: + fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) + except OSError as jit_exceptions: # pypy, graalpy + assert jit_exceptions.errno == errno.EMFILE + assert "Too many open files" in str(jit_exceptions) expected_exceptions = SystemExit, OSError, RuntimeError with pytest.raises(expected_exceptions) as too_many_open_files_exc: From e799e5bf00be0bcf4ea007794ab91c328d3bd2d4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 5 Aug 2025 14:58:29 +0000 Subject: [PATCH 6/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/test_file_limit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index 13241755f..da71cfc10 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -5,8 +5,8 @@ import sys import pytest -from virtualenv.info import IMPLEMENTATION +from virtualenv.info import IMPLEMENTATION from virtualenv.run import cli_run @@ -24,7 +24,7 @@ def test_too_many_open_files(tmp_path): resource.setrlimit(resource.RLIMIT_NOFILE, (32, hard_limit)) except ValueError: pytest.skip("could not lower the soft limit for open files") - except AttributeError as exc: # pypy, graalpy + except AttributeError as exc: # pypy, graalpy if "module 'resource' has no attribute 'setrlimit'" in str(exc): pytest.skip(f"{IMPLEMENTATION} does not support resource.setrlimit") From 905b15ef6f5f3ceedc951dc5014e35e0bb0061a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Tue, 5 Aug 2025 12:32:15 -0400 Subject: [PATCH 7/8] Improve handling of file limit tests for JIT implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add checks for `IS_GRAALPY` and `IS_PYPY` to conditionally adjust file descriptor limits. * Refactor the test to use `pytest.raises` for more robust assertion of `OSError` during file descriptor exhaustion. * Ensure that the `EMFILE` error code and specific error message are checked for JIT environments. Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- tests/unit/test_file_limit.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index da71cfc10..f54b1a974 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -6,6 +6,7 @@ import pytest +from src.virtualenv.info import IS_GRAALPY, IS_PYPY from virtualenv.info import IMPLEMENTATION from virtualenv.run import cli_run @@ -32,11 +33,10 @@ def test_too_many_open_files(tmp_path): fds = [] try: # JIT implementations use more file descriptors up front so we can run out early - try: - fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) - except OSError as jit_exceptions: # pypy, graalpy - assert jit_exceptions.errno == errno.EMFILE - assert "Too many open files" in str(jit_exceptions) + if IS_GRAALPY or IS_PYPY: + with pytest.raises(OSError, match="Too many open files") as jit_exception: + fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) + assert jit_exception.value.errno == errno.EMFILE expected_exceptions = SystemExit, OSError, RuntimeError with pytest.raises(expected_exceptions) as too_many_open_files_exc: From 3103176d5d8a714272bf4a44119d1d96777e9813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emre=20=C5=9Eafak?= <3928300+esafak@users.noreply.github.com> Date: Tue, 5 Aug 2025 12:44:01 -0400 Subject: [PATCH 8/8] Revert in order to populate the file descriptors and trigger the error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise we'll have to repeat the logic since the error does not happen outside of JIT. Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com> --- tests/unit/test_file_limit.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_file_limit.py b/tests/unit/test_file_limit.py index f54b1a974..0c66561f8 100644 --- a/tests/unit/test_file_limit.py +++ b/tests/unit/test_file_limit.py @@ -6,7 +6,6 @@ import pytest -from src.virtualenv.info import IS_GRAALPY, IS_PYPY from virtualenv.info import IMPLEMENTATION from virtualenv.run import cli_run @@ -33,10 +32,11 @@ def test_too_many_open_files(tmp_path): fds = [] try: # JIT implementations use more file descriptors up front so we can run out early - if IS_GRAALPY or IS_PYPY: - with pytest.raises(OSError, match="Too many open files") as jit_exception: - fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) - assert jit_exception.value.errno == errno.EMFILE + try: + fds.extend(os.open(os.devnull, os.O_RDONLY) for _ in range(20)) + except OSError as jit_exceptions: # pypy, graalpy + assert jit_exceptions.errno == errno.EMFILE # noqa: PT017 + assert "Too many open files" in str(jit_exceptions) # noqa: PT017 expected_exceptions = SystemExit, OSError, RuntimeError with pytest.raises(expected_exceptions) as too_many_open_files_exc: