[ci]: collect Modal checkout tests in unit lane

fastvideo/tests/modal/pr_test.py

@@ -1,5 +1,9 @@
 import os
+import re
+import shutil
+import subprocess
 import sys
+import time
 
 import modal
 
@@ -96,6 +100,95 @@ wandb_secret = modal.Secret.from_dict(
     {"WANDB_API_KEY": os.environ.get("WANDB_API_KEY", "")})
 
 
+def _run_git_with_retries(command: list[str],
+                          *,
+                          cwd: str,
+                          cleanup_path: str | None = None) -> None:
+    last_returncode = 1
+    for attempt in range(1, 4):
+        if cleanup_path is not None:
+            shutil.rmtree(cleanup_path, ignore_errors=True)
+
+        result = subprocess.run(command, cwd=cwd, check=False)
+        if result.returncode == 0:
+            return
+
+        last_returncode = result.returncode
+        if attempt < 3:
+            sleep_seconds = 5 * attempt
+            print(
+                f"Git command failed (attempt {attempt}/3, exit {last_returncode}); "
+                f"retrying in {sleep_seconds}s",
+                flush=True)
+            time.sleep(sleep_seconds)
+
+    raise RuntimeError(
+        f"Git command failed after 3 attempts with exit code {last_returncode}: "
+        + " ".join(command))
+
+
+def _checkout_repository(git_repo: str,
+                         git_commit: str,
+                         pr_number: str | None,
+                         repo_root: str = "/FastVideo") -> None:
+    if not git_repo or git_repo.startswith("-"):
+        raise RuntimeError("BUILDKITE_REPO must be a non-empty repository URL.")
+
+    if pr_number and pr_number != "false":
+        try:
+            pr_id = int(pr_number)
+        except ValueError as error:
+            raise RuntimeError(
+                f"Invalid BUILDKITE_PULL_REQUEST value: {pr_number}") from error
+        if pr_id <= 0:
+            raise RuntimeError(
+                f"Invalid BUILDKITE_PULL_REQUEST value: {pr_number}")
+        target = f"refs/pull/{pr_id}/head"
+        print(f"Using PR ref for checkout: {target}")
+    else:
+        if not git_commit or re.fullmatch(r"[0-9a-fA-F]{7,64}",
+                                          git_commit) is None:
+            raise RuntimeError(
+                f"Invalid BUILDKITE_COMMIT value: {git_commit}")
+        target = git_commit
+        print(f"Using direct commit checkout: {target}")
+
+    clone_command = [
+        "git",
+        "-c",
+        "http.version=HTTP/1.1",
+        "clone",
+        "--config",
+        "http.version=HTTP/1.1",
+        "--depth=1",
+        "--filter=blob:none",
+        "--no-checkout",
+        git_repo,
+        repo_root,
+    ]
+    _run_git_with_retries(clone_command,
+                          cwd="/",
+                          cleanup_path=repo_root)
+
+    git_prefix = ["git", "-c", "http.version=HTTP/1.1"]
+    _run_git_with_retries(
+        git_prefix + [
+            "fetch",
+            "--prune",
+            "--no-tags",
+            "--depth=1",
+            "--filter=blob:none",
+            "origin",
+            target,
+        ],
+        cwd=repo_root)
+    _run_git_with_retries(
+        git_prefix + ["checkout", "--detach", "FETCH_HEAD"], cwd=repo_root)
+    _run_git_with_retries(
+        git_prefix + ["submodule", "update", "--init", "--recursive"],
+        cwd=repo_root)
+
+
 def run_test(pytest_command: str):
     """Helper function to run a test suite with custom pytest command"""
     run_test_command(pytest_command, build_kernel=True)
