mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-30 09:42:21 +01:00
fix(tests): bind the docker-socket fixtures somewhere sun_path fits
Four more tests in the same family as the /tmp ones this change already
fixes, and they hide for the same reason: the failure depends on how
long $TMPDIR happens to be.
tests/test_shell_routes.py::TestHostDockerAccess (three) and
tests/test_cookbook_docker_access.py::test_container_opt_in_with_unix_
socket_is_allowed each bind an AF_UNIX socket at tmp_path/"docker.sock".
macOS gives sun_path 104 bytes including the terminator. pytest roots
tmp_path at $TMPDIR, which on a stock Mac is a 49-character
/var/folders/<2>/<30>/T/; add pytest-of-<user>/pytest-<n>/ and the
test's own name and the bind path is 115 bytes before the filename.
OSError: AF_UNIX path too long
Linux allows 108 and roots $TMPDIR at /tmp, so CI never sees it. Under a
shortened $TMPDIR the path lands at exactly 103 and passes — until
pytest's run counter reaches two digits and it becomes 104. That is why
the ledger's counts did not include these: they were measured somewhere
the path fit.
Adds tests/helpers/unix_sockets.bound_unix_socket, which binds under a
short directory and asserts the length before it tries, so the next
socket fixture fails with a sentence rather than an errno. Records the
trap in KNOWN_FAILURES.md along with the instruction to re-measure with
the default $TMPDIR.
This commit is contained in:
+34
-5
@@ -6,10 +6,12 @@ uninterpretable: you cannot tell "you broke something" from "you are on a Mac",
|
|||||||
so the usual result is either chasing a non-bug or ignoring a real one.
|
so the usual result is either chasing a non-bug or ignoring a real one.
|
||||||
|
|
||||||
This is that list. It is a record of observation, not a permission slip: a test
|
This is that list. It is a record of observation, not a permission slip: a test
|
||||||
here is still a test that does not pass, and three of the six below are
|
here is still a test that does not pass, and the three that remain below are
|
||||||
defects someone should fix.
|
all still worth someone's time.
|
||||||
|
|
||||||
Last measured: `lab @ c499c01b` plus the fixes in this change, macOS 15 on Apple Silicon, Python 3.11.
|
Last measured: `lab @ c499c01b` plus the fixes in this change, macOS 15 on
|
||||||
|
Apple Silicon, Python 3.11, with the **default** `$TMPDIR` — see the socket
|
||||||
|
entry below for why that qualifier is load-bearing.
|
||||||
|
|
||||||
```
|
```
|
||||||
3 failed, 10658 passed, 6 skipped
|
3 failed, 10658 passed, 6 skipped
|
||||||
@@ -38,9 +40,9 @@ including one holding real data.
|
|||||||
Miss `npm ci` and roughly 36 browser tests fail on `Cannot find package
|
Miss `npm ci` and roughly 36 browser tests fail on `Cannot find package
|
||||||
'playwright'`. That is not a regression, it is the missing install.
|
'playwright'`. That is not a regression, it is the missing install.
|
||||||
|
|
||||||
## The three
|
## The three that remain, and the seven that no longer do
|
||||||
|
|
||||||
### Test bugs: fixed
|
### Test bugs: comparing an unresolved path against a resolved one
|
||||||
|
|
||||||
Three failures compared an unresolved `/tmp` path against a resolved
|
Three failures compared an unresolved `/tmp` path against a resolved
|
||||||
`/private/tmp` one, and are fixed rather than listed:
|
`/private/tmp` one, and are fixed rather than listed:
|
||||||
@@ -57,6 +59,33 @@ Both now resolve consistently. They are recorded here because the shape recurs:
|
|||||||
on macOS, mixing a resolved and an unresolved temp path is a test bug that
|
on macOS, mixing a resolved and an unresolved temp path is a test bug that
|
||||||
looks like a platform failure.
|
looks like a platform failure.
|
||||||
|
|
||||||
|
### Test bugs: a temp path too long to bind a socket to
|
||||||
|
|
||||||
|
Four more, same family, invisible unless `$TMPDIR` is long enough:
|
||||||
|
|
||||||
|
- `tests/test_shell_routes.py::TestHostDockerAccess` (three tests)
|
||||||
|
- `tests/test_cookbook_docker_access.py::test_container_opt_in_with_unix_socket_is_allowed`
|
||||||
|
|
||||||
|
```
|
||||||
|
OSError: AF_UNIX path too long
|
||||||
|
```
|
||||||
|
|
||||||
|
Each bound an `AF_UNIX` socket at `tmp_path / "docker.sock"`. macOS gives
|
||||||
|
`sun_path` 104 bytes including the terminator, and pytest's `tmp_path` is
|
||||||
|
rooted at `$TMPDIR`, which on a stock Mac is a 49-character
|
||||||
|
`/var/folders/<2>/<30>/T/`. Add `pytest-of-<user>/pytest-<n>/` and the test's
|
||||||
|
own name and the bind path is 115 bytes before the filename.
|
||||||
|
|
||||||
|
This is why the counts above depend on where you run from: under a shortened
|
||||||
|
`$TMPDIR` the path lands at 103 and the tests pass, and it tips over the moment
|
||||||
|
pytest's run counter reaches two digits. Linux allows 108 bytes and roots
|
||||||
|
`$TMPDIR` at `/tmp`, so it never bites there and CI stays green.
|
||||||
|
|
||||||
|
They now bind through `tests/helpers/unix_sockets.bound_unix_socket`, which
|
||||||
|
puts the socket under a short directory. **Measure with the default `$TMPDIR`**
|
||||||
|
— `env -u TMPDIR` or an explicit `/var/folders/...` — or this whole file
|
||||||
|
records a run nobody else has.
|
||||||
|
|
||||||
### Optional dependency: ffmpeg without a WebP encoder
|
### Optional dependency: ffmpeg without a WebP encoder
|
||||||
|
|
||||||
- `tests/test_inspect_media_tool.py::test_inspect_media_exports_final_decodable_frame_at_exact_duration`
|
- `tests/test_inspect_media_tool.py::test_inspect_media_exports_final_decodable_frame_at_exact_duration`
|
||||||
|
|||||||
@@ -0,0 +1,46 @@
|
|||||||
|
"""Bind an AF_UNIX socket at a path the kernel will actually accept.
|
||||||
|
|
||||||
|
``sun_path`` is 104 bytes on macOS, terminator included, so a bind path longer
|
||||||
|
than 103 characters fails with ``OSError: AF_UNIX path too long``. pytest's
|
||||||
|
``tmp_path`` is rooted at ``$TMPDIR``, which on stock macOS is a 49-character
|
||||||
|
``/var/folders/<2>/<30>/T/`` path; adding ``pytest-of-<user>/pytest-<n>/`` and
|
||||||
|
the test's own (truncated) name spends the rest of the budget before the
|
||||||
|
filename is appended.
|
||||||
|
|
||||||
|
That is why this reads as flaky rather than broken. Linux allows 108 bytes and
|
||||||
|
roots ``$TMPDIR`` at ``/tmp``, so it never bites there; on macOS whether it
|
||||||
|
bites depends on the length of ``$TMPDIR``, the test's name, and how many
|
||||||
|
digits pytest's run counter is currently using. A run under a shortened
|
||||||
|
``$TMPDIR`` passes, the same checkout under the default one does not.
|
||||||
|
|
||||||
|
The path is resolved before it is handed back, for the same reason the rest of
|
||||||
|
this change resolves temp paths: on macOS ``/tmp`` is a symlink to
|
||||||
|
``/private/tmp``, and a test that binds one spelling and asserts on the other
|
||||||
|
is comparing two names for the same socket.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
import socket
|
||||||
|
import tempfile
|
||||||
|
from contextlib import contextmanager
|
||||||
|
|
||||||
|
# Short enough to leave room for the socket's own name under every platform's
|
||||||
|
# sun_path budget. A relative root would depend on the working directory.
|
||||||
|
_SHORT_ROOT = os.path.realpath(tempfile.gettempdir() if os.name == "nt" else "/tmp")
|
||||||
|
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def bound_unix_socket(name="docker.sock"):
|
||||||
|
"""Yield the path of a listening AF_UNIX socket, cleaned up on exit."""
|
||||||
|
directory = os.path.realpath(tempfile.mkdtemp(prefix="odysseus-sock-", dir=_SHORT_ROOT))
|
||||||
|
path = os.path.join(directory, name)
|
||||||
|
if len(path) > 103: # pragma: no cover - guards the guard
|
||||||
|
raise AssertionError(f"socket path is {len(path)} bytes, over the limit: {path}")
|
||||||
|
sock = socket.socket(socket.AF_UNIX)
|
||||||
|
try:
|
||||||
|
sock.bind(path)
|
||||||
|
yield path
|
||||||
|
finally:
|
||||||
|
sock.close()
|
||||||
|
shutil.rmtree(directory, ignore_errors=True)
|
||||||
@@ -1,4 +1,3 @@
|
|||||||
import socket
|
|
||||||
from unittest.mock import AsyncMock
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -9,6 +8,7 @@ from starlette.requests import Request
|
|||||||
import routes.cookbook_routes as cookbook_routes
|
import routes.cookbook_routes as cookbook_routes
|
||||||
from routes.cookbook_helpers import ServeRequest, _validate_serve_cmd
|
from routes.cookbook_helpers import ServeRequest, _validate_serve_cmd
|
||||||
from src.host_docker_access import HOST_DOCKER_ACCESS_HINT
|
from src.host_docker_access import HOST_DOCKER_ACCESS_HINT
|
||||||
|
from tests.helpers.unix_sockets import bound_unix_socket
|
||||||
|
|
||||||
|
|
||||||
def _model_serve_endpoint():
|
def _model_serve_endpoint():
|
||||||
@@ -57,19 +57,18 @@ async def test_container_cli_only_is_rejected(monkeypatch, tmp_path):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_container_opt_in_with_unix_socket_is_allowed(monkeypatch, tmp_path):
|
async def test_container_opt_in_with_unix_socket_is_allowed(monkeypatch):
|
||||||
monkeypatch.setattr(cookbook_routes.shutil, "which", lambda binary: "/usr/bin/docker")
|
monkeypatch.setattr(cookbook_routes.shutil, "which", lambda binary: "/usr/bin/docker")
|
||||||
socket_path = tmp_path / "docker.sock"
|
|
||||||
|
|
||||||
with socket.socket(socket.AF_UNIX) as unix_socket:
|
# Not tmp_path: binding under $TMPDIR overruns sun_path on macOS.
|
||||||
unix_socket.bind(str(socket_path))
|
with bound_unix_socket() as socket_path:
|
||||||
available = await cookbook_routes._binary_available(
|
available = await cookbook_routes._binary_available(
|
||||||
"docker",
|
"docker",
|
||||||
None,
|
None,
|
||||||
None,
|
None,
|
||||||
in_container=True,
|
in_container=True,
|
||||||
environ={"ODYSSEUS_ENABLE_HOST_DOCKER": "true"},
|
environ={"ODYSSEUS_ENABLE_HOST_DOCKER": "true"},
|
||||||
socket_path=str(socket_path),
|
socket_path=socket_path,
|
||||||
)
|
)
|
||||||
|
|
||||||
assert available is True
|
assert available is True
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ import importlib
|
|||||||
import importlib.util
|
import importlib.util
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import socket
|
|
||||||
import sys
|
import sys
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
@@ -28,6 +27,7 @@ from routes.shell_routes import (
|
|||||||
_venv_activate_prefix,
|
_venv_activate_prefix,
|
||||||
DOCKER_IN_CONTAINER_HINT,
|
DOCKER_IN_CONTAINER_HINT,
|
||||||
)
|
)
|
||||||
|
from tests.helpers.unix_sockets import bound_unix_socket
|
||||||
|
|
||||||
|
|
||||||
def test_shell_routes_import_without_posix_pty_modules(monkeypatch):
|
def test_shell_routes_import_without_posix_pty_modules(monkeypatch):
|
||||||
@@ -294,30 +294,25 @@ class TestHostDockerAccess:
|
|||||||
def test_socket_without_explicit_opt_in_is_disabled(
|
def test_socket_without_explicit_opt_in_is_disabled(
|
||||||
self,
|
self,
|
||||||
monkeypatch,
|
monkeypatch,
|
||||||
tmp_path,
|
|
||||||
flag,
|
flag,
|
||||||
):
|
):
|
||||||
socket_path = tmp_path / "docker.sock"
|
# Not tmp_path: binding under $TMPDIR overruns sun_path on macOS.
|
||||||
with socket.socket(socket.AF_UNIX) as unix_socket:
|
with bound_unix_socket() as socket_path:
|
||||||
unix_socket.bind(str(socket_path))
|
|
||||||
if flag is None:
|
if flag is None:
|
||||||
monkeypatch.delenv("ODYSSEUS_ENABLE_HOST_DOCKER", raising=False)
|
monkeypatch.delenv("ODYSSEUS_ENABLE_HOST_DOCKER", raising=False)
|
||||||
else:
|
else:
|
||||||
monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", flag)
|
monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", flag)
|
||||||
|
|
||||||
assert _host_docker_access_enabled(str(socket_path)) is False
|
assert _host_docker_access_enabled(socket_path) is False
|
||||||
|
|
||||||
def test_explicit_opt_in_with_unix_socket_is_enabled(
|
def test_explicit_opt_in_with_unix_socket_is_enabled(
|
||||||
self,
|
self,
|
||||||
monkeypatch,
|
monkeypatch,
|
||||||
tmp_path,
|
|
||||||
):
|
):
|
||||||
socket_path = tmp_path / "docker.sock"
|
with bound_unix_socket() as socket_path:
|
||||||
with socket.socket(socket.AF_UNIX) as unix_socket:
|
|
||||||
unix_socket.bind(str(socket_path))
|
|
||||||
monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", "true")
|
monkeypatch.setenv("ODYSSEUS_ENABLE_HOST_DOCKER", "true")
|
||||||
|
|
||||||
assert _host_docker_access_enabled(str(socket_path)) is True
|
assert _host_docker_access_enabled(socket_path) is True
|
||||||
|
|
||||||
|
|
||||||
class TestPackageProbeStatus:
|
class TestPackageProbeStatus:
|
||||||
|
|||||||
Reference in New Issue
Block a user