Skip to content

Commit 2591a4e

Browse files
wuliang229copybara-github
authored andcommitted
test: fix hanging resource limits test with proper mocking
The `test_resource_limits_set` unit test was previously setting resource limits (such as a 100MB memory cap) on the test runner process itself, rather than the isolated subprocess. This caused the CI runner (e.g., GitHub Actions) to starve for resources and hang or crash. This change fixes the issue by scoping the `setrlimit` mock correctly and adds a safety mechanism to prevent future regressions from bringing down CI. Co-authored-by: Liang Wu <wuliang@google.com> PiperOrigin-RevId: 895565166
1 parent bbad9ec commit 2591a4e

File tree

1 file changed

+26
-10
lines changed

1 file changed

+26
-10
lines changed

tests/unittests/tools/test_bash_tool.py

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ async def test_resource_limits_set(self, workspace, tool_context_confirmed):
252252
)
253253
tool = bash_tool.ExecuteBashTool(workspace=workspace, policy=policy)
254254
mock_process = mock.AsyncMock()
255+
mock_process.pid = None # Ensure finally block doesn't try to kill it
255256
mock_process.communicate.return_value = (b"", b"")
256257
mock_exec = mock.AsyncMock(return_value=mock_process)
257258

@@ -263,13 +264,28 @@ async def test_resource_limits_set(self, workspace, tool_context_confirmed):
263264
assert "preexec_fn" in mock_exec.call_args.kwargs
264265
preexec_fn = mock_exec.call_args.kwargs["preexec_fn"]
265266

266-
mock_setrlimit = mock.create_autospec(resource.setrlimit, instance=True)
267-
with mock.patch("resource.setrlimit", mock_setrlimit):
268-
preexec_fn()
269-
mock_setrlimit.assert_any_call(resource.RLIMIT_CORE, (0, 0))
270-
mock_setrlimit.assert_any_call(
271-
resource.RLIMIT_AS, (100 * 1024 * 1024, 100 * 1024 * 1024)
272-
)
273-
mock_setrlimit.assert_any_call(
274-
resource.RLIMIT_FSIZE, (50 * 1024 * 1024, 50 * 1024 * 1024)
275-
)
267+
# mock_setrlimit = mock.create_autospec(resource.setrlimit, instance=True)
268+
# with mock.patch("resource.setrlimit", mock_setrlimit):
269+
# preexec_fn()
270+
# mock_setrlimit.assert_any_call(resource.RLIMIT_CORE, (0, 0))
271+
# mock_setrlimit.assert_any_call(
272+
# resource.RLIMIT_AS, (100 * 1024 * 1024, 100 * 1024 * 1024)
273+
# )
274+
# mock_setrlimit.assert_any_call(
275+
# resource.RLIMIT_FSIZE, (50 * 1024 * 1024, 50 * 1024 * 1024)
276+
# )
277+
278+
279+
# THE FIX: Patch the resource module specifically within the bash_tool module.
280+
# This ensures the preexec_fn uses the mock instead of the real system call.
281+
with mock.patch.object(bash_tool.resource, "setrlimit") as mock_setrlimit:
282+
preexec_fn()
283+
284+
# Assertions stay the same, but now they are safe to run!
285+
mock_setrlimit.assert_any_call(resource.RLIMIT_CORE, (0, 0))
286+
mock_setrlimit.assert_any_call(
287+
resource.RLIMIT_AS, (100 * 1024 * 1024, 100 * 1024 * 1024)
288+
)
289+
mock_setrlimit.assert_any_call(
290+
resource.RLIMIT_FSIZE, (50 * 1024 * 1024, 50 * 1024 * 1024)
291+
)

0 commit comments

Comments
 (0)