mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
test(tool-output): stop expressing "unwritable path" as a magic absolute path (#4722)
* test(tool-output): stop expressing "unwritable path" as a magic absolute path `test_returns_none_on_invalid_path` and `test_fallback_when_disk_write_fails` both need an `outputs_path` that `os.makedirs` refuses to create, so they can reach `_externalize`'s `except OSError: return None` branch. They spell that as the literal path `/dev/null/cannot-mkdir-here`, which only works where `/dev/null` is a character device. On Windows it is an ordinary relative path, so `os.makedirs` succeeds, both tests fail, and the suite writes real files to `C:\dev\null\cannot-mkdir-here\ .tool-results\` -- outside any temporary directory, at the drive root. Running the backend suite a few times leaves dozens of stray files behind. The comment above the first test records that this is the second time the same assumption has broken: `/nonexistent/...` was silently created by `mkdir -p` when CI ran as root in a container, and `/dev/null/...` was the fix. Both encode a guess about the environment rather than the condition under test. Use a regular file as the parent component instead. Creating a directory below a file fails with an `OSError` subclass on every platform -- `NotADirectoryError` (errno 20) on POSIX, `FileNotFoundError` (errno 2) on Windows -- so the branch is reached deterministically, and the path lives inside the test's own `TemporaryDirectory`, so nothing is written outside it. Verified both spellings on Linux (WSL Ubuntu, non-root) and Windows; only the file-as-parent form fails on both. The two tests still have teeth: dropping `_externalize`'s `except OSError` guard makes both fail rather than pass. Tests only -- no production code or documented behaviour changes. * test(tool-output): touch the blocker file instead of writing content Only its existence as a regular file matters for os.makedirs to fail below it, so touch() states that directly.
This commit is contained in:
parent
38ff44778a
commit
1fe71110af
@ -8,8 +8,10 @@ sync/async code paths.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import tempfile
|
||||
from types import SimpleNamespace
|
||||
|
||||
@ -66,6 +68,27 @@ def _make_request(tool_name: str = "remote_executor", tool_call_id: str = "tc-1"
|
||||
)
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _unwritable_outputs_path():
|
||||
"""Yield an ``outputs_path`` that ``os.makedirs`` cannot create, on any platform.
|
||||
|
||||
The parent component is a regular file, so creating a directory below it
|
||||
fails with an ``OSError`` subclass everywhere (``NotADirectoryError`` on
|
||||
POSIX, ``FileNotFoundError`` on Windows) and nothing is written outside the
|
||||
temporary directory.
|
||||
|
||||
This deliberately avoids expressing "unwritable" as a magic absolute path.
|
||||
``/nonexistent/...`` was creatable by root in the CI container, and its
|
||||
replacement ``/dev/null/...`` relies on ``/dev/null`` being a character
|
||||
device, which is only true on POSIX -- on Windows it is an ordinary
|
||||
relative path that ``os.makedirs`` happily creates at the drive root.
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
blocker = pathlib.Path(tmpdir) / "not-a-directory"
|
||||
blocker.touch()
|
||||
yield os.path.join(blocker, "outputs")
|
||||
|
||||
|
||||
def _tm(content: str = "ok", name: str = "tool", tool_call_id: str = "tc-1") -> ToolMessage:
|
||||
return ToolMessage(content=content, name=name, tool_call_id=tool_call_id)
|
||||
|
||||
@ -160,20 +183,15 @@ class TestExternalize:
|
||||
assert f.read() == "full content here"
|
||||
|
||||
def test_returns_none_on_invalid_path(self):
|
||||
# ``/dev/null`` is a character device on both Linux and macOS, so
|
||||
# ``os.makedirs`` cannot create any subdirectory under it for any
|
||||
# user (including root). The previously-used ``/nonexistent/...``
|
||||
# path was silently created by ``mkdir -p`` when the test process
|
||||
# ran as root inside the CI container, which made this test fail
|
||||
# in CI independently of the externalization logic under test.
|
||||
path = _externalize(
|
||||
"data",
|
||||
tool_name="test",
|
||||
tool_call_id="tc-1",
|
||||
outputs_path="/dev/null/cannot-mkdir-here",
|
||||
storage_subdir=".tool-results",
|
||||
)
|
||||
assert path is None
|
||||
with _unwritable_outputs_path() as outputs_path:
|
||||
path = _externalize(
|
||||
"data",
|
||||
tool_name="test",
|
||||
tool_call_id="tc-1",
|
||||
outputs_path=outputs_path,
|
||||
storage_subdir=".tool-results",
|
||||
)
|
||||
assert path is None
|
||||
|
||||
def test_txt_extension_for_unknown_tool(self):
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
@ -710,9 +728,10 @@ class TestWrapToolCallFallback:
|
||||
mw = ToolOutputBudgetMiddleware(config=config)
|
||||
content = "x" * 500
|
||||
msg = _tm(content, name="tool")
|
||||
req = _make_request(outputs_path="/dev/null/cannot-mkdir-here")
|
||||
|
||||
result = mw.wrap_tool_call(req, lambda _: msg)
|
||||
with _unwritable_outputs_path() as outputs_path:
|
||||
req = _make_request(outputs_path=outputs_path)
|
||||
result = mw.wrap_tool_call(req, lambda _: msg)
|
||||
|
||||
assert isinstance(result, ToolMessage)
|
||||
assert "omitted from tool output" in result.content
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user