This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 319a369d4a5 Simplify Python prechecks and policy summary reporting
(#40228)
319a369d4a5 is described below
commit 319a369d4a5488e30ba74bc04fccaceea4aedf12
Author: Liang Zhang <[email protected]>
AuthorDate: Sun Oct 18 23:38:36 2026 +0800
Simplify Python prechecks and policy summary reporting (#40228)
---
.codex/harness/agents/run.py | 69 +++++------
.codex/harness/agents/test_run.py | 73 ++++++++++++
.codex/skills/gen-ut/scripts/scan_quality_rules.py | 57 +++++----
.../skills/gen-ut/tests/test_scan_quality_rules.py | 131 +++++++++++++++++++++
.../review-pr/tests/test_build_review_inventory.py | 20 ----
.../skills/review-pr/tests/test_review_common.py | 21 ++++
.../skills/review-pr/tests/test_review_ledger.py | 4 -
7 files changed, 289 insertions(+), 86 deletions(-)
diff --git a/.codex/harness/agents/run.py b/.codex/harness/agents/run.py
index 27cb0fe99f7..c55bc25fcef 100644
--- a/.codex/harness/agents/run.py
+++ b/.codex/harness/agents/run.py
@@ -2515,31 +2515,39 @@ def main() -> int:
trace_results = []
if args.mode in {"trace", "all"}:
trace_results = run_local_read_traces(bundle, selected_profiles,
args.trace_cwd or ["root", "nested"])
+ policy_summary = {
+ "policy_manifest": bundle["manifest_file"],
+ "policy_manifest_sha256": bundle["manifest_sha256"],
+ "policy_bundle_sha256": bundle["bundle_sha256"],
+ "policy_inventory_sha256": bundle["inventory_sha256"],
+ "policy_sources": bundle["inventory"],
+ "policy_source_changes": inventory_changes,
+ "authorized_policy_source_changes":
sorted(args.authorized_policy_source_change),
+ "policy_source_baseline_available": baseline is not None and
"policy_sources" in baseline,
+ "policy_binding_sha256": digest_json(policy_bindings),
+ "policy_bindings": policy_bindings,
+ "policy_binding_changes": binding_changes,
+ "authorized_policy_binding_changes":
sorted(args.authorized_policy_binding_change),
+ "policy_binding_baseline_available": baseline is not None and
"policy_bindings" in baseline,
+ "profiles": (
+ {each: bundle["profiles"][each] for each in selected_profiles}
+ if args.mode in {"validate", "trace"} else bundle["profiles"]
+ ),
+ "profile_case_counts": profile_case_counts,
+ "root_max_bytes": bundle["root_max_bytes"],
+ "default_project_doc_max_bytes": DEFAULT_PROJECT_DOC_MAX_BYTES,
+ "semantic_evaluation": semantic_evaluation_metadata(
+ semantic_policy_assertions, executed=args.mode not in {"validate",
"trace"}
+ ),
+ "routing_trace": routing_trace_metadata(trace_results),
+ "impact": impact,
+ }
if args.mode in {"validate", "trace"}:
deterministic_regressions = sorted(set(contract_regressions +
inventory_regressions + binding_regressions))
summary = {
"label": args.label,
"mode": args.mode,
- "policy_manifest": bundle["manifest_file"],
- "policy_manifest_sha256": bundle["manifest_sha256"],
- "policy_bundle_sha256": bundle["bundle_sha256"],
- "policy_inventory_sha256": bundle["inventory_sha256"],
- "policy_sources": bundle["inventory"],
- "policy_source_changes": inventory_changes,
- "authorized_policy_source_changes":
sorted(args.authorized_policy_source_change),
- "policy_source_baseline_available": baseline is not None and
"policy_sources" in baseline,
- "policy_binding_sha256": digest_json(policy_bindings),
- "policy_bindings": policy_bindings,
- "policy_binding_changes": binding_changes,
- "authorized_policy_binding_changes":
sorted(args.authorized_policy_binding_change),
- "policy_binding_baseline_available": baseline is not None and
"policy_bindings" in baseline,
- "profiles": {each: bundle["profiles"][each] for each in
selected_profiles},
- "profile_case_counts": profile_case_counts,
- "root_max_bytes": bundle["root_max_bytes"],
- "default_project_doc_max_bytes": DEFAULT_PROJECT_DOC_MAX_BYTES,
- "semantic_evaluation":
semantic_evaluation_metadata(semantic_policy_assertions, executed=False),
- "routing_trace": routing_trace_metadata(trace_results),
- "impact": impact,
+ **policy_summary,
"trace_results": trace_results,
"critical_regressions": deterministic_regressions,
"output_dir": str(output_dir),
@@ -2644,26 +2652,7 @@ def main() -> int:
"mode": args.mode,
"policy_file": "AGENTS.md",
"policy_sha256": policy_sha256,
- "policy_manifest": bundle["manifest_file"],
- "policy_manifest_sha256": bundle["manifest_sha256"],
- "policy_bundle_sha256": bundle["bundle_sha256"],
- "policy_inventory_sha256": bundle["inventory_sha256"],
- "policy_sources": bundle["inventory"],
- "policy_source_changes": inventory_changes,
- "authorized_policy_source_changes":
sorted(args.authorized_policy_source_change),
- "policy_source_baseline_available": baseline is not None and
"policy_sources" in baseline,
- "policy_binding_sha256": digest_json(policy_bindings),
- "policy_bindings": policy_bindings,
- "policy_binding_changes": binding_changes,
- "authorized_policy_binding_changes":
sorted(args.authorized_policy_binding_change),
- "policy_binding_baseline_available": baseline is not None and
"policy_bindings" in baseline,
- "profiles": bundle["profiles"],
- "profile_case_counts": profile_case_counts,
- "root_max_bytes": bundle["root_max_bytes"],
- "default_project_doc_max_bytes": DEFAULT_PROJECT_DOC_MAX_BYTES,
- "semantic_evaluation":
semantic_evaluation_metadata(semantic_policy_assertions),
- "routing_trace": routing_trace_metadata(trace_results),
- "impact": impact,
+ **policy_summary,
"evaluator": {"model": EVALUATOR_MODEL, "reasoning_effort":
EVALUATOR_REASONING_EFFORT},
"runner_sha256":
hashlib.sha256(Path(__file__).read_bytes()).hexdigest(),
"cases_sha256":
hashlib.sha256(Path(__file__).with_name("cases.toml").read_bytes()).hexdigest(),
diff --git a/.codex/harness/agents/test_run.py
b/.codex/harness/agents/test_run.py
index 3a4325d5ca1..92910c2f4e6 100644
--- a/.codex/harness/agents/test_run.py
+++ b/.codex/harness/agents/test_run.py
@@ -64,6 +64,79 @@ class RunTest(unittest.TestCase):
self.assertEqual(0, run.main())
run_cases.assert_not_called()
+ def test_main_summary_contract_for_each_mode(self) -> None:
+ case = run.load_cases(["read_only_review"])[0]
+ bundle =
run.load_policy_manifest(Path(run.__file__).resolve().parents[3])
+ policy_keys = [
+ "policy_manifest", "policy_manifest_sha256",
"policy_bundle_sha256", "policy_inventory_sha256",
+ "policy_sources", "policy_source_changes",
"authorized_policy_source_changes",
+ "policy_source_baseline_available", "policy_binding_sha256",
"policy_bindings", "policy_binding_changes",
+ "authorized_policy_binding_changes",
"policy_binding_baseline_available", "profiles", "profile_case_counts",
+ "root_max_bytes", "default_project_doc_max_bytes",
"semantic_evaluation", "routing_trace", "impact",
+ ]
+ actual = {"results": [self._create_case_result(case, "Review
FooService without editing it.")]}
+ evaluations = [self._create_evaluation([case["id"]], [])]
+ trace_results = [{"passed": True}]
+ for mode in ("validate", "trace", "semantic", "all"):
+ with self.subTest(mode=mode), tempfile.TemporaryDirectory() as
directory:
+ output_dir = Path(directory) / "output"
+ with patch("sys.argv", [
+ "run.py", "--mode", mode, "--case", case["id"],
"--profile", "root",
+ "--output-dir", str(output_dir)
+ ]), patch.object(run, "resolve_codex_home",
return_value=Path(directory)), patch.object(
+ run, "run_cases", return_value=(0, 1.0, actual,
evaluations)
+ ) as run_cases, patch.object(
+ run, "semantic_surface_digest",
return_value="semantic-surface"
+ ), patch.object(
+ run, "run_local_read_traces",
return_value=trace_results
+ ), patch.object(
+ run, "compare_policy_inventory",
return_value=(["inventory-regression"], [])
+ ), patch.object(
+ run, "compare_with_baseline",
return_value=(["semantic-regression"], [])
+ ), patch.object(
+ run, "read_usage", return_value={
+ "input_tokens": 1, "cached_input_tokens": 0,
"uncached_input_tokens": 1, "output_tokens": 1,
+ }
+ ), patch("builtins.print") as print_summary:
+ self.assertEqual(1, run.main())
+ summary = json.loads((output_dir /
"summary.json").read_text(encoding="utf-8"))
+ self.assertEqual(summary,
json.loads(print_summary.call_args.args[0]))
+ deterministic = mode in {"validate", "trace"}
+ policy_start = 2 if deterministic else 4
+ self.assertEqual(policy_keys,
list(summary)[policy_start:policy_start + len(policy_keys)])
+ self.assertEqual(["label", "mode"], list(summary)[:2])
+ self.assertEqual(mode, summary["mode"])
+ self.assertEqual(bundle["manifest_file"],
summary["policy_manifest"])
+ self.assertEqual(bundle["manifest_sha256"],
summary["policy_manifest_sha256"])
+ self.assertEqual(bundle["bundle_sha256"],
summary["policy_bundle_sha256"])
+ self.assertEqual(bundle["inventory_sha256"],
summary["policy_inventory_sha256"])
+ self.assertEqual(bundle["inventory"],
summary["policy_sources"])
+ self.assertEqual([], summary["policy_source_changes"])
+ self.assertEqual([],
summary["authorized_policy_source_changes"])
+ self.assertFalse(summary["policy_source_baseline_available"])
+ self.assertFalse(summary["policy_binding_baseline_available"])
+ self.assertEqual(bundle["root_max_bytes"],
summary["root_max_bytes"])
+ self.assertEqual(run.DEFAULT_PROJECT_DOC_MAX_BYTES,
summary["default_project_doc_max_bytes"])
+ self.assertIsNone(summary["impact"])
+ expected_profiles = {"root": bundle["profiles"]["root"]} if
deterministic else bundle["profiles"]
+ self.assertEqual(expected_profiles, summary["profiles"])
+ self.assertEqual(not deterministic,
summary["semantic_evaluation"]["executed"])
+ expected_traces = trace_results if mode in {"trace", "all"}
else []
+ self.assertEqual(expected_traces, summary["trace_results"])
+ self.assertEqual(bool(expected_traces),
summary["routing_trace"]["executed"])
+ expected_regressions = ["inventory-regression"] if
deterministic else ["inventory-regression", "semantic-regression"]
+ self.assertEqual(expected_regressions,
summary["critical_regressions"])
+ if deterministic:
+ run_cases.assert_not_called()
+ self.assertEqual([
+ "trace_results", "critical_regressions", "output_dir",
"policy_snapshot_dir"
+ ], list(summary)[policy_start + len(policy_keys):])
+ else:
+ run_cases.assert_called_once()
+ self.assertEqual(["policy_file", "policy_sha256"],
list(summary)[2:4])
+ self.assertEqual("AGENTS.md", summary["policy_file"])
+
self.assertEqual(hashlib.sha256(Path(run.__file__).read_bytes()).hexdigest(),
summary["runner_sha256"])
+
@staticmethod
def _create_case_result(case: dict[str, Any], summary: str) -> dict[str,
Any]:
return {
diff --git a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
index fa348550ba4..6778929658b 100644
--- a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
+++ b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
@@ -784,11 +784,11 @@ def checkstyle_preview_final_parameters(path: Path,
source: str) -> list[str]:
return violations
-def collect_precheck_violations(context: FileScanContext) -> dict[str,
list[str]]:
+def collect_precheck_violations(path: Path, source: str, method_bodies:
dict[str, str]) -> dict[str, list[str]]:
return {
- "checkstyle-final-parameters":
checkstyle_preview_final_parameters(context.path, context.source),
- "parameterized-methodsource": check_r15_d(context.path,
context.source, context.method_bodies),
- "parameterized-name-parameter": check_r15_e(context.path,
context.source),
+ "checkstyle-final-parameters":
checkstyle_preview_final_parameters(path, source),
+ "parameterized-methodsource": check_r15_d(path, source, method_bodies),
+ "parameterized-name-parameter": check_r15_e(path, source),
}
@@ -808,12 +808,12 @@ def create_file_scan_context(path: Path, scope_baseline:
dict | None = None) ->
)
-def file_rule_violations(context: FileScanContext) -> dict[str, list[str]]:
+def file_rule_violations(context: FileScanContext, precheck_violations:
dict[str, list[str]]) -> dict[str, list[str]]:
violations = defaultdict(list)
violations["R8"].extend(check_parameterized_name(context.path,
context.source))
violations["R14"].extend(check_r14(context.path, context.source))
- violations["R15-D"].extend(check_r15_d(context.path, context.source,
context.method_bodies))
- violations["R15-E"].extend(check_r15_e(context.path, context.source))
+
violations["R15-D"].extend(precheck_violations["parameterized-methodsource"])
+
violations["R15-E"].extend(precheck_violations["parameterized-name-parameter"])
violations["R15-F"].extend(check_r15_f(context.path, context.source))
violations["R15-H"].extend(check_r15_h(context.path, context.source))
violations["R15-I"].extend(check_r15_i(context.path, context.source,
context.method_bodies))
@@ -832,17 +832,36 @@ def build_rule_result(violations_by_rule: dict[str,
list[str]]) -> dict[str, dic
}
-def collect_scan_result(paths: list[Path], scope_baseline_path: Path | None)
-> dict:
+def collect_scan_result(paths: list[Path], scope_baseline_path: Path | None,
precheck_only: bool = False) -> dict:
violations_by_rule = defaultdict(list)
precheck_violations = defaultdict(list)
java_paths = [each for each in paths if each.suffix == ".java"]
- scope_baseline = load_scope_baseline(scope_baseline_path) if
scope_baseline_path and scope_baseline_path.exists() else None
- contexts = [create_file_scan_context(each, scope_baseline) for each in
java_paths]
- for context in contexts:
- for rule, entries in file_rule_violations(context).items():
- violations_by_rule[rule].extend(entries)
- for name, entries in collect_precheck_violations(context).items():
+ scope_baseline = load_scope_baseline(scope_baseline_path) if not
precheck_only and scope_baseline_path and scope_baseline_path.exists() else None
+ contexts = []
+ for path in java_paths:
+ if precheck_only:
+ source = path.read_text(encoding="utf-8")
+ method_bodies = {name: body for name, (_, body) in
parse_method_blocks(source).items()}
+ else:
+ context = create_file_scan_context(path, scope_baseline)
+ contexts.append(context)
+ source = context.source
+ method_bodies = context.method_bodies
+ file_prechecks = collect_precheck_violations(path, source,
method_bodies)
+ for name, entries in file_prechecks.items():
precheck_violations[name].extend(entries)
+ if not precheck_only:
+ for rule, entries in file_rule_violations(context,
file_prechecks).items():
+ violations_by_rule[rule].extend(entries)
+ prechecks = {
+ name: {
+ "message": PRECHECK_MESSAGES[name],
+ "violations": precheck_violations[name],
+ }
+ for name in PRECHECK_ORDER
+ }
+ if precheck_only:
+ return {"prechecks": prechecks}
violations_by_rule["R15-C"].extend(
check_scope_baseline(scope_baseline_path, paths) if
scope_baseline_path else ["missing scope baseline"]
)
@@ -853,13 +872,7 @@ def collect_scan_result(paths: list[Path],
scope_baseline_path: Path | None) ->
]
return {
"rules": build_rule_result(violations_by_rule),
- "prechecks": {
- name: {
- "message": PRECHECK_MESSAGES[name],
- "violations": precheck_violations[name],
- }
- for name in PRECHECK_ORDER
- },
+ "prechecks": prechecks,
"candidates": [asdict(each) for context in contexts for each in
context.candidates],
"reviews": {
"R15-A": [str(context.path) for context in contexts if not
context.target_public_methods],
@@ -954,7 +967,7 @@ def main() -> int:
print(f"[scope-baseline] path={output_path.resolve()}
allowedPathCount={len(paths)}")
return 0
baseline_path = Path(args.scope_baseline) if args.scope_baseline else
None
- result = collect_scan_result(paths, baseline_path)
+ result = collect_scan_result(paths, baseline_path,
precheck_only=args.precheck_only and not args.json)
if args.json:
print(json.dumps(result, indent=2, sort_keys=True))
return 1 if failed_rule_names(result) else 0
diff --git a/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
b/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
index 0443cd68864..e7d3d61060b 100644
--- a/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
+++ b/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
@@ -15,6 +15,7 @@
# limitations under the License.
#
+import io
import json
import subprocess
import sys
@@ -167,6 +168,136 @@ class ScanQualityRulesTest(unittest.TestCase):
actual =
rules.resolve_target_source_path(Path("src/test/java/sample/TargetTest.java"),
source)
self.assertEqual(Path("src/main/java/sample/Target.java"), actual)
+ def test_scan_reuses_parameterized_prechecks(self):
+ source = 'class TargetTest {\n@ParameterizedTest(name = "{0}")\nvoid
assertRun(int value) {}\n}'
+ with tempfile.TemporaryDirectory() as temp_dir:
+ paths = [Path(temp_dir) / "TargetTest.java", Path(temp_dir) /
"OtherTest.java"]
+ for path in paths:
+ path.write_text(source, encoding="utf-8")
+ with patch.object(rules, "check_r15_d", wraps=rules.check_r15_d)
as check_d, patch.object(rules, "check_r15_e", wraps=rules.check_r15_e) as
check_e:
+ actual = rules.collect_scan_result([*paths, Path(temp_dir) /
"sample.bin"], None)
+ self.assertEqual(2, check_d.call_count)
+ self.assertEqual(2, check_e.call_count)
+ expected_d = [f"{path}:2 method=assertRun missing @MethodSource" for
path in paths]
+ expected_e = [f"{path}:2 method=assertRun firstParam=int value" for
path in paths]
+ expected_violations = {"R15-C": ["missing scope baseline"], "R15-D":
expected_d, "R15-E": expected_e}
+ self.assertEqual({
+ "rules": {
+ each.name: {"message": each.message, "mode": each.mode,
"violations": expected_violations.get(each.name, [])}
+ for each in rules.RULE_SPECS
+ },
+ "prechecks": {
+ "checkstyle-final-parameters": {
+ "message":
rules.PRECHECK_MESSAGES["checkstyle-final-parameters"],
+ "violations": [f"{path}:2 method=assertRun param=int
value" for path in paths],
+ },
+ "parameterized-methodsource": {"message":
rules.PRECHECK_MESSAGES["parameterized-methodsource"], "violations":
expected_d},
+ "parameterized-name-parameter": {"message":
rules.PRECHECK_MESSAGES["parameterized-name-parameter"], "violations":
expected_e},
+ },
+ "candidates": [],
+ "reviews": {"R15-A": [str(path) for path in paths], "R15-B": [],
"R15-D": [], "R15-G": [], "R15-I": [], "R15-J": [str(path) for path in paths]},
+ "java_file_count": 2,
+ }, actual)
+
+ def test_precheck_only_skips_full_scan_work(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ path = Path(temp_dir) / "src/test/java/sample/TargetTest.java"
+ target = Path(temp_dir) / "src/main/java/sample/Target.java"
+ baseline = Path(temp_dir) / "scope.json"
+ path.parent.mkdir(parents=True)
+ target.parent.mkdir(parents=True)
+ path.write_text('class TargetTest { @ParameterizedTest(name =
"{0}") @MethodSource("Samples#cases") void assertRun(final String name) {} }',
encoding="utf-8")
+ target.write_bytes(b"\xff")
+ baseline.write_text("invalid baseline", encoding="utf-8")
+ with (
+ patch.object(sys, "argv", ["scan", "--precheck-only",
"--scope-baseline", str(baseline), str(path)]),
+ patch.object(sys, "stdout", new_callable=io.StringIO) as
stdout,
+ patch.object(Path, "read_text", autospec=True,
side_effect=Path.read_text) as read_text,
+ patch.object(rules, "create_file_scan_context",
side_effect=AssertionError("full context requested")),
+ patch.object(rules.subprocess, "run",
side_effect=AssertionError("Git requested")),
+ ):
+ actual = rules.main()
+ read_text.assert_called_once_with(path, encoding="utf-8")
+ self.assertEqual(0, actual)
+ self.assertEqual("[precheck:checkstyle-final-parameters]
ok\n[precheck:parameterized-methodsource]
ok\n[precheck:parameterized-name-parameter] ok\n", stdout.getvalue())
+
+ def test_precheck_only_diagnostics(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ path = Path(temp_dir) / "TargetTest.java"
+ valid_source = (
+ 'class TargetTest { @ParameterizedTest(name = "{0}")
@MethodSource("samples") void assertRun(final String name) {} '
+ 'static Stream<Arguments> samples() { return
Stream.of(Arguments.of("foo"), Arguments.of("bar"), Arguments.of("baz")); } }'
+ )
+ invalid_source = 'class TargetTest { @ParameterizedTest(name =
"{0}") void assertRun(int value) {} }'
+ expected_violations = (
+ "[precheck:checkstyle-final-parameters] test method parameters
should be final to avoid Checkstyle FinalParameters failures\n"
+ f"{path}:1 method=assertRun param=int value\n"
+ "[precheck:parameterized-methodsource] parameterized tests
should use @MethodSource providers with at least 3 Arguments rows\n"
+ f"{path}:1 method=assertRun missing @MethodSource\n"
+ "[precheck:parameterized-name-parameter] parameterized tests
should declare the first parameter exactly as `final String name`\n"
+ f"{path}:1 method=assertRun firstParam=int value\n"
+ )
+ expected_ok = "[precheck:checkstyle-final-parameters]
ok\n[precheck:parameterized-methodsource]
ok\n[precheck:parameterized-name-parameter] ok\n"
+ expected_short_provider = (
+ "[precheck:checkstyle-final-parameters] ok\n"
+ "[precheck:parameterized-methodsource] parameterized tests
should use @MethodSource providers with at least 3 Arguments rows\n"
+ f"{path}:1 method=assertRun argumentsRows=2\n"
+ "[precheck:parameterized-name-parameter] ok\n"
+ )
+ fixtures = (
+ ("valid", path, valid_source, 0, expected_ok, ""),
+ ("violations", path, invalid_source, 1, expected_violations,
""),
+ ("short-provider", path, valid_source.replace(',
Arguments.of("baz")', ""), 1, expected_short_provider, ""),
+ ("missing-java", Path(temp_dir) / "MissingTest.java", None, 2,
"", f"[Errno 2] No such file or directory: '{Path(temp_dir) /
'MissingTest.java'}'\n"),
+ ("non-java", Path(temp_dir) / "sample.bin", None, 0,
expected_ok, ""),
+ )
+ for name, input_path, source, expected_exit, expected_stdout,
expected_stderr in fixtures:
+ with self.subTest(name=name):
+ if source is not None:
+ input_path.write_text(source, encoding="utf-8")
+ with (
+ patch.object(sys, "argv", ["scan", "--precheck-only",
"--summary-only", str(input_path)]),
+ patch.object(sys, "stdout", new_callable=io.StringIO)
as stdout,
+ patch.object(sys, "stderr", new_callable=io.StringIO)
as stderr,
+ ):
+ actual = rules.main()
+ self.assertEqual(expected_exit, actual)
+ self.assertEqual(expected_stdout, stdout.getvalue())
+ self.assertEqual(expected_stderr, stderr.getvalue())
+
+ def test_json_precedes_precheck_and_summary(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ path = Path(temp_dir) / "TargetTest.java"
+ path.write_text("class TargetTest {}", encoding="utf-8")
+ with (
+ patch.object(sys, "argv", ["scan", "--json",
"--precheck-only", "--summary-only", str(path)]),
+ patch.object(sys, "stdout", new_callable=io.StringIO) as
stdout,
+ ):
+ actual = rules.main()
+ result = json.loads(stdout.getvalue())
+ self.assertEqual(1, actual)
+ self.assertEqual(["candidates", "java_file_count", "prechecks",
"reviews", "rules"], list(result))
+ self.assertEqual(["missing scope baseline"],
result["rules"]["R15-C"]["violations"])
+ self.assertEqual(1, result["java_file_count"])
+ self.assertEqual(json.dumps(result, indent=2, sort_keys=True) + "\n",
stdout.getvalue())
+
+ def test_capture_precedes_report_flags(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ repo_root = Path(temp_dir)
+ baseline = repo_root / "scope.json"
+ path = repo_root / "TargetTest.java"
+ with (
+ patch.object(sys, "argv", ["scan", "--capture-scope-baseline",
str(baseline), "--json", "--precheck-only", "--summary-only", str(path)]),
+ patch.object(sys, "stdout", new_callable=io.StringIO) as
stdout,
+ patch.object(rules, "get_repo_root", return_value=repo_root),
+ patch.object(rules, "write_scope_baseline") as write_baseline,
+ patch.object(rules, "collect_scan_result",
side_effect=AssertionError("scan requested")),
+ ):
+ actual = rules.main()
+ write_baseline.assert_called_once_with(baseline, repo_root, [path])
+ self.assertEqual(0, actual)
+ self.assertEqual(f"[scope-baseline] path={baseline.resolve()}
allowedPathCount=1\n", stdout.getvalue())
+
def test_scope_baseline_detects_change_to_preexisting_dirty_file(self):
with tempfile.TemporaryDirectory() as temp_dir:
repo_root = Path(temp_dir)
diff --git a/.codex/skills/review-pr/tests/test_build_review_inventory.py
b/.codex/skills/review-pr/tests/test_build_review_inventory.py
index 7786eb3fc9c..0e8ebf44565 100644
--- a/.codex/skills/review-pr/tests/test_build_review_inventory.py
+++ b/.codex/skills/review-pr/tests/test_build_review_inventory.py
@@ -16,7 +16,6 @@
#
import sys
-import tempfile
import unittest
from argparse import Namespace
from pathlib import Path
@@ -64,25 +63,6 @@ class BuildReviewInventoryTest(unittest.TestCase):
self.assertEqual("AGENTS.md",
actual["changed_files_by_category"]["docs"][0]["path"])
resolve_candidate_changes.assert_called_once_with(Path("/repo"),
"merge-base", "candidate.txt")
- def test_resolve_candidate_changes_includes_untracked_files(self):
- with tempfile.TemporaryDirectory() as temp_dir:
- candidate_file = Path(temp_dir) / "candidate.txt"
- candidate_file.write_text("AGENTS.md\n.codex/new.toml\n",
encoding="utf-8")
- with patch.object(common, "run_git",
side_effect=["M\tAGENTS.md\n", ".codex/new.toml\n"]) as run_git:
- actual = common.resolve_candidate_changes(Path("/repo"),
"baseline", str(candidate_file))
- self.assertEqual([common.ChangedFile(status="M", path="AGENTS.md"),
- common.ChangedFile(status="A",
path=".codex/new.toml")], actual)
- literal_paths = [":(literal)AGENTS.md", ":(literal).codex/new.toml"]
- run_git.assert_any_call(["diff", "--name-status", "baseline", "--",
*literal_paths], Path("/repo"))
- run_git.assert_any_call(["ls-files", "--others", "--exclude-standard",
"--", *literal_paths], Path("/repo"), allow_empty=True)
-
- def test_resolve_candidate_changes_rejects_unchanged_paths(self):
- with tempfile.TemporaryDirectory() as temp_dir:
- candidate_file = Path(temp_dir) / "candidate.txt"
- candidate_file.write_text("AGENTS.md\n", encoding="utf-8")
- with patch.object(common, "run_git", side_effect=["", ""]),
self.assertRaisesRegex(RuntimeError, "not changed"):
- common.resolve_candidate_changes(Path("/repo"), "baseline",
str(candidate_file))
-
if __name__ == "__main__":
unittest.main()
diff --git a/.codex/skills/review-pr/tests/test_review_common.py
b/.codex/skills/review-pr/tests/test_review_common.py
index ce399b9a915..0493a0f772a 100644
--- a/.codex/skills/review-pr/tests/test_review_common.py
+++ b/.codex/skills/review-pr/tests/test_review_common.py
@@ -19,12 +19,14 @@ import sys
import tempfile
import unittest
from pathlib import Path
+from unittest.mock import patch
SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts"
if str(SCRIPTS_DIR) not in sys.path:
sys.path.insert(0, str(SCRIPTS_DIR))
+import review_common as common
from review_common import ChangedFile, categorize, compare_github_files,
parse_name_status
@@ -46,6 +48,25 @@ class ReviewCommonTest(unittest.TestCase):
self.assertEqual("build-config", categorize(".github/README.md"))
self.assertEqual("docs", categorize("docs/community/index.md"))
+ def test_resolve_candidate_changes_includes_untracked_files(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ candidate_file = Path(temp_dir) / "candidate.txt"
+ candidate_file.write_text("AGENTS.md\n.codex/new.toml\n",
encoding="utf-8")
+ with patch.object(common, "run_git",
side_effect=["M\tAGENTS.md\n", ".codex/new.toml\n"]) as run_git:
+ actual = common.resolve_candidate_changes(Path("/repo"),
"baseline", str(candidate_file))
+ self.assertEqual([common.ChangedFile(status="M", path="AGENTS.md"),
+ common.ChangedFile(status="A",
path=".codex/new.toml")], actual)
+ literal_paths = [":(literal)AGENTS.md", ":(literal).codex/new.toml"]
+ run_git.assert_any_call(["diff", "--name-status", "baseline", "--",
*literal_paths], Path("/repo"))
+ run_git.assert_any_call(["ls-files", "--others", "--exclude-standard",
"--", *literal_paths], Path("/repo"), allow_empty=True)
+
+ def test_resolve_candidate_changes_rejects_unchanged_paths(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ candidate_file = Path(temp_dir) / "candidate.txt"
+ candidate_file.write_text("AGENTS.md\n", encoding="utf-8")
+ with patch.object(common, "run_git", side_effect=["", ""]),
self.assertRaisesRegex(RuntimeError, "not changed"):
+ common.resolve_candidate_changes(Path("/repo"), "baseline",
str(candidate_file))
+
def test_compare_github_files_reports_bounded_differences(self):
with tempfile.TemporaryDirectory() as temp_dir:
github_files = Path(temp_dir) / "github-files.txt"
diff --git a/.codex/skills/review-pr/tests/test_review_ledger.py
b/.codex/skills/review-pr/tests/test_review_ledger.py
index c18bce95765..f1c43fd1a0a 100644
--- a/.codex/skills/review-pr/tests/test_review_ledger.py
+++ b/.codex/skills/review-pr/tests/test_review_ledger.py
@@ -712,10 +712,6 @@ class ReviewLedgerTest(unittest.TestCase):
self.save_session_report(actual, len(actual["reviews"]) - 1, report)
self.assertTrue(ledger.validate_session(actual))
- def test_session_allows_second_pair_after_disagreement(self):
- actual = self.create_session([[("F1", "confirmed")], [], [("F1",
"confirmed")], [("F1", "confirmed")]])
- self.assertEqual([], ledger.validate_session(actual))
-
def test_session_preserves_finding_discovered_during_result_review(self):
original = self.create_session([[], [], [("F1", "confirmed")], [("F1",
"confirmed")]])
original["rounds"][0]["findings"].append({