@@ -116,26 +209,15 @@ def run_test_command(test_command: str,
     lane would then test stale kernels. Pass install_command="" for commands
     that manage their own installs.
     """
-    import subprocess
-    import sys
-    import os
-
-    git_repo = os.environ.get("BUILDKITE_REPO")
-    git_commit = os.environ.get("BUILDKITE_COMMIT")
+    git_repo = os.environ.get("BUILDKITE_REPO", "")
+    git_commit = os.environ.get("BUILDKITE_COMMIT", "")
     pr_number = os.environ.get("BUILDKITE_PULL_REQUEST")
 
     print(f"Cloning repository: {git_repo}")
     print(f"Target commit: {git_commit}")
     if pr_number:
         print(f"PR number: {pr_number}")
-
-    # For PRs (including forks), use GitHub's PR refs to get the correct commit
-    if pr_number and pr_number != "false":
-        checkout_command = f"git fetch --prune origin refs/pull/{pr_number}/head && git checkout FETCH_HEAD"
-        print(f"Using PR ref for checkout: {checkout_command}")
-    else:
-        checkout_command = f"git checkout {git_commit}"
-        print(f"Using direct commit checkout: {checkout_command}")
+    _checkout_repository(git_repo, git_commit, pr_number)
 
     build_kernel_command = """
     cd fastvideo-kernel &&
@@ -148,10 +230,7 @@ def run_test_command(test_command: str,
     command = f"""
     source $HOME/.local/bin/env &&
     source /opt/venv/bin/activate &&
-    git clone {git_repo} /FastVideo &&
     cd /FastVideo &&
-    {checkout_command} &&
-    git submodule update --init --recursive &&
     {install_clause}
     {build_kernel_command}
     {test_command}
@@ -286,7 +365,13 @@ def run_self_forcing_tests():
 @app.function(gpu="L40S:1", image=image, timeout=900, secrets=[ci_env_secret])
 def run_unit_test():
     run_test(
-        "pytest ./fastvideo/tests/api/ ./fastvideo/tests/contract/ ./fastvideo/tests/dataset/ ./fastvideo/tests/workflow/ ./fastvideo/tests/entrypoints/ ./fastvideo/tests/train/ ./fastvideo/tests/stages/ ./fastvideo/tests/ops/ ./fastvideo/tests/worker/ ./fastvideo/tests/training/test_trackers.py ./fastvideo/tests/attention/test_sdpa_metadata_mask_contract.py --ignore=./fastvideo/tests/entrypoints/test_openai_api_integration.py --ignore=./fastvideo/tests/train/models --ignore=./fastvideo/tests/train/methods -vs"
+        "pytest ./fastvideo/tests/api/ ./fastvideo/tests/contract/ ./fastvideo/tests/dataset/ "
+        "./fastvideo/tests/workflow/ ./fastvideo/tests/entrypoints/ ./fastvideo/tests/train/ "
+        "./fastvideo/tests/stages/ ./fastvideo/tests/ops/ ./fastvideo/tests/worker/ "
+        "./fastvideo/tests/training/test_trackers.py "
+        "./fastvideo/tests/attention/test_sdpa_metadata_mask_contract.py ./fastvideo/tests/modal/test_pr_test.py "
+        "--ignore=./fastvideo/tests/entrypoints/test_openai_api_integration.py "
+        "--ignore=./fastvideo/tests/train/models --ignore=./fastvideo/tests/train/methods -vs"
     )
 
 

fastvideo/tests/modal/test_pr_test.py

@@ -0,0 +1,277 @@
+# SPDX-License-Identifier: Apache-2.0
+
+from __future__ import annotations
+
+import importlib.util
+import subprocess
+import sys
+import types
+from pathlib import Path
+
+import pytest
+
+
+class _FakeImage:
+
+    @classmethod
+    def from_registry(cls, *_args, **_kwargs):
+        return cls()
+
+    def apt_install(self, *_args, **_kwargs):
+        return self
+
+    def run_commands(self, *_args, **_kwargs):
+        return self
+
+    def env(self, *_args, **_kwargs):
+        return self
+
+
+class _FakeVolume:
+
+    @classmethod
+    def from_name(cls, *_args, **_kwargs):
+        return cls()
+
+
+class _FakeSecret:
+
+    @classmethod
+    def from_dict(cls, *_args, **_kwargs):
+        return cls()
+
+
+class _FakeApp:
+
+    def function(self, *_args, **_kwargs):
+        def decorator(func):
+            return func
+
+        return decorator
+
+
+def _load_pr_test_module(monkeypatch):
+    fake_modal = types.SimpleNamespace(
+        App=lambda: _FakeApp(),
+        Image=_FakeImage,
+        Secret=_FakeSecret,
+        Volume=_FakeVolume,
+    )
+    fake_image_utils = types.SimpleNamespace(
+        resolve_image_ref=lambda image_ref: image_ref,
+        resolve_uv_torch_backend=lambda _image_tag: None,
+    )
+    monkeypatch.setitem(sys.modules, "modal", fake_modal)
+    monkeypatch.setitem(sys.modules, "modal_image_utils", fake_image_utils)
+    module_path = Path(__file__).with_name("pr_test.py")
+    spec = importlib.util.spec_from_file_location(
+        "modal_pr_test_under_test", module_path)
+    module = importlib.util.module_from_spec(spec)
+    assert spec.loader is not None
+    spec.loader.exec_module(module)
+    return module
+
+
+def test_checkout_repository_retries_clone_and_fetches_pr_ref(monkeypatch):
+    module = _load_pr_test_module(monkeypatch)
+    commands = []
+    cleanup_paths = []
+    sleep_seconds = []
+    returncodes = iter([128, 0, 0, 0, 0])
+
+    def fake_run(command, **kwargs):
+        commands.append((command, kwargs))
+        return types.SimpleNamespace(returncode=next(returncodes))
+
+    monkeypatch.setattr(module.subprocess, "run", fake_run)
+    monkeypatch.setattr(
+        module.shutil,
+        "rmtree",
+        lambda path, ignore_errors: cleanup_paths.append(
+            (path, ignore_errors)),
+    )
+    monkeypatch.setattr(module.time, "sleep", sleep_seconds.append)
+
+    module._checkout_repository(
+        "https://github.com/hao-ai-lab/FastVideo.git",
+        "0123456789abcdef0123456789abcdef01234567",
+        "1584",
+        repo_root="/tmp/FastVideo",
+    )
+
+    assert len(commands) == 5
+    clone_command = commands[0][0]
+    assert commands[1][0] == clone_command
+    assert clone_command == [
+        "git",
+        "-c",
+        "http.version=HTTP/1.1",
+        "clone",
+        "--config",
+        "http.version=HTTP/1.1",
+        "--depth=1",
+        "--filter=blob:none",
+        "--no-checkout",
+        "https://github.com/hao-ai-lab/FastVideo.git",
+        "/tmp/FastVideo",
+    ]
+    assert cleanup_paths == [
+        ("/tmp/FastVideo", True),
+        ("/tmp/FastVideo", True),
+    ]
+    assert sleep_seconds == [5]
+    assert [kwargs for _, kwargs in commands] == [
+        {"cwd": "/", "check": False},
+        {"cwd": "/", "check": False},
+        {"cwd": "/tmp/FastVideo", "check": False},
+        {"cwd": "/tmp/FastVideo", "check": False},
+        {"cwd": "/tmp/FastVideo", "check": False},
+    ]
+
+    fetch_command = commands[2][0]
+    assert fetch_command[-2:] == [
+        "origin",
+        "refs/pull/1584/head",
+    ]
+    assert "--depth=1" in fetch_command
+    assert "--filter=blob:none" in fetch_command
+    assert commands[3][0][-3:] == ["checkout", "--detach", "FETCH_HEAD"]
+    assert commands[4][0][-4:] == [
+        "submodule",
+        "update",
+        "--init",
+        "--recursive",
+    ]
+    assert all(command[0:3] == [
+        "git",
+        "-c",
+        "http.version=HTTP/1.1",
+    ] for command, _ in commands)
+
+
+def test_checkout_repository_fetches_direct_commit(monkeypatch):
+    module = _load_pr_test_module(monkeypatch)
+    commands = []
+    commit = "0123456789abcdef0123456789abcdef01234567"
+
+    def fake_run(command, **kwargs):
+        commands.append((command, kwargs))
+        return types.SimpleNamespace(returncode=0)
+
+    monkeypatch.setattr(module.subprocess, "run", fake_run)
+    monkeypatch.setattr(module.shutil, "rmtree", lambda *_args, **_kwargs: None)
+
+    module._checkout_repository(
+        "git@github.com:macthecadillac/FastVideo.git",
+        commit,
+        "false",
+        repo_root="/tmp/FastVideo",
+    )
+
+    assert len(commands) == 4
+    assert commands[1][0][-2:] == ["origin", commit]
+    assert commands[2][0][-3:] == ["checkout", "--detach", "FETCH_HEAD"]
+
+
+def test_git_retry_exhaustion_is_bounded_and_cleans_each_attempt(monkeypatch):
+    module = _load_pr_test_module(monkeypatch)
+    calls = []
+    cleanup_paths = []
+    sleep_seconds = []
+
+    def fake_run(command, **kwargs):
+        calls.append((command, kwargs))
+        return types.SimpleNamespace(returncode=128)
+
+    monkeypatch.setattr(module.subprocess, "run", fake_run)
+    monkeypatch.setattr(
+        module.shutil,
+        "rmtree",
+        lambda path, ignore_errors: cleanup_paths.append(
+            (path, ignore_errors)),
+    )
+    monkeypatch.setattr(module.time, "sleep", sleep_seconds.append)
+
+    with pytest.raises(RuntimeError, match="failed after 3 attempts"):
+        module._run_git_with_retries(
+            ["git", "clone", "repo", "/tmp/FastVideo"],
+            cwd="/",
+            cleanup_path="/tmp/FastVideo",
+        )
+
+    assert len(calls) == 3
+    assert cleanup_paths == [("/tmp/FastVideo", True)] * 3
+    assert sleep_seconds == [5, 10]
+
+
+@pytest.mark.parametrize(
+    ("git_repo", "git_commit", "pr_number"),
+    [
+        ("", "0123456789abcdef", "false"),
+        ("-bad-option", "0123456789abcdef", "false"),
+        ("https://example.com/repo.git", "", "false"),
+        ("https://example.com/repo.git", "not-a-commit", "false"),
+        ("https://example.com/repo.git", "0123456789abcdef", "not-a-pr"),
+        ("https://example.com/repo.git", "0123456789abcdef", "0"),
+    ],
+)
+def test_checkout_repository_rejects_invalid_buildkite_values(
+        monkeypatch, git_repo, git_commit, pr_number):
+    module = _load_pr_test_module(monkeypatch)
+    monkeypatch.setattr(
+        module.subprocess,
+        "run",
+        lambda *_args, **_kwargs: pytest.fail(
+            "git must not run for invalid input"),
+    )
+
+    with pytest.raises(RuntimeError):
+        module._checkout_repository(git_repo, git_commit, pr_number)
+
+
+@pytest.mark.parametrize(
+    ("build_kernel", "install_command"),
+    [
+        (True, 'uv pip install -e ".[test]"'),
+        (False, ""),
+    ],
+)
+def test_run_test_command_composes_valid_post_checkout_shell(
+        monkeypatch, build_kernel, install_command):
+    module = _load_pr_test_module(monkeypatch)
+    real_run = subprocess.run
+    events = []
+
+    monkeypatch.setenv(
+        "BUILDKITE_REPO", "https://github.com/hao-ai-lab/FastVideo.git")
+    monkeypatch.setenv(
+        "BUILDKITE_COMMIT", "0123456789abcdef0123456789abcdef01234567")
+    monkeypatch.setenv("BUILDKITE_PULL_REQUEST", "false")
+    monkeypatch.setattr(
+        module,
+        "_checkout_repository",
+        lambda *args: events.append(("checkout", args)),
+    )
+
+    def fake_run(args, **_kwargs):
+        events.append(("run", args))
+        return types.SimpleNamespace(returncode=0)
+
+    monkeypatch.setattr(module.subprocess, "run", fake_run)
+
+    module.run_test_command(
+        "pytest fastvideo/tests/api -q",
+        build_kernel=build_kernel,
+        install_command=install_command,
+    )
+
+    assert [event for event, _ in events] == ["checkout", "run"]
+    shell_command = events[1][1][-1]
+    assert "cd /FastVideo" in shell_command
+    assert "git clone" not in shell_command
+    assert ("./build.sh" in shell_command) is build_kernel
+    if install_command:
+        assert install_command in shell_command
+    else:
+        assert "uv pip install -e" not in shell_command
+    real_run(["/bin/bash", "-n"], input=shell_command, text=True, check=True)