You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by zh...@apache.org on 2026/10/23 13:56:45 UTC
(shardingsphere) branch master updated: Remove redundant manual review reminders from UT quality scanner (#40357)
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 9499557ae3c Remove redundant manual review reminders from UT quality scanner (#40357)
9499557ae3c is described below
commit 9499557ae3ce47486c3d4b53d7b59e955c752154
Author: Liang Zhang <zh...@apache.org>
AuthorDate: Fri Oct 23 21:56:38 2026 +0800
Remove redundant manual review reminders from UT quality scanner (#40357)
---
.codex/harness/agents/cases.toml | 4 +-
.codex/skills/gen-ut/references/verification.md | 4 +-
.codex/skills/gen-ut/scripts/scan_quality_rules.py | 32 ++++------
.../skills/gen-ut/tests/test_scan_quality_rules.py | 68 ++++++++++++++--------
4 files changed, 58 insertions(+), 50 deletions(-)
diff --git a/.codex/harness/agents/cases.toml b/.codex/harness/agents/cases.toml
index b6c7277ae9e..c79b3401948 100644
--- a/.codex/harness/agents/cases.toml
+++ b/.codex/harness/agents/cases.toml
@@ -3850,7 +3850,7 @@ required_summary_prefix = "[NON_TRANSPORT]=allow; [TRANSPORT]=allow; [UNKNOWN]=r
max_summary_chars = 58
critical = true
policy_binding_profile = "gen-ut"
-policy_assertions = [{source = "gen-ut", statements = ["`R15-G`: inspect each new nested-type candidate used in argument sets against the grouping rule in `CODE_OF_CONDUCT.md`.", "A nested type is not a violation by itself.", "Unresolved grouping keeps the task at `R10-D`."]}, {source = "gen-ut-verification", statements = ["The scanner labels semantic or partly semantic rules as `semanticReviewRequired=true`. Resolve them through source inspection and record evidence; zero mechanical vio [...]
+policy_assertions = [{source = "gen-ut", statements = ["`R15-G`: inspect each new nested-type candidate used in argument sets against the grouping rule in `CODE_OF_CONDUCT.md`.", "A nested type is not a violation by itself.", "Unresolved grouping keeps the task at `R10-D`."]}, {source = "gen-ut-verification", statements = ["Complete the semantic reviews required by `R14` and `R15` from source and record evidence; zero mechanical violations alone is not `R10-A`."]}, {source = "code-of-con [...]
[[cases]]
id = "issue_uses_applicable_component_first_party_evidence"
@@ -3955,7 +3955,7 @@ required_summary_prefix = "[MISSING_SCOPE_BASELINE]=block; [OUTSIDE_SCOPE_WRITE]
max_summary_chars = 107
critical = true
policy_binding_profile = "gen-ut"
-policy_assertions = [{source = "gen-ut", statements = ["Edit only `<ResolvedTestFileSet>` under `src/test/java` or `src/test/resources`.", "Do not edit production, generated output, unrelated tests, or another file type.", "Capture `Task scope baseline` after resolving the file set and before editing.", "Any candidate-relevant changed path outside that set is a failure, including further mutation of a path already dirty at task start.", "Exclude only untracked or ignored reproducible ver [...]
+policy_assertions = [{source = "gen-ut", statements = ["Edit only `<ResolvedTestFileSet>` under `src/test/java` or `src/test/resources`.", "Do not edit production, generated output, unrelated tests, or another file type.", "Capture `Task scope baseline` after resolving the file set and before editing.", "Any candidate-relevant changed path outside that set is a failure, including further mutation of a path already dirty at task start.", "Exclude only untracked or ignored reproducible ver [...]
[[cases]]
id = "same_review_basis_has_same_assessment_across_candidate_types"
diff --git a/.codex/skills/gen-ut/references/verification.md b/.codex/skills/gen-ut/references/verification.md
index abaac9795bb..b5f7f21d472 100644
--- a/.codex/skills/gen-ut/references/verification.md
+++ b/.codex/skills/gen-ut/references/verification.md
@@ -93,8 +93,8 @@ python3 .codex/skills/gen-ut/scripts/scan_quality_rules.py --json \
<ResolvedTestFileSet>
```
-The scanner labels semantic or partly semantic rules as `semanticReviewRequired=true`. Resolve them through source inspection and record evidence; zero mechanical violations alone is not `R10-A`.
-In JSON output, review the `manual` and `hybrid` rules and their `reviews` entries.
+Complete the semantic reviews required by `R14` and `R15` from source and record evidence; zero mechanical violations alone is not `R10-A`.
+In JSON output, inspect `rules` for mechanical results, `candidates` for parameterization groups, and `reviews` for source locations and files without resolved target public-method information.
Inspect actual argument sets for every standard JUnit source, including external or dynamic providers; source-code call counts do not prove executed row counts.
Review every test against the test-value rule in `CODE_OF_CONDUCT.md`, independently of method names.
Review `switch`, `Consumer`, nested-type, and assertion-control-flow candidates in parameterized tests against the rule in `CODE_OF_CONDUCT.md` for combining cases that invoke the same method under test and use the same form of assertion.
diff --git a/.codex/skills/gen-ut/scripts/scan_quality_rules.py b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
index 2649bb8d912..2ef28a8b17e 100644
--- a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
+++ b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
@@ -61,16 +61,8 @@ class FileScanContext:
RULE_SPECS = (
- RuleSpec("R14", "boolean assertions require repository-standard source review", "manual"),
- RuleSpec("R15-A", "parameterization suitability requires semantic evidence", "manual"),
- RuleSpec("R15-B", "test value and owned or external contracts require source review", "manual"),
RuleSpec("R15-C", "out-of-scope worktree mutation detected", "automated"),
- RuleSpec("R15-D", "each parameterized test requires at least three actual argument sets", "manual"),
- RuleSpec("R15-F", "switch candidates require method and assertion grouping review when parameterized", "manual"),
- RuleSpec("R15-G", "added nested types require method and assertion grouping review when used in argument sets", "manual"),
- RuleSpec("R15-H", "boolean assertions and parameterized grouping require source review", "manual"),
- RuleSpec("R15-I", "Consumer candidates require method and assertion grouping review when used in argument sets", "manual"),
- RuleSpec("R15-J", "helper and provider target invocations require semantic ownership review", "hybrid"),
+ RuleSpec("R15-J", "helper or provider invokes a target public method", "hybrid"),
)
CONSUMER_TOKEN_PATTERN = re.compile(r"\bConsumer\s*(?:<|\b)")
CONSTRUCTOR_CALL_PATTERN = re.compile(r"\bnew\s+(\w+)\s*\(")
@@ -528,8 +520,7 @@ def analyze_parameterization_candidates(path: Path, source: str, target_public_m
def describe_candidate(candidate: dict) -> str:
- decision = "already parameterized" if candidate["parameterized_present"] else "manual-review-required"
- return f'{candidate["path"]}: method={candidate["method"]} plainTestCount={candidate["plain_test_count"]} parameterizedPresent={candidate["parameterized_present"]} decision={decision}'
+ return f'{candidate["path"]}: method={candidate["method"]} plainTestCount={candidate["plain_test_count"]} parameterizedPresent={candidate["parameterized_present"]}'
def find_switch_dispatch_candidates(path: Path, source: str) -> list[str]:
@@ -650,7 +641,7 @@ def find_argument_source_reviews(path: Path, source: str) -> list[str]:
method_name = match.group(2)
providers = parse_method_sources(method_name, match.group(1))
detail = f"providers={','.join(providers)}" if providers else "configured argument source"
- result.append(f"{path}:{line_number(source, match.start())} method={method_name} {detail}; verify at least three actual argument sets")
+ result.append(f"{path}:{line_number(source, match.start())} method={method_name} {detail}")
return result
@@ -668,9 +659,6 @@ def collect_scan_result(paths: list[Path], scope_baseline_path: Path | None) ->
"rules": build_rule_result(violations_by_rule),
"candidates": [asdict(each) for context in contexts for each in context.candidates],
"reviews": {
- "R14": [str(context.path) for context in contexts],
- "R15-A": [str(context.path) for context in contexts if not context.target_public_methods],
- "R15-B": [str(context.path) for context in contexts],
"R15-D": [each for context in contexts for each in find_argument_source_reviews(context.path, context.source)],
"R15-F": [each for context in contexts for each in find_switch_dispatch_candidates(context.path, context.source)],
"R15-G": [each for context in contexts for each in check_r15_g(context.path, context.source, context.added_lines)],
@@ -697,12 +685,11 @@ def print_rule_summary(result: dict) -> int:
continue
if "automated" == each.mode:
print(f"[{each.name}] ok")
- elif "manual" == each.mode:
- print(f"[{each.name}] semanticReviewRequired=true")
else:
- print(f"[{each.name}] mechanical=ok semanticReviewRequired=true")
- for review in result.get("reviews", {}).get(each.name, []):
- print(f"review: {review}")
+ print(f"[{each.name}] mechanical=ok")
+ for name, reviews in result.get("reviews", {}).items():
+ for review in reviews:
+ print(f"[{name}] review: {review}")
return 1 if failed_rules else 0
@@ -716,8 +703,9 @@ def print_summary_only(result: dict) -> int:
print("[R8-CANDIDATES] no candidates")
failed_rules = [f"{each.name}={len(result['rules'][each.name]['violations'])}" for each in RULE_SPECS if result["rules"][each.name]["violations"]]
print(f"[summary] javaFiles={result['java_file_count']}")
- manual_rules = ",".join(each.name for each in RULE_SPECS if "automated" != each.mode)
- print(f"[summary] semanticReviewRequired={manual_rules}")
+ reviews = [f"{name}={len(locations)}" for name, locations in result.get("reviews", {}).items() if locations]
+ if reviews:
+ print(f"[summary] reviews={' '.join(reviews)}")
if failed_rules:
print(f"[summary] violations={' '.join(failed_rules)}")
return 1
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 cb8e6ba33a9..c01f7a2602d 100644
--- a/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
+++ b/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
@@ -35,7 +35,7 @@ import scan_quality_rules as rules
class ScanQualityRulesTest(unittest.TestCase):
- def test_candidate_count_requires_manual_decision(self):
+ def test_candidate_reports_facts_without_deciding_suitability(self):
source = """
class TargetTest {
@Test void assertTargetFirst() { assertTrue(target.run(1)); }
@@ -46,7 +46,7 @@ class ScanQualityRulesTest(unittest.TestCase):
candidates = rules.analyze_parameterization_candidates(Path("TargetTest.java"), source, {"run"})
self.assertEqual(1, len(candidates))
self.assertNotIn("high_fit", asdict(candidates[0]))
- self.assertIn("decision=manual-review-required", rules.describe_candidate(asdict(candidates[0])))
+ self.assertEqual("TargetTest.java: method=run plainTestCount=3 parameterizedPresent=False", rules.describe_candidate(asdict(candidates[0])))
def test_non_target_invocation_is_not_candidate(self):
source = """
@@ -113,8 +113,6 @@ class ScanQualityRulesTest(unittest.TestCase):
)
with patch.object(rules, "create_file_scan_context", return_value=context), patch.object(rules, "check_scope_baseline", return_value=[]):
actual = rules.collect_scan_result([path], Path("foo_scope_baseline.json"))
- self.assertEqual("manual", actual["rules"]["R15-G"]["mode"])
- self.assertEqual([], actual["rules"]["R15-G"]["violations"])
self.assertEqual([expected_review], actual["reviews"]["R15-G"])
self.assertEqual([], rules.failed_rule_names(actual))
@@ -129,21 +127,18 @@ class ScanQualityRulesTest(unittest.TestCase):
with self.subTest(name=name):
self.assertEqual([], rules.check_r15_g(Path("TargetTest.java"), candidate_source, added_lines))
- def test_test_value_review_is_independent_of_method_name(self):
+ def test_scan_reports_only_automatic_rules_and_source_locations(self):
path = Path("TargetTest.java")
- for method_name in ("assertGetType", "assertGetOrder", "assertGetTypeClass", "assertRun"):
- with self.subTest(method_name=method_name):
- source = f"class TargetTest {{ @Test void {method_name}() {{ assertTrue(target.run()); }} }}"
- context = rules.FileScanContext(
- path=path, source=source, method_bodies={}, method_blocks={}, candidates=[],
- target_type_name="Target", target_public_methods={"run"}, added_lines=[],
- )
- with patch.object(rules, "create_file_scan_context", return_value=context), patch.object(rules, "check_scope_baseline", return_value=[]):
- actual = rules.collect_scan_result([path], Path("scope.json"))
- self.assertEqual("manual", actual["rules"]["R15-B"]["mode"])
- self.assertEqual([], actual["rules"]["R15-B"]["violations"])
- self.assertEqual([str(path)], actual["reviews"]["R15-B"])
- self.assertEqual([], rules.failed_rule_names(actual))
+ context = rules.FileScanContext(
+ path=path, source="class TargetTest { @Test void assertRun() { assertTrue(target.run()); } }",
+ method_bodies={}, method_blocks={}, candidates=[], target_type_name="Target", target_public_methods={"run"}, added_lines=[],
+ )
+ with patch.object(rules, "create_file_scan_context", return_value=context), patch.object(rules, "check_scope_baseline", return_value=[]):
+ actual = rules.collect_scan_result([path], Path("scope.json"))
+ self.assertEqual({"R15-C", "R15-J"}, set(actual["rules"]))
+ self.assertEqual({"R15-D", "R15-F", "R15-G", "R15-H", "R15-I", "R15-J"}, set(actual["reviews"]))
+ self.assertTrue(all(not locations for locations in actual["reviews"].values()))
+ self.assertEqual([], rules.failed_rule_names(actual))
def test_boolean_dispatch_rule_reports_switch_review_candidate(self):
path = Path("TargetTest.java")
@@ -162,16 +157,15 @@ class ScanQualityRulesTest(unittest.TestCase):
with patch.object(rules, "create_file_scan_context", return_value=context), patch.object(rules, "check_scope_baseline", return_value=[]):
actual = rules.collect_scan_result([path], Path("scope.json"))
self.assertEqual(["TargetTest.java:3 method=assertRun"], actual["reviews"]["R15-H"])
- self.assertEqual("manual", actual["rules"]["R15-H"]["mode"])
self.assertEqual([], rules.failed_rule_names(actual))
- def test_standard_argument_sources_require_actual_row_count_review(self):
+ def test_standard_argument_sources_report_locations(self):
for annotation in ('@CsvSource({"foo,1", "bar,2", "baz,3"})', '@ValueSource(strings = {"foo", "bar", "baz"})', '@MethodSource("cases")'):
with self.subTest(annotation=annotation):
source = f'@ParameterizedTest(name = "{{0}}") {annotation} void assertRun(final String name) {{ target.run(name); }}'
actual = rules.find_argument_source_reviews(Path("TargetTest.java"), source)
- self.assertEqual(1, len(actual))
- self.assertIn("verify at least three actual argument sets", actual[0])
+ detail = "providers=cases" if "@MethodSource" in annotation else "configured argument source"
+ self.assertEqual([f"TargetTest.java:1 method=assertRun {detail}"], actual)
def test_branch_and_business_callback_candidates_require_review_without_mechanical_rejection(self):
path = Path("TargetTest.java")
@@ -192,10 +186,33 @@ class ScanQualityRulesTest(unittest.TestCase):
self.assertEqual([], rules.failed_rule_names(actual))
for rule in ("R15-F", "R15-I"):
with self.subTest(rule=rule):
- self.assertEqual("manual", actual["rules"][rule]["mode"])
- self.assertEqual([], actual["rules"][rule]["violations"])
self.assertTrue(actual["reviews"][rule])
+ def test_reports_preserve_locations_and_mechanical_exit_codes(self):
+ locations = {name: [f"TargetTest.java:{line} method=assertRun"] for line, name in enumerate(
+ ("R15-D", "R15-F", "R15-G", "R15-H", "R15-I", "R15-J"), start=1,
+ )}
+ candidate = {"path": "TargetTest.java", "method": "run", "plain_test_count": 3, "parameterized_present": False}
+ for failed_rule in (None, "R15-C", "R15-J"):
+ result = {
+ "rules": rules.build_rule_result({name: ["violation"] if name == failed_rule else [] for name in ("R15-C", "R15-J")}),
+ "reviews": locations, "candidates": [candidate], "java_file_count": 1,
+ }
+ for printer in (rules.print_rule_summary, rules.print_summary_only):
+ with self.subTest(failed_rule=failed_rule, printer=printer.__name__), patch.object(sys, "stdout", new_callable=io.StringIO) as stdout:
+ self.assertEqual(int(failed_rule is not None), printer(result))
+ output = stdout.getvalue()
+ self.assertNotIn("semanticReviewRequired", output)
+ self.assertNotIn("decision=", output)
+ for name, entries in locations.items():
+ if printer is rules.print_rule_summary:
+ self.assertIn(f"[{name}] review: {entries[0]}", output)
+ else:
+ self.assertIn(f"{name}=1", output)
+ self.assertNotIn(entries[0], output)
+ if printer is rules.print_summary_only:
+ self.assertIn("TargetTest.java: method=run plainTestCount=3 parameterizedPresent=False", output)
+
def test_repeated_test_is_an_entry_but_helper_still_fails_ownership_gate(self):
source = '''
class TargetTest {
@@ -240,6 +257,9 @@ class ScanQualityRulesTest(unittest.TestCase):
result = json.loads(stdout.getvalue())
self.assertEqual(1, actual)
self.assertEqual(["candidates", "java_file_count", "reviews", "rules"], list(result))
+ self.assertEqual(["R15-C", "R15-J"], list(result["rules"]))
+ self.assertEqual([str(path)], result["reviews"]["R15-J"])
+ self.assertNotIn("R15-A", result["reviews"])
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())