General-purpose coding policy for Baruch's AI agents
74
93%
Does it follow best practices?
Run evals on this skill
Adds up to 20 points to the overall score
Medium
Suggest reviewing before use
"""Real Git fixtures for review packages and their brief-composition gate."""
import json
import os
import shutil
import subprocess
import tempfile
import unittest
from pathlib import Path
SKILL = Path(__file__).resolve().parents[1]
class ReviewPackageTests(unittest.TestCase):
def setUp(self):
self.temp = tempfile.TemporaryDirectory()
self.addCleanup(self.temp.cleanup)
self.root = Path(self.temp.name).resolve()
self.repo = self.root / "repo"
self.repo.mkdir()
self.env = {key: value for key, value in os.environ.items()
if not key.startswith("GIT_") and key != "FOREMAN_REPORTS_DIR"}
self.env.update({
"GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull,
"GIT_AUTHOR_NAME": "Fixture", "GIT_AUTHOR_EMAIL": "fixture@example.test",
"GIT_COMMITTER_NAME": "Fixture", "GIT_COMMITTER_EMAIL": "fixture@example.test",
"GIT_AUTHOR_DATE": "2020-01-02T03:04:05+00:00",
"GIT_COMMITTER_DATE": "2020-01-02T03:04:05+00:00",
"LC_ALL": "C",
})
self.git("init", "-q")
self.lines = [f"line {number}" for number in range(1, 51)]
self.source = self.repo / "source.txt"
self.base = self.commit("initial")
self.lines[14] = "first change"
self.first = self.commit("first implementation")
self.lines[34] = "second change"
self.head = self.commit("second implementation")
self.output = self.root / "round reports" / "package.diff"
def git(self, *args):
return subprocess.run(["git", *args], cwd=self.repo, env=self.env,
text=True, capture_output=True, check=True).stdout.strip()
def commit(self, message):
self.source.write_text("\n".join(self.lines) + "\n", encoding="utf-8")
self.git("add", ".")
self.git("commit", "-qm", message)
return self.git("rev-parse", "HEAD")
def package(self, *args, env=None, cwd=None):
return subprocess.run(["bash", str(SKILL / "review-package.sh"), *map(str, args)],
cwd=cwd or self.repo, env=env or self.env,
text=True, capture_output=True, check=False)
def test_multicommit_range_contains_both_commits_stat_and_ten_context_lines(self):
result = self.package(self.base, self.head, self.output)
self.assertEqual(result.returncode, 0, result.stderr)
self.assertEqual(result.stdout, f"{self.output}\n")
text = self.output.read_text(encoding="utf-8")
self.assertIn(f"BASE: {self.base}\nHEAD: {self.head}", text)
self.assertIn(f"{self.first} first implementation", text)
self.assertIn(f"{self.head} second implementation", text)
self.assertIn("1 file changed, 2 insertions(+), 2 deletions(-)", text)
self.assertIn("+first change", text)
self.assertIn("+second change", text)
self.assertIn(" line 5\n", text)
self.assertNotIn(" line 4", text.splitlines())
self.assertIn(" line 45\n", text)
self.assertNotIn(" line 46", text.splitlines())
self.assertEqual(self.git("status", "--porcelain"), "")
def test_default_name_changes_after_fix_and_old_package_stays_unchanged(self):
env = dict(self.env, FOREMAN_REPORTS_DIR=str(self.output.parent))
before = self.package(self.base, self.first, env=env)
after = self.package(self.base, self.head, env=env)
self.assertEqual(before.returncode, 0, before.stderr)
self.assertEqual(after.returncode, 0, after.stderr)
old_path = self.output.parent / f"review-{self.base[:7]}..{self.first[:7]}.diff"
new_path = self.output.parent / f"review-{self.base[:7]}..{self.head[:7]}.diff"
self.assertEqual(before.stdout, f"{old_path}\n")
self.assertEqual(after.stdout, f"{new_path}\n")
self.assertNotIn("second change", old_path.read_text(encoding="utf-8"))
self.assertIn("second change", new_path.read_text(encoding="utf-8"))
def test_identical_run_reuses_artifact_even_with_display_config_overrides(self):
self.assertEqual(self.package(self.base, self.head, self.output).returncode, 0)
before = self.output.read_bytes()
for key, value in (
("color.ui", "always"), ("log.decorate", "full"),
("log.showSignature", "true"), ("diff.context", "1"),
("diff.noprefix", "true"), ("diff.relative", "true"),
("diff.algorithm", "histogram"), ("diff.indentHeuristic", "true"),
):
self.git("config", key, value)
result = self.package(self.base, self.head, self.output)
self.assertEqual(result.returncode, 0, result.stderr)
self.assertEqual(self.output.read_bytes(), before)
def test_bad_refs_exit_two_without_creating_output(self):
tree = self.git("rev-parse", "HEAD^{tree}")
for base, head in (("missing-ref", self.head), (self.base, "missing-ref"),
("--help", self.head), (tree, self.head)):
with self.subTest(base=base, head=head):
result = self.package(base, head, self.output)
self.assertEqual(result.returncode, 2, result.stderr)
self.assertEqual(result.stdout, "")
self.assertIn("fetch", result.stderr)
self.assertFalse(self.output.parent.exists())
def test_usage_and_missing_default_output_directory_are_actionable(self):
for args in ((), (self.base,), (self.base, self.head),
(self.base, self.head, self.output, "extra")):
with self.subTest(args=args):
result = self.package(*args)
self.assertEqual(result.returncode, 2)
self.assertEqual(result.stdout, "")
self.assertTrue(result.stderr)
def test_relative_output_and_empty_predevelopment_range(self):
result = self.package(self.base, self.base, "round/package.diff")
self.assertEqual(result.returncode, 0, result.stderr)
path = self.repo / "round/package.diff"
self.assertEqual(result.stdout, f"{path}\n")
self.assertIn(f"BASE: {self.base}\nHEAD: {self.base}", path.read_text())
self.assertNotIn("diff --git", path.read_text())
def test_different_existing_file_is_preserved(self):
self.output.parent.mkdir()
self.output.write_text("unrelated report\n", encoding="utf-8")
result = self.package(self.base, self.head, self.output)
self.assertEqual(result.returncode, 1, result.stderr)
self.assertEqual(result.stdout, "")
self.assertEqual(self.output.read_text(), "unrelated report\n")
self.assertEqual(list(self.output.parent.iterdir()), [self.output])
def test_symlink_and_multiline_output_are_refused(self):
self.output.parent.mkdir()
self.output.symlink_to(self.source)
before = self.source.read_bytes()
for path in (self.output, str(self.output) + "\nsecond", self.output.parent):
with self.subTest(path=path):
result = self.package(self.base, self.head, path)
self.assertEqual(result.returncode, 2, result.stderr)
self.assertEqual(result.stdout, "")
self.assertEqual(self.source.read_bytes(), before)
def test_comparison_failure_is_not_reported_as_different_content(self):
self.output.parent.mkdir()
self.output.write_text("existing artifact\n", encoding="utf-8")
fakebin = self.root / "bin"
fakebin.mkdir()
fakecmp = fakebin / "cmp"
fakecmp.write_text('#!/usr/bin/env bash\nset -euo pipefail\nexit 2\n', encoding="utf-8")
fakecmp.chmod(0o755)
env = dict(self.env, PATH=f"{fakebin}:{self.env['PATH']}")
result = self.package(self.base, self.head, self.output, env=env)
self.assertEqual(result.returncode, 1)
self.assertEqual(result.stdout, "")
self.assertIn("cannot compare", result.stderr)
self.assertIn("cmp exit 2", result.stderr)
self.assertNotIn("different content", result.stderr)
self.assertEqual(self.output.read_text(), "existing artifact\n")
self.assertEqual(list(self.output.parent.iterdir()), [self.output])
def test_binary_change_is_not_reduced_to_a_filename(self):
(self.repo / "binary.dat").write_bytes(bytes(range(256)))
head = self.commit("binary fixture")
result = self.package(self.base, head, self.output)
self.assertEqual(result.returncode, 0, result.stderr)
self.assertIn("GIT binary patch", self.output.read_text())
def test_git_failure_does_not_publish_partial_content(self):
executable = shutil.which("git")
self.assertIsNotNone(executable)
fakebin = self.root / "bin"
fakebin.mkdir()
fakegit = fakebin / "git"
fakegit.write_text(
'#!/usr/bin/env bash\nset -euo pipefail\n'
'if [[ "${2:-}" == diff ]]; then printf "partial diff\\n"; exit 42; fi\n'
'exec "$PACKAGE_TEST_REAL_GIT" "$@"\n', encoding="utf-8")
fakegit.chmod(0o755)
env = dict(self.env, PATH=f"{fakebin}:{self.env['PATH']}",
PACKAGE_TEST_REAL_GIT=str(executable))
result = self.package(self.base, self.head, self.output, env=env)
self.assertEqual(result.returncode, 1, result.stderr)
self.assertEqual(result.stdout, "")
self.assertIn("no artifact published", result.stderr)
self.assertEqual(list(self.output.parent.iterdir()), [])
def test_concurrent_directory_cannot_be_reported_as_a_written_package(self):
executable = shutil.which("ln")
self.assertIsNotNone(executable)
fakebin = self.root / "bin"
fakebin.mkdir()
fakeln = fakebin / "ln"
fakeln.write_text(
'#!/usr/bin/env bash\nset -euo pipefail\n'
'mkdir "$PACKAGE_TEST_OUTPUT"\n'
'exec "$PACKAGE_TEST_REAL_LN" "$@"\n', encoding="utf-8")
fakeln.chmod(0o755)
env = dict(self.env, PATH=f"{fakebin}:{self.env['PATH']}",
PACKAGE_TEST_REAL_LN=str(executable), PACKAGE_TEST_OUTPUT=str(self.output))
result = self.package(self.base, self.head, self.output, env=env)
self.assertEqual(result.returncode, 1, result.stderr)
self.assertEqual(result.stdout, "")
self.assertTrue(self.output.is_dir())
self.assertEqual(list(self.output.iterdir()), [])
self.assertEqual(list(self.output.parent.iterdir()), [self.output])
def test_sourcing_does_not_build_or_print_a_package(self):
result = subprocess.run(["bash", "-c", 'source "$1"', "test",
str(SKILL / "review-package.sh")],
cwd=self.root, env=self.env, text=True,
capture_output=True, check=False)
self.assertEqual(result.returncode, 0, result.stderr)
self.assertEqual(result.stdout, "")
def team_operation(self):
"""The team-round contract path every composition requires."""
path = self.root / "team-operation.md"
path.write_text("Team-round contract fixture\n", encoding="utf-8")
return path
def test_package_gate_leaves_entire_round_unwritten(self):
templates = self.root / "templates"
templates.mkdir()
(templates / "COMMON.md").write_text("Common instructions\nContract: {{TEAM_OPERATION}}\n", encoding="utf-8")
(templates / "brief-developer.md").write_text("Develop {{ISSUE}}\nREPORT: {{REPORT}}\n", encoding="utf-8")
for role in ("reviewer", "tester"):
(templates / f"brief-{role}.md").write_text(
"Review {{ISSUE}} with {{REVIEW_PACKAGE}} over {{REVIEW_BASE}}..{{REVIEW_HEAD}}\nREPORT: {{REPORT}}\n", encoding="utf-8")
empty = self.root / "empty.diff"
empty.touch()
missing = self.root / "missing.diff"
values = self.root / "values.json"
for role in ("reviewer", "tester"):
for package in (None, str(missing), str(empty), str(self.repo), "relative.diff"):
with self.subTest(role=role, package=package):
review_values = {"REVIEW_BASE": self.base, "REVIEW_HEAD": self.head, "REPORT": f"/r/{role}.md"}
if package is not None:
review_values["REVIEW_PACKAGE"] = package
values.write_text(json.dumps({"shared": {"ISSUE": "#323", "TEAM_OPERATION": str(self.team_operation())},
"roles": {"developer": {"REPORT": "/r/developer.md"}, role: review_values}}), encoding="utf-8")
outdir = self.root / "briefs"
result = subprocess.run(["bash", str(SKILL / "compose-briefs.sh"),
str(templates), str(values), str(outdir)],
env=self.env, text=True, capture_output=True, check=False)
self.assertEqual(result.returncode, 2, result.stderr)
self.assertEqual(result.stdout, "")
self.assertIn("REVIEW_PACKAGE", result.stderr)
self.assertFalse(outdir.exists())
# A real generated package flows through the same gate into both briefs.
result = self.package(self.base, self.head, self.output)
self.assertEqual(result.returncode, 0, result.stderr)
values.write_text(json.dumps({"shared": {"ISSUE": "#323", "TEAM_OPERATION": str(self.team_operation())}, "roles": {
role: {"REVIEW_PACKAGE": result.stdout.strip(), "REVIEW_BASE": self.base, "REVIEW_HEAD": self.head, "REPORT": f"/r/{role}.md"} for role in ("reviewer", "tester")
}}), encoding="utf-8")
outdir = self.root / "briefs"
result = subprocess.run(["bash", str(SKILL / "compose-briefs.sh"),
str(templates), str(values), str(outdir)],
env=self.env, text=True, capture_output=True, check=False)
self.assertEqual(result.returncode, 0, result.stderr)
for role in ("reviewer", "tester"):
self.assertIn(str(self.output), (outdir / f"brief-{role}.md").read_text())
def test_malformed_review_range_refuses_before_writing_any_brief(self):
templates = self.root / "range-templates"
templates.mkdir()
(templates / "COMMON.md").write_text("Common\nContract: {{TEAM_OPERATION}}\n", encoding="utf-8")
(templates / "brief-developer.md").write_text("Develop\nREPORT: {{REPORT}}\n", encoding="utf-8")
for role in ("reviewer", "tester"):
(templates / f"brief-{role}.md").write_text(
"Review {{REVIEW_BASE}}..{{REVIEW_HEAD}} in {{REVIEW_PACKAGE}}\nREPORT: {{REPORT}}\n", encoding="utf-8")
self.assertEqual(self.package(self.base, self.head, self.output).returncode, 0)
values = self.root / "invalid-range.json"
for role in ("reviewer", "tester"):
for key in ("REVIEW_BASE", "REVIEW_HEAD"):
for invalid in ("", "main", "abc123", " " + self.base, "G" * 40, 123):
with self.subTest(role=role, key=key, value=invalid):
fields = {"REVIEW_BASE": self.base, "REVIEW_HEAD": self.head, "REVIEW_PACKAGE": str(self.output), "REPORT": f"/r/{role}.md", key: invalid}
values.write_text(json.dumps({"shared": {"TEAM_OPERATION": str(self.team_operation())}, "roles": {"developer": {"REPORT": "/r/developer.md"}, role: fields}}))
outdir = self.root / "invalid-briefs"
result = subprocess.run(["bash", str(SKILL / "compose-briefs.sh"), str(templates), str(values), str(outdir)],
env=self.env, capture_output=True, text=True, check=False)
self.assertEqual(result.returncode, 2, result.stderr)
self.assertIn(key, result.stderr)
self.assertFalse(outdir.exists())
if __name__ == "__main__":
unittest.main().tessl-plugin
hooks
rules
skills
adopt-fork-pr
herdr-foreman
classify
foreman
references
specialists
templates
tests
herdr-standup
migrate-to-plugin
onboard-repo
release
references
tests