diff --git a/.github/workflows/api-tests.yml b/.github/workflows/api-tests.yml index a1f29fd6529..c99c3c3cc25 100644 --- a/.github/workflows/api-tests.yml +++ b/.github/workflows/api-tests.yml @@ -126,6 +126,7 @@ jobs: uv run --project api pytest \ -p no:benchmark \ --start-middleware \ + --middleware-stop-timeout 2 \ -n auto \ -o 'addopts=--cov=./api --cov-branch --import-mode=importlib' \ --cov-report= \ diff --git a/api/conftest.py b/api/conftest.py index e967a723d2a..a008a020771 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -8,6 +8,7 @@ tests below their directory, and this setup is shared by api/tests and api/provi from __future__ import annotations +from dataclasses import replace from pathlib import Path import pytest @@ -33,6 +34,12 @@ ensure_backend_test_environment(_REPO_ROOT) def pytest_addoption(parser: pytest.Parser) -> None: group = parser.getgroup("dify") + group.addoption( + "--middleware-stop-timeout", + type=int, + default=None, + help="Override the shutdown grace period in seconds for pytest-started middleware only.", + ) group.addoption( "--shard-index", type=int, @@ -76,6 +83,9 @@ def pytest_configure(config: pytest.Config) -> None: raise pytest.UsageError("--shard-total must be at least 1") if not 1 <= shard_index <= shard_total: raise pytest.UsageError("--shard-index must be between 1 and --shard-total") + stop_timeout = config.getoption("middleware_stop_timeout") + if stop_timeout is not None and stop_timeout < 0: + raise pytest.UsageError("--middleware-stop-timeout must be nonnegative") config.stash[_DIFY_COMPOSE_STACKS_KEY] = [] @@ -106,6 +116,10 @@ def pytest_sessionstart(session: pytest.Session) -> None: if config.getoption("start_middleware"): ensure_compose_env_files(_REPO_ROOT) stack = build_middleware_stack(_REPO_ROOT, parse_services(config.getoption("middleware_services"))) + stack = replace( + stack, + shutdown_timeout_seconds=config.getoption("middleware_stop_timeout"), + ) stack.up() stacks.append(stack) diff --git a/api/tests/pytest_dify.py b/api/tests/pytest_dify.py index 7cdbb4e9da6..4451dfdc2d3 100644 --- a/api/tests/pytest_dify.py +++ b/api/tests/pytest_dify.py @@ -85,8 +85,8 @@ class DockerComposeStack: env_file: Path services: tuple[str, ...] profiles: tuple[str, ...] = () - ready_delay_seconds: float = 0.0 warmup_urls: tuple[str, ...] = () + shutdown_timeout_seconds: int | None = None def _compose_command(self) -> list[str]: command = [ @@ -115,8 +115,6 @@ class DockerComposeStack: ] completed = subprocess.run(wait_command, cwd=self.repo_root, text=True, capture_output=True) if completed.returncode == 0: - if self.ready_delay_seconds > 0: - time.sleep(self.ready_delay_seconds) self._warm_up() return @@ -135,8 +133,11 @@ class DockerComposeStack: ) def down(self) -> None: - """Stop services started for this pytest run.""" - subprocess.run(self._compose_command() + ["down"], cwd=self.repo_root, check=True) + """Stop test services, optionally overriding Compose's shutdown grace period.""" + command = self._compose_command() + ["down"] + if self.shutdown_timeout_seconds is not None: + command.extend(("--timeout", str(self.shutdown_timeout_seconds))) + subprocess.run(command, cwd=self.repo_root, check=True) def _warm_up(self) -> None: for url in self.warmup_urls: @@ -164,10 +165,12 @@ def build_middleware_stack(repo_root: Path, services: list[str]) -> DockerCompos name="middleware", project_name="dify-pytest-middleware", repo_root=repo_root, - compose_files=(repo_root / "docker" / "docker-compose.middleware.yaml",), + compose_files=( + repo_root / "docker" / "docker-compose.middleware.yaml", + repo_root / "docker" / "docker-compose.pytest.middleware.yaml", + ), env_file=repo_root / "docker" / "middleware.env", services=tuple(services), - ready_delay_seconds=5.0, warmup_urls=("http://127.0.0.1:8194/health",), ) diff --git a/api/tests/unit_tests/test_pytest_dify.py b/api/tests/unit_tests/test_pytest_dify.py index c902df5df4d..c41229d0b5c 100644 --- a/api/tests/unit_tests/test_pytest_dify.py +++ b/api/tests/unit_tests/test_pytest_dify.py @@ -1,6 +1,10 @@ import os import subprocess +from dataclasses import replace from pathlib import Path +from unittest.mock import MagicMock + +import pytest from tests.pytest_dify import ( DEFAULT_LOG_FORMAT, @@ -106,12 +110,65 @@ def test_builders_use_expected_compose_files(tmp_path: Path): middleware = build_middleware_stack(tmp_path, ["db_postgres"]) vdb = build_vdb_stack(tmp_path, ["weaviate", "qdrant"]) - assert middleware.compose_files == (tmp_path / "docker" / "docker-compose.middleware.yaml",) + assert middleware.compose_files == ( + tmp_path / "docker" / "docker-compose.middleware.yaml", + tmp_path / "docker" / "docker-compose.pytest.middleware.yaml", + ) assert middleware.env_file == tmp_path / "docker" / "middleware.env" - assert middleware.ready_delay_seconds == 5.0 + assert middleware.warmup_urls == ("http://127.0.0.1:8194/health",) assert vdb.compose_files == ( tmp_path / "docker" / "docker-compose.yaml", tmp_path / "docker" / "docker-compose.pytest.ports.yaml", ) assert vdb.env_file == tmp_path / "docker" / ".env" assert vdb.profiles == ("weaviate", "qdrant") + + +@pytest.mark.parametrize( + ("timeout", "expected_suffix"), [(None, ["down"]), (0, ["down", "--timeout", "0"]), (2, ["down", "--timeout", "2"])] +) +def test_stack_down_preserves_default_or_uses_explicit_timeout( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, timeout: int | None, expected_suffix: list[str] +) -> None: + calls = [] + monkeypatch.setattr(subprocess, "run", lambda args, **kwargs: calls.append((args, kwargs))) + stack = replace( + build_middleware_stack(tmp_path, ["db_postgres"]), + shutdown_timeout_seconds=timeout, + ) + + stack.down() + + assert calls == [(stack._compose_command() + expected_suffix, {"cwd": tmp_path, "check": True})] + assert build_vdb_stack(tmp_path, ["weaviate"]).shutdown_timeout_seconds is None + + +def test_stack_down_propagates_failure(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + failure = subprocess.CalledProcessError(1, ["docker", "compose", "down"]) + + def fail_run(*args: object, **kwargs: object) -> None: + raise failure + + monkeypatch.setattr(subprocess, "run", fail_run) + stack = build_middleware_stack(tmp_path, ["redis"]) + + with pytest.raises(subprocess.CalledProcessError) as exc: + stack.down() + + assert exc.value is failure + + +def test_healthy_middleware_still_checks_http_readiness_without_fixed_sleep( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setattr(subprocess, "run", lambda args, **_kwargs: subprocess.CompletedProcess(args=args, returncode=0)) + sleep = MagicMock() + monkeypatch.setattr("time.sleep", sleep) + urlopen = MagicMock() + urlopen.return_value.__enter__.return_value.status = 200 + monkeypatch.setattr("urllib.request.urlopen", urlopen) + + build_middleware_stack(tmp_path, ["redis", "sandbox"]).up() + + urlopen.assert_called_once_with("http://127.0.0.1:8194/health", timeout=5) + sleep.assert_not_called() diff --git a/docker/docker-compose.pytest.middleware.yaml b/docker/docker-compose.pytest.middleware.yaml new file mode 100644 index 00000000000..baa8e22906b --- /dev/null +++ b/docker/docker-compose.pytest.middleware.yaml @@ -0,0 +1,14 @@ +# Pytest-only readiness polling. Keep the healthcheck commands from the base +# middleware configuration, but avoid waiting for the default 30-second interval. +services: + redis: + healthcheck: + interval: 1s + # Let Compose's existing 180-second --wait-timeout bound startup, rather + # than making the default three failed probes reject a slow cold start. + retries: 180 + + sandbox: + healthcheck: + interval: 1s + retries: 180