mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
perf(ci): sparse+blobless checkout for the test-slice generate job
The "Generate slices" job spent ~90% of its wall time in actions/checkout
(4-27s across the last 10 runs, of a 10-32s job) pulling a ~212MB working
tree. Everything in the test matrix waits on it.
It doesn't need those files. `--generate-slices` returns before
`_approximately_count_tests`, so it only ever uses test file *paths* plus
the cached durations — it never opens a test file.
The blocker was discovery: `_discover_files` rglobs the filesystem, which
finds nothing under a sparse checkout. So add `--discover-from-git`, which
lists paths via `git ls-files`. Sparse checkout only clears the worktree
(entries are marked skip-worktree), so the index still carries every path
and enumerates exactly the same set. Skip-part filtering and the
root-override rule are duplicated to match `_discover_files` semantics.
Measured on a real clone of this repo:
full depth=1 clone: 7s 212M
blobless+sparse clone: 3s 3.4M (still sees all 2510 test paths)
Both discovery paths produce byte-identical slice JSON over the full 2472
test files, so slice assignment is unchanged.
Tests assert the properties that make this safe: the two discovery paths
agree on a full checkout, the git path still works when the files are
absent from disk, and the skip-part override behaves the same either way.
This commit is contained in:
parent
f702bba63c
commit
4e3768ea36
3 changed files with 176 additions and 2 deletions
13
.github/workflows/tests.yml
vendored
13
.github/workflows/tests.yml
vendored
|
|
@ -26,6 +26,17 @@ jobs:
|
|||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
# This job only needs test file *paths* — it computes the LPT
|
||||
# distribution and never opens a test file. So skip the blobs
|
||||
# (~212MB working tree -> ~3MB) and materialize only the script
|
||||
# itself. `--discover-from-git` below lists the test paths from
|
||||
# the git index, which a sparse checkout leaves fully populated.
|
||||
# Checkout was ~90% of this job's wall time, and every test slice
|
||||
# waits on it.
|
||||
filter: blob:none
|
||||
sparse-checkout: scripts/run_tests_parallel.py
|
||||
sparse-checkout-cone-mode: false
|
||||
|
||||
- name: Restore duration cache
|
||||
uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
|
||||
|
|
@ -42,7 +53,7 @@ jobs:
|
|||
- name: Generate test slices
|
||||
id: matrix
|
||||
run: |
|
||||
MATRIX=$(python3 scripts/run_tests_parallel.py --generate-slices ${{ inputs.slice_count }})
|
||||
MATRIX=$(python3 scripts/run_tests_parallel.py --generate-slices ${{ inputs.slice_count }} --discover-from-git)
|
||||
echo "matrix=$MATRIX" >> "$GITHUB_OUTPUT"
|
||||
|
||||
test:
|
||||
|
|
|
|||
|
|
@ -170,6 +170,56 @@ def _discover_files(roots: List[Path]) -> List[Path]:
|
|||
return sorted(out)
|
||||
|
||||
|
||||
def _discover_files_from_git(roots: List[Path], repo_root: Path) -> List[Path]:
|
||||
"""Like :func:`_discover_files`, but list paths from the git index.
|
||||
|
||||
``_discover_files`` walks the filesystem, which requires a full
|
||||
working tree. The CI ``--generate-slices`` job only needs test file
|
||||
*names* (it never opens them), so it checks out sparsely — a blobless,
|
||||
sparse clone is ~3MB against ~212MB for the full tree. Under a sparse
|
||||
checkout ``rglob`` finds nothing, because the files aren't on disk.
|
||||
|
||||
The git *index* still carries every path (sparse checkout only clears
|
||||
the worktree, marking entries skip-worktree), so ``git ls-files``
|
||||
enumerates the same set the filesystem walk would have. Skip-part
|
||||
filtering and root-override semantics match ``_discover_files``
|
||||
exactly, so both discovery paths produce identical slicing input.
|
||||
"""
|
||||
seen: set[Path] = set()
|
||||
out: List[Path] = []
|
||||
for root in roots:
|
||||
rel = root.relative_to(repo_root) if root.is_absolute() else root
|
||||
# Same opt-in rule as _discover_files: naming a skipped dir as a
|
||||
# root overrides the skip for that subtree.
|
||||
root_skip_overrides = {part for part in rel.parts if part in _SKIP_PARTS}
|
||||
effective_skips = _SKIP_PARTS - root_skip_overrides
|
||||
try:
|
||||
listed = subprocess.run(
|
||||
["git", "ls-files", "-z", "--", str(rel)],
|
||||
cwd=repo_root,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
).stdout
|
||||
except (subprocess.CalledProcessError, FileNotFoundError) as e:
|
||||
print(f"error: git ls-files failed for {rel}: {e}", file=sys.stderr)
|
||||
raise SystemExit(2) from e
|
||||
for entry in listed.split("\0"):
|
||||
if not entry:
|
||||
continue
|
||||
path = Path(entry)
|
||||
if not path.name.startswith("test_") or path.suffix != ".py":
|
||||
continue
|
||||
if any(part in effective_skips for part in path.parts):
|
||||
continue
|
||||
abs_path = repo_root / path
|
||||
if abs_path in seen:
|
||||
continue
|
||||
seen.add(abs_path)
|
||||
out.append(abs_path)
|
||||
return sorted(out)
|
||||
|
||||
|
||||
def _kill_tree(proc: "subprocess.Popen", pgid: int | None = None) -> None:
|
||||
"""Kill the pytest subprocess and every descendant it spawned.
|
||||
|
||||
|
|
@ -773,6 +823,15 @@ def main() -> int:
|
|||
"so the CI generate job can feed it directly into a matrix."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--discover-from-git",
|
||||
action="store_true",
|
||||
help=(
|
||||
"List test files from the git index (git ls-files) instead of "
|
||||
"walking the filesystem. Lets --generate-slices run against a "
|
||||
"blobless sparse checkout, where the test files aren't on disk."
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
"--files",
|
||||
metavar="LIST",
|
||||
|
|
@ -811,6 +870,7 @@ def main() -> int:
|
|||
OUR_FLAGS = {
|
||||
"-j", "--jobs", "--paths", "--include-integration",
|
||||
"--file-timeout", "--file-retries", "--slice", "--generate-slices", "--files",
|
||||
"--discover-from-git",
|
||||
}
|
||||
# pytest short flags that consume the NEXT token as their value.
|
||||
PYTEST_VALUE_FLAGS = {"-k", "-m", "-p", "-o", "-c", "-r", "-W"}
|
||||
|
|
@ -929,7 +989,11 @@ def main() -> int:
|
|||
global _SKIP_PARTS # noqa: PLW0603 — config knob
|
||||
_SKIP_PARTS = set()
|
||||
|
||||
files = _discover_files(roots)
|
||||
files = (
|
||||
_discover_files_from_git(roots, repo_root)
|
||||
if args.discover_from_git
|
||||
else _discover_files(roots)
|
||||
)
|
||||
|
||||
if not files:
|
||||
print("No test files to run", file=sys.stderr)
|
||||
|
|
|
|||
|
|
@ -378,3 +378,102 @@ def test_explicit_k_wins_over_node_id_inference(tmp_path: Path) -> None:
|
|||
# -k test_beta wins: one test ran, and it wasn't filtered to nothing.
|
||||
assert proc.returncode == 0, proc.stdout
|
||||
assert "1 tests passed" in proc.stdout
|
||||
|
||||
|
||||
# ── Git-index discovery (--discover-from-git) ────────────────────────────────
|
||||
#
|
||||
# The CI "Generate slices" job only needs test file *paths* — it computes the
|
||||
# LPT distribution and never opens a test file. So it checks out blobless +
|
||||
# sparse (~212MB working tree -> ~3MB), which makes the filesystem rglob in
|
||||
# ``_discover_files`` find nothing. ``--discover-from-git`` lists paths from
|
||||
# the git index instead, which a sparse checkout leaves fully populated.
|
||||
#
|
||||
# These are behavior contracts, not snapshots: the two discovery paths must
|
||||
# agree on a full checkout (otherwise the flag silently changes which tests
|
||||
# run in which slice), and the git path must still work when the files are
|
||||
# absent from disk (otherwise it doesn't solve the problem it exists for).
|
||||
|
||||
|
||||
def _git_repo_with_tests(tmp_path: Path) -> Path:
|
||||
"""A real git repo containing test files, some under skipped dirs."""
|
||||
repo = tmp_path / "repo"
|
||||
(repo / "tests" / "sub").mkdir(parents=True)
|
||||
(repo / "tests" / "docker").mkdir()
|
||||
(repo / "tests" / "test_a.py").write_text("def test_a():\n assert True\n")
|
||||
(repo / "tests" / "sub" / "test_b.py").write_text("def test_b():\n assert True\n")
|
||||
# Not a test_*.py file — must never be discovered.
|
||||
(repo / "tests" / "helper.py").write_text("X = 1\n")
|
||||
# Under a _SKIP_PARTS dir — must be filtered out, same as the fs walk.
|
||||
(repo / "tests" / "docker" / "test_c.py").write_text("def test_c():\n assert True\n")
|
||||
subprocess.run(["git", "init", "-q"], cwd=repo, check=True)
|
||||
subprocess.run(["git", "add", "-A"], cwd=repo, check=True)
|
||||
subprocess.run(
|
||||
["git", "-c", "user.email=t@t", "-c", "user.name=t",
|
||||
"commit", "-qm", "init"],
|
||||
cwd=repo, check=True,
|
||||
)
|
||||
return repo
|
||||
|
||||
|
||||
def _discovery_fns():
|
||||
"""Import both discovery functions straight from the runner script."""
|
||||
import importlib.util
|
||||
|
||||
repo_root = Path(__file__).resolve().parent.parent
|
||||
spec = importlib.util.spec_from_file_location(
|
||||
"_rtp_under_test", repo_root / "scripts" / "run_tests_parallel.py"
|
||||
)
|
||||
assert spec is not None and spec.loader is not None
|
||||
mod = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(mod)
|
||||
return mod
|
||||
|
||||
|
||||
def test_git_discovery_matches_filesystem_discovery(tmp_path: Path) -> None:
|
||||
"""On a full checkout both discovery paths return the same file set.
|
||||
|
||||
This is the safety property for the CI flag: flipping discovery from
|
||||
rglob to ``git ls-files`` must not change which files get sliced, or
|
||||
slice assignment silently shifts under us.
|
||||
"""
|
||||
repo = _git_repo_with_tests(tmp_path)
|
||||
mod = _discovery_fns()
|
||||
roots = [repo / "tests"]
|
||||
from_fs = mod._discover_files(roots)
|
||||
from_git = mod._discover_files_from_git(roots, repo)
|
||||
assert from_git == from_fs
|
||||
# Sanity: the fixture actually produced something to compare. Sorted by
|
||||
# full path, so tests/sub/test_b.py precedes tests/test_a.py.
|
||||
assert [p.relative_to(repo).as_posix() for p in from_git] == [
|
||||
"tests/sub/test_b.py",
|
||||
"tests/test_a.py",
|
||||
]
|
||||
|
||||
|
||||
def test_git_discovery_finds_files_absent_from_disk(tmp_path: Path) -> None:
|
||||
"""The git path works when the worktree is sparse (files not checked out).
|
||||
|
||||
Simulates the CI sparse checkout by deleting the test files while
|
||||
leaving the index intact. rglob finds nothing; git ls-files still
|
||||
enumerates every path.
|
||||
"""
|
||||
repo = _git_repo_with_tests(tmp_path)
|
||||
mod = _discovery_fns()
|
||||
roots = [repo / "tests"]
|
||||
expected = mod._discover_files_from_git(roots, repo)
|
||||
|
||||
for path in repo.rglob("test_*.py"):
|
||||
path.unlink()
|
||||
|
||||
assert mod._discover_files(roots) == []
|
||||
assert mod._discover_files_from_git(roots, repo) == expected
|
||||
|
||||
|
||||
def test_git_discovery_honors_skip_part_override(tmp_path: Path) -> None:
|
||||
"""Naming a skipped dir as a root opts into it, same as the fs walk."""
|
||||
repo = _git_repo_with_tests(tmp_path)
|
||||
mod = _discovery_fns()
|
||||
roots = [repo / "tests" / "docker"]
|
||||
from_git = mod._discover_files_from_git(roots, repo)
|
||||
assert [p.name for p in from_git] == ["test_c.py"]
|
||||
assert from_git == mod._discover_files(roots)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue