From dadec59f41e6953972b47c75824e05cb79407fd9 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:00:31 -0700 Subject: [PATCH] fix(circleci): let the PR path filter read history after the shallow clone and test the clone step itself --- .circleci/scripts/path_filter.sh | 4 +- tests/unit/test_circleci_checkout.py | 113 +++++++++++++++++++++++- tests/unit/test_circleci_path_filter.py | 11 +++ 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/.circleci/scripts/path_filter.sh b/.circleci/scripts/path_filter.sh index c3cf6e566ff..73171dce75c 100755 --- a/.circleci/scripts/path_filter.sh +++ b/.circleci/scripts/path_filter.sh @@ -14,7 +14,9 @@ run_full() { candidate_bases="${PATH_FILTER_BASE_BRANCH:-main}" merge_base="" for base in $candidate_bases; do - git fetch --quiet origin "$base" 2>/dev/null || continue + unshallow="" + [ "$(git rev-parse --is-shallow-repository)" = true ] && unshallow="--unshallow" + git fetch --quiet $unshallow origin "$base" 2>/dev/null || continue candidate="$(git merge-base HEAD FETCH_HEAD 2>/dev/null)" || continue [ -n "$candidate" ] || continue if [ -z "$merge_base" ] || git merge-base --is-ancestor "$merge_base" "$candidate" 2>/dev/null; then diff --git a/tests/unit/test_circleci_checkout.py b/tests/unit/test_circleci_checkout.py index 056f2aea56a..8d45b0ba9b8 100644 --- a/tests/unit/test_circleci_checkout.py +++ b/tests/unit/test_circleci_checkout.py @@ -1,10 +1,48 @@ -from collections.abc import Iterator +import os +import subprocess +from collections.abc import Iterator, Mapping +from dataclasses import dataclass from pathlib import Path from typing import Final +import pytest import yaml +from pydantic import BaseModel, ConfigDict CONFIG: Final = Path(__file__).resolve().parents[2] / ".circleci" / "config.yml" +PUBLIC_REPO: Final = "https://github.com/BerriAI/litellm.git" + + +class _Run(BaseModel): + model_config = ConfigDict(frozen=True) + command: str + + +class _Step(BaseModel): + model_config = ConfigDict(frozen=True) + run: _Run + + +class _Command(BaseModel): + model_config = ConfigDict(frozen=True) + steps: tuple[_Step] + + +class _Commands(BaseModel): + model_config = ConfigDict(frozen=True) + checkout_over_https: _Command + + +class _Config(BaseModel): + model_config = ConfigDict(frozen=True) + commands: _Commands + + +@dataclass(frozen=True, slots=True) +class _Upstream: + url: str + older: str + newer: str def _step_name(step: object) -> str: @@ -18,7 +56,7 @@ def _step_name(step: object) -> str: def _step_names(node: object) -> Iterator[str]: if isinstance(node, dict): for key, value in node.items(): - if key == "steps" and isinstance(value, list): + if key in ("steps", "pre-steps", "post-steps") and isinstance(value, list): yield from map(_step_name, value) yield from _step_names(value) if isinstance(node, list): @@ -26,7 +64,78 @@ def _step_names(node: object) -> Iterator[str]: yield from _step_names(item) +def _git(cwd: Path, *args: str) -> str: + return subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True).stdout.strip() + + +def _commit(work: Path, message: str) -> str: + _git(work, "commit", "-q", "--allow-empty", "-m", message) + return _git(work, "rev-parse", "HEAD") + + +def _publish_two_commits(tmp_path: Path) -> _Upstream: + remote: Final = tmp_path / "remote.git" + work: Final = tmp_path / "work" + work.mkdir() + _git(tmp_path, "init", "-q", "--bare", str(remote)) + _git(work, "init", "-q", "-b", "main") + _git(work, "config", "user.email", "t@t") + _git(work, "config", "user.name", "t") + older: Final = _commit(work, "pipeline commit") + newer: Final = _commit(work, "main moved on") + _git(work, "push", "-q", str(remote), "main") + return _Upstream(url=remote.as_uri(), older=older, newer=newer) + + +def _run_checkout(tmp_path: Path, upstream: _Upstream, revisions: Mapping[str, str]) -> subprocess.CompletedProcess[str]: + job: Final = tmp_path / "job" + job.mkdir() + command: Final = _Config.model_validate(yaml.safe_load(CONFIG.read_text())).commands.checkout_over_https + return subprocess.run( + ["bash", "-eo", "pipefail", "-c", command.steps[0].run.command], + cwd=job, + env={ + "PATH": os.environ["PATH"], + "HOME": str(tmp_path), + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_TERMINAL_PROMPT": "0", + "GIT_SSH_COMMAND": "false", + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": f"url.{upstream.url}.insteadOf", + "GIT_CONFIG_VALUE_0": PUBLIC_REPO, + **revisions, + }, + capture_output=True, + text=True, + ) + + def test_every_clone_goes_over_https_because_the_project_has_no_ssh_checkout_key() -> None: names: Final = tuple(_step_names(yaml.safe_load(CONFIG.read_text()))) assert "checkout_over_https" in names assert "checkout" not in names + + +@pytest.mark.parametrize( + "revisions", + ( + {"CIRCLE_SHA1": "older"}, + {"CIRCLE_SHA1": "older", "REQUESTED_REVISION": ""}, + {"CIRCLE_SHA1": "newer", "REQUESTED_REVISION": "older"}, + ), +) +def test_a_job_checks_out_its_own_commit_from_the_public_https_url_after_main_moves( + tmp_path: Path, revisions: Mapping[str, str] +) -> None: + upstream: Final = _publish_two_commits(tmp_path) + shas: Final = {"older": upstream.older, "newer": upstream.newer, "": ""} + result: Final = _run_checkout(tmp_path, upstream, {name: shas[value] for name, value in revisions.items()}) + assert result.returncode == 0, result.stderr + assert _git(tmp_path / "job", "rev-parse", "HEAD") == upstream.older + + +def test_a_revision_that_is_not_a_full_sha_is_refused_before_git_runs(tmp_path: Path) -> None: + upstream: Final = _publish_two_commits(tmp_path) + result: Final = _run_checkout(tmp_path, upstream, {"CIRCLE_SHA1": upstream.newer, "REQUESTED_REVISION": "main"}) + assert result.returncode != 0 + assert not (tmp_path / "job" / ".git").exists() diff --git a/tests/unit/test_circleci_path_filter.py b/tests/unit/test_circleci_path_filter.py index 9f695960517..1e1e641ab24 100644 --- a/tests/unit/test_circleci_path_filter.py +++ b/tests/unit/test_circleci_path_filter.py @@ -236,6 +236,17 @@ def test_path_filter_runs_backend_pr(tmp_path: Path) -> None: assert "halt" not in result.stdout +def test_path_filter_halts_docs_only_pr_from_a_depth_one_checkout(tmp_path: Path) -> None: + work = _pr_repo(tmp_path, {"README.md": "# docs\n"}) + _git(work, "push", "-q", "origin", "litellm_feature") + shallow = tmp_path / "shallow" + remote = (tmp_path / "remote.git").as_uri() + subprocess.run(["git", "clone", "-q", "--depth", "1", "-b", "litellm_feature", remote, str(shallow)], check=True) + result = _run_path_filter(shallow, tmp_path, "backend", SCRIPTS_DIR) + assert result.returncode == 0 + assert "circleci-agent step halt" in result.stdout + + def test_path_filter_fails_open_when_not_a_pr(tmp_path: Path) -> None: work = _pr_repo(tmp_path, {"README.md": "# docs\n"}) result = _run_path_filter(work, tmp_path, "backend", SCRIPTS_DIR, is_pr=False)