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 62e42bf2699 Simplify Python quality reports and test fixtures (#40221)
62e42bf2699 is described below
commit 62e42bf26996ade1f9b9e1e25b583b7b4b47705e
Author: Liang Zhang <[email protected]>
AuthorDate: Sun Oct 18 17:53:20 2026 +0800
Simplify Python quality reports and test fixtures (#40221)
---
.codex/harness/agents/test_run.py | 115 ++++++---------------
.../gen-ut/scripts/collect_quality_baseline.py | 27 +----
.codex/skills/gen-ut/scripts/scan_quality_rules.py | 38 +++----
.../skills/review-pr/tests/test_review_ledger.py | 40 +++----
4 files changed, 62 insertions(+), 158 deletions(-)
diff --git a/.codex/harness/agents/test_run.py
b/.codex/harness/agents/test_run.py
index 9cc7ec80acf..3a4325d5ca1 100644
--- a/.codex/harness/agents/test_run.py
+++ b/.codex/harness/agents/test_run.py
@@ -75,6 +75,30 @@ class RunTest(unittest.TestCase):
"response_style": case.get("response_style", "concise"),
}
+ def _create_passing_baseline(self) -> dict[str, Any]:
+ return {
+ "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
+ "case_contracts": run.normalize_case_contracts([self.case]),
+ "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
+ "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
+ }
+
+ @staticmethod
+ def _create_evaluation(case_ids: list[str], failed_case_ids: list[str]) ->
dict[str, Any]:
+ return {
+ "evaluation": 1,
+ "case_count": len(case_ids),
+ "case_ids": case_ids,
+ "returned_case_ids": case_ids,
+ "input_bytes": 1,
+ "prompt_sha256": "prompt",
+ "duration_seconds": 1.0,
+ "runner_exit_code": 0,
+ "failed_case_ids": failed_case_ids,
+ "events_log": "events-evaluation-1.jsonl",
+ "stderr_log": "stderr-evaluation-1.log",
+ }
+
def test_duplicate_action(self) -> None:
with patch.object(run, "ACTIONS", [*run.ACTIONS, run.ACTIONS[0]]),
self.assertRaisesRegex(ValueError, "Duplicate actions"):
run.validate_cases([self.case])
@@ -513,12 +537,7 @@ class RunTest(unittest.TestCase):
actual_case["actions"] = ["edit_code"]
actual = {"results": [actual_case]}
results = run.grade([self.case], actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
stability = run.summarize_semantic_stability([self.case], actual,
results, baseline, [])
self.assertFalse(stability["passed"])
self.assertEqual(["semantic-stability-safety-regression"],
stability["blockers"])
@@ -528,12 +547,7 @@ class RunTest(unittest.TestCase):
actual_case["decision"] = "refuse"
actual = {"results": [actual_case]}
results = run.grade([self.case], actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
stability = run.summarize_semantic_stability([self.case], actual,
results, baseline, [])
self.assertFalse(stability["aggregate_non_regression"])
self.assertTrue(stability["aggregate_non_regression_is_diagnostic"])
@@ -544,12 +558,7 @@ class RunTest(unittest.TestCase):
actual_case["decision"] = "refuse"
actual = {"results": [actual_case]}
results = run.grade([self.case], actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
confirmation = {
"safety_regressions": [],
"blockers": [],
@@ -567,12 +576,7 @@ class RunTest(unittest.TestCase):
actual_case["decision"] = "refuse"
actual = {"results": [actual_case]}
results = run.grade([self.case], actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
confirmation = {
"safety_regressions": [],
"blockers": [],
@@ -970,12 +974,7 @@ class RunTest(unittest.TestCase):
cases = [self.case, added_case]
actual = {"results": [self._create_case_result(each, "Proceed.") for
each in cases]}
results = run.grade(cases, actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
confirmation = {
"safety_regressions": [],
"blockers": [],
@@ -999,12 +998,7 @@ class RunTest(unittest.TestCase):
failed_original["decision"] = "refuse"
actual = {"results": [failed_original,
self._create_case_result(added_case, "Proceed.")]}
results = run.grade(cases, actual)
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
confirmation = {
"safety_regressions": [],
"blockers": [],
@@ -1021,12 +1015,7 @@ class RunTest(unittest.TestCase):
def
test_semantic_stability_rejects_unreported_addition_and_removed_original(self)
-> None:
added_case = copy.deepcopy(self.case)
added_case["id"] = "added_case"
- baseline = {
- "evaluator": {"model": run.EVALUATOR_MODEL, "reasoning_effort":
run.EVALUATOR_REASONING_EFFORT},
- "case_contracts": run.normalize_case_contracts([self.case]),
- "evaluations": [{"case_count": 1, "runner_exit_code": 0,
"failed_case_ids": []}],
- "results": [{"case_id": self.case["id"], "critical": True,
"passed": True, "failures": []}],
- }
+ baseline = self._create_passing_baseline()
for cases, contract_changes in (([self.case, added_case], []),
([added_case], ["added_case:case-added"])):
with self.subTest(cases=cases, contract_changes=contract_changes):
actual = {"results": [self._create_case_result(each,
"Proceed.") for each in cases]}
@@ -1186,19 +1175,7 @@ class RunTest(unittest.TestCase):
passing_result = self._create_case_result(case, "Review FooService
without editing it.")
failing_result = self._create_case_result(case, "Refuse.")
failing_result["decision"] = "refuse"
- passing_evaluations = [{
- "evaluation": 1,
- "case_count": 1,
- "case_ids": [case["id"]],
- "returned_case_ids": [case["id"]],
- "input_bytes": 1,
- "prompt_sha256": "prompt",
- "duration_seconds": 1.0,
- "runner_exit_code": 0,
- "failed_case_ids": [],
- "events_log": "events-evaluation-1.jsonl",
- "stderr_log": "stderr-evaluation-1.log",
- }]
+ passing_evaluations = [self._create_evaluation([case["id"]], [])]
failing_evaluations = copy.deepcopy(passing_evaluations)
failing_evaluations[0]["failed_case_ids"] = [case["id"]]
with tempfile.TemporaryDirectory() as output_parent:
@@ -1246,19 +1223,7 @@ class RunTest(unittest.TestCase):
failed_case_ids = [
each["case_id"] for each in run.grade(cases, {"results":
actual_results}) if not each["passed"]
]
- evaluations = [{
- "evaluation": 1,
- "case_count": len(cases),
- "case_ids": case_ids,
- "returned_case_ids": case_ids,
- "input_bytes": 1,
- "prompt_sha256": "prompt",
- "duration_seconds": 1.0,
- "runner_exit_code": 0,
- "failed_case_ids": failed_case_ids,
- "events_log": "events-evaluation-1.jsonl",
- "stderr_log": "stderr-evaluation-1.log",
- }]
+ evaluations = [self._create_evaluation(case_ids, failed_case_ids)]
with tempfile.TemporaryDirectory() as output_parent:
baseline_dir = Path(output_parent) / "baseline"
common_patches = (
@@ -1318,19 +1283,7 @@ class RunTest(unittest.TestCase):
failed_case_ids = [
each["case_id"] for each in run.grade(cases, {"results":
actual_results}) if not each["passed"]
]
- return [{
- "evaluation": 1,
- "case_count": len(cases),
- "case_ids": case_ids,
- "returned_case_ids": case_ids,
- "input_bytes": 1,
- "prompt_sha256": "prompt",
- "duration_seconds": 1.0,
- "runner_exit_code": 0,
- "failed_case_ids": failed_case_ids,
- "events_log": "events-evaluation-1.jsonl",
- "stderr_log": "stderr-evaluation-1.log",
- }]
+ return [self._create_evaluation(case_ids, failed_case_ids)]
confirmation = {
"executed": True,
diff --git a/.codex/skills/gen-ut/scripts/collect_quality_baseline.py
b/.codex/skills/gen-ut/scripts/collect_quality_baseline.py
index ffa1ba3a2e4..405347a5a38 100644
--- a/.codex/skills/gen-ut/scripts/collect_quality_baseline.py
+++ b/.codex/skills/gen-ut/scripts/collect_quality_baseline.py
@@ -92,31 +92,8 @@ def print_rule_baseline(scan_result: dict) -> None:
print(quality_rules.describe_candidate(each))
else:
print("[R8-CANDIDATES] no candidates")
- for rule in quality_rules.RULE_ORDER:
- violations = scan_result["rules"][rule]["violations"]
- if violations:
- print(f"[{rule}] {scan_result['rules'][rule]['message']}")
- for each in violations:
- print(each)
- else:
- mode = scan_result["rules"][rule]["mode"]
- if "automated" == mode:
- print(f"[{rule}] ok")
- elif "manual" == mode:
- print(f"[{rule}] semanticReviewRequired=true")
- else:
- print(f"[{rule}] mechanical=ok semanticReviewRequired=true")
- for each in scan_result.get("reviews", {}).get(rule, []):
- print(f"review: {each}")
- prechecks = scan_result.get("prechecks", {})
- for name in sorted(prechecks):
- violations = prechecks[name]["violations"]
- if not violations:
- print(f"[precheck:{name}] ok")
- continue
- print(f"[precheck:{name}] {prechecks[name]['message']}")
- for each in violations:
- print(each)
+ quality_rules.print_rule_summary(scan_result)
+ quality_rules.print_precheck_summary(scan_result)
def print_coverage_baseline(jacoco_xml_path: Path, target_classes: list[str],
minimum_ratio: float | None) -> bool:
diff --git a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
index 46f016fa48d..fa348550ba4 100644
--- a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
+++ b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
@@ -60,7 +60,6 @@ class FileScanContext:
added_lines: list[str]
-RULE_ORDER = ("R8", "R14", "R15-A", "R15-B", "R15-C", "R15-D", "R15-E",
"R15-F", "R15-G", "R15-H", "R15-I", "R15-J")
PRECHECK_ORDER = (
"checkstyle-final-parameters",
"parameterized-methodsource",
@@ -71,29 +70,20 @@ PRECHECK_MESSAGES = {
"parameterized-methodsource": "parameterized tests should use
@MethodSource providers with at least 3 Arguments rows",
"parameterized-name-parameter": "parameterized tests should declare the
first parameter exactly as `final String name`",
}
-RULE_MESSAGES = {
- "R8": "@ParameterizedTest must use name = \"{0}\"",
- "R14": "forbidden boolean assertion found",
- "R15-A": "parameterization suitability requires semantic evidence",
- "R15-B": "metadata accessor candidates require request-scope review",
- "R15-C": "out-of-scope worktree mutation detected",
- "R15-D": "each @ParameterizedTest must have >= 3 Arguments rows from
@MethodSource",
- "R15-E": "each @ParameterizedTest method must declare first parameter as
`final String name`",
- "R15-F": "@ParameterizedTest method body must not contain switch",
- "R15-G": "added type declarations require nested scenario-transport
review",
- "R15-H": "do not dispatch boolean assertions by control flow to choose
assertTrue/assertFalse",
- "R15-I": "parameterized tests must not use Consumer in signatures or
@MethodSource argument rows",
- "R15-J": "helper and provider target invocations require semantic
ownership review",
-}
-RULE_MODES = {
- "R15-A": "manual",
- "R15-B": "manual",
- "R15-D": "hybrid",
- "R15-G": "manual",
- "R15-I": "hybrid",
- "R15-J": "hybrid",
-}
-RULE_SPECS = tuple(RuleSpec(each, RULE_MESSAGES[each], RULE_MODES.get(each,
"automated")) for each in RULE_ORDER)
+RULE_SPECS = (
+ RuleSpec("R8", "@ParameterizedTest must use name = \"{0}\"", "automated"),
+ RuleSpec("R14", "forbidden boolean assertion found", "automated"),
+ RuleSpec("R15-A", "parameterization suitability requires semantic
evidence", "manual"),
+ RuleSpec("R15-B", "metadata accessor candidates require request-scope
review", "manual"),
+ RuleSpec("R15-C", "out-of-scope worktree mutation detected", "automated"),
+ RuleSpec("R15-D", "each @ParameterizedTest must have >= 3 Arguments rows
from @MethodSource", "hybrid"),
+ RuleSpec("R15-E", "each @ParameterizedTest method must declare first
parameter as `final String name`", "automated"),
+ RuleSpec("R15-F", "@ParameterizedTest method body must not contain
switch", "automated"),
+ RuleSpec("R15-G", "added type declarations require nested
scenario-transport review", "manual"),
+ RuleSpec("R15-H", "do not dispatch boolean assertions by control flow to
choose assertTrue/assertFalse", "automated"),
+ RuleSpec("R15-I", "parameterized tests must not use Consumer in signatures
or @MethodSource argument rows", "hybrid"),
+ RuleSpec("R15-J", "helper and provider target invocations require semantic
ownership review", "hybrid"),
+)
BOOLEAN_ASSERTION_BAN_PATTERN = re.compile(
r"assertThat\s*\((?s:[^;]*?)is\s*\(\s*(?:true|false|Boolean\.TRUE|Boolean\.FALSE)\s*\)\s*\)"
r"|assertEquals\s*\(\s*(?:true|false|Boolean\.TRUE|Boolean\.FALSE)\s*,"
diff --git a/.codex/skills/review-pr/tests/test_review_ledger.py
b/.codex/skills/review-pr/tests/test_review_ledger.py
index 09aa7d23acd..0c5b17c7924 100644
--- a/.codex/skills/review-pr/tests/test_review_ledger.py
+++ b/.codex/skills/review-pr/tests/test_review_ledger.py
@@ -82,20 +82,12 @@ class ReviewLedgerTest(unittest.TestCase):
def test_confirmed_finding_requires_non_blank_proof(self):
actual_ledger = self.create_valid_ledger()
- actual_ledger["findings"] = [{
- "id": "F1",
- "status": "confirmed",
- "origin": " ",
- "fix_boundary": "boundary",
- "evidence": [""],
- "full_path": [" "],
- "counter_evidence": [],
- "necessity": " ",
- "scope_proof": " ",
- "files": [""],
- "reason": "",
- "anchors": [],
- }]
+ finding = self.create_confirmed_finding("F1")
+ finding.update({
+ "origin": " ", "fix_boundary": "boundary", "evidence": [""],
"full_path": [" "], "counter_evidence": [],
+ "necessity": " ", "scope_proof": " ", "files": [""], "anchors": [],
+ })
+ actual_ledger["findings"] = [finding]
errors = ledger.validate_ledger(actual_ledger)
self.assertIn("F1 confirmed finding is missing origin", errors)
self.assertIn("F1 confirmed finding is missing evidence", errors)
@@ -162,20 +154,12 @@ class ReviewLedgerTest(unittest.TestCase):
def test_incomplete_gap_requires_complete_proof(self):
actual_ledger = self.create_valid_ledger()
- actual_ledger["findings"] = [{
- "id": "G1",
- "status": "review-incomplete-gap",
- "origin": " ",
- "fix_boundary": "",
- "evidence": [""],
- "full_path": [" "],
- "counter_evidence": [],
- "necessity": " ",
- "scope_proof": " ",
- "files": [""],
- "reason": " ",
- "anchors": [],
- }]
+ finding = self.create_incomplete_gap()
+ finding.update({
+ "origin": " ", "evidence": [""], "full_path": [" "],
"counter_evidence": [],
+ "necessity": " ", "scope_proof": " ", "files": [""], "reason": "
", "anchors": [],
+ })
+ actual_ledger["findings"] = [finding]
errors = ledger.validate_ledger(actual_ledger, incomplete_result=True)
expected_errors = [
"G1 incomplete gap is missing the unavailable fact",
