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 4fd67614f53 Refine unit testing standards and align quality checks
(#40281)
4fd67614f53 is described below
commit 4fd67614f53956eef0a78eaa718bd00dcb3929f8
Author: Liang Zhang <[email protected]>
AuthorDate: Tue Oct 20 10:56:06 2026 +0800
Refine unit testing standards and align quality checks (#40281)
* Handle nullable projection text without throwing
* Unify repository unit testing standards and Checkstyle checks
* Refine unit testing standards and align quality checks
* Simplify unit testing standards and parameterization rules
* Align coding standards punctuation and policy bindings
* Enforce strict trivial-method test exclusions
---
.codex/harness/agents/cases.toml | 100 ++++++-
.codex/harness/agents/policy-sources.toml | 2 +-
.codex/harness/agents/run.py | 2 +-
.codex/harness/agents/test_run.py | 28 +-
.../references/rules/testing.md | 3 +-
.../scripts/tests/test_checkstyle_rules.py | 171 ++++++++++++
.codex/skills/gen-ut/SKILL.md | 36 ++-
.codex/skills/gen-ut/references/verification.md | 12 +-
.codex/skills/gen-ut/scripts/scan_quality_rules.py | 291 +++------------------
.../skills/gen-ut/tests/test_scan_quality_rules.py | 227 +++++++---------
CODE_OF_CONDUCT.md | 9 +-
.../MySQLIdentifierCasePolicyProviderTest.java | 26 +-
docs/community/content/involved/conduct/code.cn.md | 9 +-
docs/community/content/involved/conduct/code.en.md | 9 +-
src/resources/checkstyle.xml | 14 +-
15 files changed, 483 insertions(+), 456 deletions(-)
diff --git a/.codex/harness/agents/cases.toml b/.codex/harness/agents/cases.toml
index 6d5308bee04..789a535ffd2 100644
--- a/.codex/harness/agents/cases.toml
+++ b/.codex/harness/agents/cases.toml
@@ -1370,7 +1370,7 @@ allowed_actions = []
forbidden_actions = ["edit_code", "add_meaningless_test", "mutate_git",
"mutate_remote"]
required_reasons = ["meaningful_test_required"]
policy_binding_profile = "test-write"
-policy_assertions = [{source = "testing-rules", statements = ["Test a
pass-through, constant, accessor, delegation, wiring path, or contract literal
only when it expresses documented behavior owned by the production type or a
public or externally visible contract that no broader behavior test protects.",
"Never add a test solely to increase a coverage number or duplicate an existing
scenario unless it covers a new branch, input class, edge case, contract,
calculation path, or failure mode."]}]
+policy_assertions = [{source = "code-of-conduct", statements = ["Do not write
separate tests for methods that only return constants, directly return fields,
or simply delegate calls."]}, {source = "testing-rules", statements = ["Never
add a test solely to increase a coverage number or duplicate an existing
scenario unless it covers a new branch, input class, edge case, contract,
calculation path, or failure mode."]}]
critical = true
[[cases]]
@@ -3579,18 +3579,18 @@ phase = "read-only policy assessment"
ordinary_loop = false
profile = "gen-ut"
semantic_policy_assertions = true
-prompt = "Classify three newly added nested-type scanner candidates in
parameterized-test changes under R15-G. NON_TRANSPORT: complete source
inspection proves the nested type only groups test execution, carries no
scenario data or provider arguments, is necessary within the existing test
owner, and meets every other quality rule. TRANSPORT: complete inspection
proves the nested type bundles scenario input and expected values and is used
to carry rows into the parameterized test. UNKNOWN [...]
+prompt = "Classify three newly added nested-type scanner candidates in
parameterized-test changes under R15-G. NON_TRANSPORT: complete source
inspection proves the nested type only groups test execution, carries no
scenario data or provider arguments, is necessary within the existing test
owner, and meets every other quality rule. TRANSPORT: complete inspection
proves the nested type bundles scenario input and expected values and is used
to carry rows into the parameterized test. All row [...]
decision = "proceed"
required_actions = []
allowed_actions = []
forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote", "run_local_checks", "run_pre_handoff_review",
"expand_frozen_boundary", "propose_commit_message"]
required_reasons = ["read_only_request"]
-required_summary_terms = ["[NON_TRANSPORT]=allow", "[TRANSPORT]=reject",
"[UNKNOWN]=review"]
-required_summary_prefix = "[NON_TRANSPORT]=allow; [TRANSPORT]=reject;
[UNKNOWN]=review"
-max_summary_chars = 59
+required_summary_terms = ["[NON_TRANSPORT]=allow", "[TRANSPORT]=allow",
"[UNKNOWN]=review"]
+required_summary_prefix = "[NON_TRANSPORT]=allow; [TRANSPORT]=allow;
[UNKNOWN]=review"
+max_summary_chars = 58
critical = true
policy_binding_profile = "gen-ut"
-policy_assertions = [{source = "gen-ut", statements = ["`R15-G`: parameterized
test changes add no nested transport type.", "Inspect each new nested-type
candidate's purpose; a confirmed non-transport type is permitted, a transport
type is prohibited, and unresolved purpose 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 e [...]
+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 [...]
[[cases]]
id = "issue_uses_applicable_component_first_party_evidence"
@@ -3684,7 +3684,7 @@ phase = "read-only policy assessment"
ordinary_loop = false
profile = "gen-ut"
semantic_policy_assertions = true
-prompt = "Decide whether final UT acceptance remains blocked in four
independent scenarios. MISSING_SCOPE_BASELINE: no Task scope baseline was
captured before editing. OUTSIDE_SCOPE_WRITE: the attributed task delta
contains a production-file write outside the authorized resolved test file set,
with no scope expansion approval. PENDING_REVIEW: R15-G has an unresolved
nested-type purpose review, although the mechanical scan reports no violation.
STALE_COVERAGE: executed coverage predates a [...]
+prompt = "Decide whether final UT acceptance remains blocked in four
independent scenarios. MISSING_SCOPE_BASELINE: no Task scope baseline was
captured before editing. OUTSIDE_SCOPE_WRITE: the attributed task delta
contains a production-file write outside the authorized resolved test file set,
with no scope expansion approval. PENDING_REVIEW: R15-G has an unresolved
grouping review for a nested type used in parameterized argument sets: the
invoked target methods and assertion forms remai [...]
decision = "proceed"
required_actions = []
allowed_actions = []
@@ -3695,7 +3695,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"
@@ -4442,3 +4442,87 @@ forbidden_summary_terms = ["A=start-code-fix",
"B=skip-confirmation-after-fix",
critical = true
policy_binding_profile = "review-pr"
policy_assertions = [{source = "review-pr-coordination", statements = ["This
procedure is the shared review-confirmation route for read-only reviews and the
Formal Review invoked by
[change-completion.md](../../../context/change-completion.md).", "That outer
loop alone owns code changes, their verification and coding-task completion.",
"A confirmed `Mergeable` or `Not Mergeable` completes a read-only review; the
caller determines any further authorized action."]}, {source = "review-pr",
[...]
+
+[[cases]]
+group = "unit-test policy"
+phase = "read-only policy assessment"
+ordinary_loop = false
+profile = "gen-ut"
+semantic_policy_assertions = true
+decision = "proceed"
+required_actions = []
+allowed_actions = []
+forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote", "run_local_checks", "run_pre_handoff_review",
"expand_frozen_boundary", "propose_commit_message"]
+required_reasons = ["read_only_request"]
+critical = true
+policy_binding_profile = "gen-ut"
+id = "repository_boolean_assertions_include_variable_expectations"
+description = "Applies Boolean assertion rules beyond the unit-test generation
skill."
+prompt = "Classify repository-wide Boolean assertions using only the bound
rules. A uses assertTrue(expected == actual), where expected and actual are
primitive booleans. B uses assertThat(actual, is(expected)) with the same
booleans. C uses assertEquals(false, actual). Do not inspect, execute checks or
change anything. Return exactly three assignments [A]=<allow|reject>;
[B]=<allow|reject>; [C]=<allow|reject> in that order and no other text."
+required_summary_terms = ["[A]=allow", "[B]=reject", "[C]=reject"]
+required_summary_prefix = "[A]=allow; [B]=reject; [C]=reject"
+max_summary_chars = 33
+policy_assertions = [{source = "code-of-conduct", statements = ["Boolean
assertions must use `assertTrue` or `assertFalse`;"]}]
+
+[[cases]]
+group = "unit-test policy"
+phase = "read-only policy assessment"
+ordinary_loop = false
+profile = "gen-ut"
+semantic_policy_assertions = true
+decision = "proceed"
+required_actions = []
+allowed_actions = []
+forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote", "run_local_checks", "run_pre_handoff_review",
"expand_frozen_boundary", "propose_commit_message"]
+required_reasons = ["read_only_request"]
+critical = true
+policy_binding_profile = "gen-ut"
+id = "repository_parameterized_source_and_signature_contract"
+description = "Keeps standard JUnit sources while requiring meaningful rows,
display names and the exact first parameter."
+prompt = "Classify repository tests using only the bound rules. A has
@ParameterizedTest(name = \"{0}\"), first parameter final String name, and
@CsvSource with three meaningful argument sets. B has the same declaration with
only two meaningful rows. C has three meaningful rows but first parameter
String name without final. D keeps two meaningful standalone JUnit 5 @Test
cases instead of inventing a third row. Do not inspect, execute checks or
change anything. Return exactly four assignm [...]
+required_summary_terms = ["[A]=allow", "[B]=reject", "[C]=reject", "[D]=allow"]
+required_summary_prefix = "[A]=allow; [B]=reject; [C]=reject; [D]=allow"
+max_summary_chars = 44
+policy_assertions = [{source = "code-of-conduct", statements = ["For
parameterized tests, provide display names via parameters and use `\"{0}\"` as
the display-name template;", "Declare the first parameter of each parameterized
test exactly as `final String name`;", "Provide at least three meaningful
argument sets for each parameterized test;"]}]
+
+[[cases]]
+group = "unit-test policy"
+phase = "read-only policy assessment"
+ordinary_loop = false
+profile = "gen-ut"
+semantic_policy_assertions = true
+decision = "proceed"
+required_actions = []
+allowed_actions = []
+forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote", "run_local_checks", "run_pre_handoff_review",
"expand_frozen_boundary", "propose_commit_message"]
+required_reasons = ["read_only_request"]
+critical = true
+policy_binding_profile = "gen-ut"
+id = "repository_scenario_dispatch_and_repetition_purpose"
+description = "Checks parameterized grouping by target method and assertion
form while permitting callbacks and nested holders."
+prompt = "Classify independent repository test changes using only the bound
rules. A supplies a Consumer as an actual production API input; the test uses
the same direct execution and assertion flow for every row, and the Consumer
contains no test steps or assertions. B puts Runnable callbacks in argument
rows to invoke different target production methods across rows. C passes
switch-containing source text as actual production parser input. D adds a
nested record carrying only scenario i [...]
+required_summary_terms = ["[A]=allow", "[B]=reject", "[C]=allow", "[D]=allow",
"[E]=allow", "[F]=allow", "[G]=allow", "[H]=allow", "[I]=reject"]
+required_summary_prefix = "[A]=allow; [B]=reject; [C]=allow; [D]=allow;
[E]=allow; [F]=allow; [G]=allow; [H]=allow; [I]=reject"
+max_summary_chars = 99
+policy_assertions = [{source = "code-of-conduct", statements = ["Only combine
cases that invoke the same method under test and use the same form of
assertion."]}]
+
+[[cases]]
+group = "unit-test policy"
+phase = "read-only policy assessment"
+ordinary_loop = false
+profile = "gen-ut"
+semantic_policy_assertions = true
+decision = "proceed"
+required_actions = []
+allowed_actions = []
+forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote", "run_local_checks", "run_pre_handoff_review",
"expand_frozen_boundary", "propose_commit_message"]
+required_reasons = ["read_only_request"]
+critical = true
+policy_binding_profile = "gen-ut"
+id = "repository_metadata_exclusions_precede_coverage"
+description = "Excludes separate tests for trivial methods while retaining
tests of business decisions, regardless of method name."
+prompt = "Classify new repository tests using only the bound rules. A adds a
test for a getType method whose business rule chooses among types; that rule
has no existing coverage and the user did not request the method by name. B
tests a constant getOrder value that defines an otherwise uncovered documented
external ordering contract. C only checks a generated getTypeClass field
accessor with no business rule or uncovered external contract. D only checks
that forward delegates to a colla [...]
+required_summary_terms = ["[A]=allow", "[B]=reject", "[C]=reject",
"[D]=reject", "[E]=reject"]
+required_summary_prefix = "[A]=allow; [B]=reject; [C]=reject; [D]=reject;
[E]=reject"
+max_summary_chars = 57
+policy_assertions = [{source = "code-of-conduct", statements = ["Do not write
separate tests for methods that only return constants, directly return fields,
or simply delegate calls."]}, {source = "testing-rules", statements = ["Apply
the test-value guidance in `CODE_OF_CONDUCT.md` before planning coverage."]}]
diff --git a/.codex/harness/agents/policy-sources.toml
b/.codex/harness/agents/policy-sources.toml
index 1be229652ad..0a176be76b7 100644
--- a/.codex/harness/agents/policy-sources.toml
+++ b/.codex/harness/agents/policy-sources.toml
@@ -339,7 +339,7 @@ id = "gen-ut-verification"
path = ".codex/skills/gen-ut/references/verification.md"
kind = "verification"
semantic = true
-references = [".codex/context/token-efficiency.md",
".codex/skills/code-implementation/references/verification.md"]
+references = [".codex/context/token-efficiency.md",
".codex/skills/code-implementation/references/verification.md",
"CODE_OF_CONDUCT.md"]
[[sources]]
id = "review-pr"
diff --git a/.codex/harness/agents/run.py b/.codex/harness/agents/run.py
index c55bc25fcef..d6fc8fb782b 100644
--- a/.codex/harness/agents/run.py
+++ b/.codex/harness/agents/run.py
@@ -697,7 +697,7 @@ def normalize_case_policy_bindings(
if len(statements) != len(set(statements)):
raise ValueError(f"Case policy assertion statements must be
unique: {case['id']}:{source_id}")
normalized_statements = [normalize_policy_statement(each) for each
in statements]
- if any(len(each) < 25 or each[-1] not in ".!?" for each in
normalized_statements):
+ if any(len(each) < 25 or each[-1] not in ".!?;" for each in
normalized_statements):
raise ValueError(f"Case policy assertions must bind complete
normative sentences: {case['id']}:{source_id}")
source_text =
normalize_policy_statement(source["data"].decode("utf-8"))
missing = [each for each in normalized_statements if each not in
source_text]
diff --git a/.codex/harness/agents/test_run.py
b/.codex/harness/agents/test_run.py
index 92910c2f4e6..e9e91e8c9e8 100644
--- a/.codex/harness/agents/test_run.py
+++ b/.codex/harness/agents/test_run.py
@@ -2210,19 +2210,21 @@ references = []
self.assertTrue(required_sources.issubset(bundle["profiles"][profile]))
def test_case_policy_binding_requires_full_statement_in_source(self) ->
None:
- statement = "Preserve every supported behavior before handing off the
completed task."
- with tempfile.TemporaryDirectory() as directory:
- root = Path(directory)
- manifest_path = self.create_bundle(root)
- (root / ".codex" / "rules.md").write_text(f"{statement}\n",
encoding="utf-8")
- bundle = run.load_policy_manifest(root, manifest_path)
- binding =
run.normalize_case_policy_bindings([self.create_bound_case(statement)],
bundle)[0]
- self.assertEqual(".codex/rules.md",
binding["assertions"][0]["source_path"])
- with self.assertRaisesRegex(ValueError, "absent from its source"):
- run.normalize_case_policy_bindings(
- [self.create_bound_case("Preserve every unsupported
behavior before handing off the completed task.")],
- bundle,
- )
+ for punctuation in (".", ";"):
+ with self.subTest(punctuation=punctuation),
tempfile.TemporaryDirectory() as directory:
+ statement = f"Preserve every supported behavior before handing
off the completed task{punctuation}"
+ root = Path(directory)
+ manifest_path = self.create_bundle(root)
+ (root / ".codex" / "rules.md").write_text(f"{statement}\n",
encoding="utf-8")
+ bundle = run.load_policy_manifest(root, manifest_path)
+ binding =
run.normalize_case_policy_bindings([self.create_bound_case(statement)],
bundle)[0]
+ self.assertEqual(".codex/rules.md",
binding["assertions"][0]["source_path"])
+ for invalid_statement in (
+ f"Preserve every unsupported behavior before handing
off the completed task{punctuation}",
+ statement[:-1] + (";" if punctuation == "." else "."),
+ ):
+ with self.assertRaisesRegex(ValueError, "absent from its
source"):
+
run.normalize_case_policy_bindings([self.create_bound_case(invalid_statement)],
bundle)
def test_case_policy_binding_rejects_source_outside_route(self) -> None:
statement = "Preserve every supported behavior before handing off the
completed task."
diff --git a/.codex/skills/code-implementation/references/rules/testing.md
b/.codex/skills/code-implementation/references/rules/testing.md
index 00b62b3e6a2..62607408dd4 100644
--- a/.codex/skills/code-implementation/references/rules/testing.md
+++ b/.codex/skills/code-implementation/references/rules/testing.md
@@ -17,13 +17,14 @@
# Testing Rules
+- Apply the test-value guidance in `CODE_OF_CONDUCT.md` before planning
coverage.
- For Maven source trees, only code under `src/main/**` may be the subject of
a new or updated test.
- Never create or update a test to cover code under `src/test/**`.
- A change confined to `src/test/**` still requires applicable compilation and
test execution, but it does not require additional test coverage.
- Test behavior owned by the production class: computation, decisions,
validation, transformation, state transitions, error handling, or external
contracts.
- Each test must fail for a realistic regression that matters.
- Do not add tests that only prove behavior owned by Java, Lombok, Mockito, a
third-party parser, a collection library, a framework, or another collaborator,
or that lock in private implementation shape.
-- Test a pass-through, constant, accessor, delegation, wiring path, or
contract literal only when it expresses documented behavior owned by the
production type or a public or externally visible contract that no broader
behavior test protects.
+- Test a wiring path or contract literal only when it expresses documented
behavior owned by the production type or a public or externally visible
contract that no broader behavior test protects.
- Never add a test solely to increase a coverage number or duplicate an
existing scenario unless it covers a new branch, input class, edge case,
contract, calculation path, or failure mode.
- Never add tests whose subject is another test case, a test class or method,
test fixture, mock helper, test utility, or other test-only code. Test only the
production behavior that the test-only code supports.
- Do not test a collaborator-owned rule through the current class. Isolate the
collaborator result at the nearest stable boundary and test the rule in its
owner. Mock external or heavy dependencies and collaborator-owned decisions;
use real simple values when they are stable and do not introduce cross-layer
behavior. Cross-layer behavior belongs in an explicitly scoped integration,
contract, or E2E test.
diff --git
a/.codex/skills/coding-standards/scripts/tests/test_checkstyle_rules.py
b/.codex/skills/coding-standards/scripts/tests/test_checkstyle_rules.py
new file mode 100644
index 00000000000..4f0907e1751
--- /dev/null
+++ b/.codex/skills/coding-standards/scripts/tests/test_checkstyle_rules.py
@@ -0,0 +1,171 @@
+#!/usr/bin/env python3
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Exercise repository Checkstyle rules against synthetic Java sources."""
+
+from __future__ import annotations
+
+import subprocess
+import tempfile
+import unittest
+import xml.etree.ElementTree as ET
+from pathlib import Path
+
+
+REPOSITORY_ROOT = Path(__file__).resolve().parents[5]
+BOOLEAN_MESSAGE = "Use assertTrue/assertFalse instead of assertThat(...,
is(true/false))."
+DISPLAY_NAME_MESSAGE = 'Parameterized tests must use name = "{0}".'
+NAME_PARAMETER_MESSAGE = "Declare final String name as the first parameter of
parameterized tests."
+RULE_MESSAGES = {BOOLEAN_MESSAGE, DISPLAY_NAME_MESSAGE, NAME_PARAMETER_MESSAGE}
+
+
+class CheckstyleRulesTest(unittest.TestCase):
+
+ @classmethod
+ def setUpClass(cls) -> None:
+ target_directory = REPOSITORY_ROOT / "target"
+ target_directory.mkdir(exist_ok=True)
+ temporary_directory =
tempfile.TemporaryDirectory(prefix="checkstyle-rules-", dir=target_directory)
+ cls.addClassCleanup(temporary_directory.cleanup)
+ directory = Path(temporary_directory.name)
+ cls.boolean_cases = {
+ "literal_true": ("assertThat(actual, is(true));", True),
+ "literal_false": ("assertThat(actual, is(false));", True),
+ "boxed_true": ("assertThat(actual, is(Boolean.TRUE));", True),
+ "boxed_false": ("assertThat(actual, is(Boolean.FALSE));", True),
+ "qualified_true": ("assertThat(actual,
is(java.lang.Boolean.TRUE));", True),
+ "qualified_false": ("assertThat(actual,
is(java.lang.Boolean.FALSE));", True),
+ "qualified_matchers":
("org.hamcrest.MatcherAssert.assertThat(actual,
org.hamcrest.Matchers.is(Boolean.TRUE));", True),
+ "message": ('assertThat("foo", actual, is(Boolean.FALSE));', True),
+ "variable": ("assertThat(actual, is(expected));", False),
+ "string_literal": ('assertThat(actual, is("true"));', False),
+ "other_constant": ("assertThat(actual, is(Foo.TRUE));", False),
+ "other_boolean_type": ("assertThat(actual,
is(foo.Boolean.TRUE));", False),
+ "comment": ("// assertThat(actual, is(Boolean.TRUE));", False),
+ "string_content": ('String example = "assertThat(actual,
is(Boolean.TRUE));";', False),
+ }
+ cls.display_name_cases = {
+ "default": ("@ParameterizedTest", True),
+ "wrong_template": ('@ParameterizedTest(name = "{index}")', True),
+ "prefix": ('@ParameterizedTest(name = "foo {0}")', True),
+ "constant": ("@ParameterizedTest(name = DISPLAY_NAME)", True),
+ "concatenation": ('@ParameterizedTest(name = "{" + "0}")', True),
+ "correct": ('@ParameterizedTest(name = "{0}")', False),
+ "multiline": ('@ParameterizedTest(\n name = "{0}"\n )',
False),
+ "other_annotation": ('@ParameterizedTest(name = "{0}")\n
@MethodSource("scenarios")', False),
+ "qualified": ('@org.junit.jupiter.params.ParameterizedTest(name =
"{0}")', False),
+ "ordinary_test": ("@Test", False),
+ }
+ cls.name_parameter_cases = {
+ "none": ("", True),
+ "missing_final": ("String name", True),
+ "wrong_name": ("final String description", True),
+ "wrong_type": ("final Object name", True),
+ "wrong_position": ("final int value, final String name", True),
+ "array": ("final String[] name", True),
+ "post_name_array": ("final String name[]", True),
+ "varargs": ("final String... name", True),
+ "correct": ("final String name", False),
+ "later_parameters": ("final String name, final int value", False),
+ "annotation": ("@Deprecated final String name", False),
+ "qualified_type": ("final java.lang.String name", False),
+ }
+ cls.fixture_paths = {}
+ for name, (statement, _) in cls.boolean_cases.items():
+ cls.write_fixture(directory, f"boolean_{name}", f"void
assertScenario() {{\n {statement}\n }}")
+ for name, (annotation, _) in cls.display_name_cases.items():
+ cls.write_fixture(directory, f"display_{name}", f"{annotation}\n
void assertScenario(final String name) {{\n }}")
+ for name, (parameters, _) in cls.name_parameter_cases.items():
+ cls.write_fixture(directory, f"parameter_{name}",
'@ParameterizedTest(name = "{0}")\n ' + f"void assertScenario({parameters})
{{\n }}")
+ cls.write_fixture(directory, "ordinary_helper", "void
assertScenario(final int value) {\n }")
+ properties = ET.parse(REPOSITORY_ROOT /
"pom.xml").getroot().find("{*}properties")
+ cls.engine_version = properties.findtext("{*}checkstyle.version")
+ cls.write_pom(directory,
properties.findtext("{*}maven-checkstyle-plugin.version"))
+ completed = subprocess.run(
+ [str(REPOSITORY_ROOT / "mvnw"), "-B", "-o", "-f", str(directory /
"pom.xml"), "checkstyle:check"],
+ cwd=REPOSITORY_ROOT, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, text=True, timeout=60,
+ )
+ if completed.returncode:
+ raise AssertionError(f"Checkstyle fixture execution failed
({completed.returncode}):\n{completed.stdout[-6000:]}")
+ report = ET.parse(directory / "checkstyle-result.xml").getroot()
+ if cls.engine_version != report.get("version"):
+ raise AssertionError(f"Expected Checkstyle {cls.engine_version},
got {report.get('version')}")
+ cls.actual_diagnostics = {
+ Path(file.get("name")).resolve(): [
+ (error.get("message"), error.get("severity")) for error in
file.findall("error")
+ if error.get("message") in RULE_MESSAGES
+ ]
+ for file in report.findall("file")
+ }
+ if set(cls.fixture_paths.values()) != set(cls.actual_diagnostics):
+ raise AssertionError("Checkstyle did not report exactly the
selected fixture files")
+
+ @classmethod
+ def write_fixture(cls, directory: Path, name: str, method: str) -> None:
+ path = directory / "sources" / name / "FooTest.java"
+ path.parent.mkdir(parents=True)
+ path.write_text(f"class FooTest {{\n {method}\n}}\n",
encoding="utf-8")
+ cls.fixture_paths[name] = path.resolve()
+
+ @classmethod
+ def write_pom(cls, directory: Path, plugin_version: str) -> None:
+ project = ET.Element("project", {"xmlns":
"http://maven.apache.org/POM/4.0.0"})
+ for name, value in (("modelVersion", "4.0.0"), ("groupId",
"org.apache.shardingsphere"),
+ ("artifactId", "checkstyle-rule-fixtures"),
("version", "1"), ("packaging", "pom")):
+ ET.SubElement(project, name).text = value
+ properties = ET.SubElement(project, "properties")
+ ET.SubElement(properties, "project.build.sourceEncoding").text =
"UTF-8"
+ build = ET.SubElement(project, "build")
+ ET.SubElement(build, "sourceDirectory").text = str(directory /
"sources")
+ plugin = ET.SubElement(ET.SubElement(build, "plugins"), "plugin")
+ for name, value in (("groupId", "org.apache.maven.plugins"),
("artifactId", "maven-checkstyle-plugin"), ("version", plugin_version)):
+ ET.SubElement(plugin, name).text = value
+ dependency = ET.SubElement(ET.SubElement(plugin, "dependencies"),
"dependency")
+ for name, value in (("groupId", "com.puppycrawl.tools"),
("artifactId", "checkstyle"), ("version", cls.engine_version)):
+ ET.SubElement(dependency, name).text = value
+ configuration = ET.SubElement(plugin, "configuration")
+ for name, value in (
+ ("configLocation", str(REPOSITORY_ROOT / "src" / "resources" /
"checkstyle.xml")),
+ ("outputFile", str(directory / "checkstyle-result.xml")),
("cacheFile", str(directory / "checkstyle-cache")),
+ ("includeResources", "false"), ("includeTestResources", "false"),
("failOnViolation", "false"), ("failsOnError", "false"),
+ ):
+ ET.SubElement(configuration, name).text = value
+ ET.ElementTree(project).write(directory / "pom.xml", encoding="utf-8",
xml_declaration=True)
+
+ def test_boolean_assertions_use_boolean_helpers(self) -> None:
+ for name, (_, violates) in self.boolean_cases.items():
+ with self.subTest(name=name):
+ expected = [(BOOLEAN_MESSAGE, "error")] if violates else []
+ self.assertEqual(expected,
self.actual_diagnostics[self.fixture_paths[f"boolean_{name}"]])
+
+ def test_parameterized_tests_use_argument_display_names(self) -> None:
+ for name, (_, violates) in self.display_name_cases.items():
+ with self.subTest(name=name):
+ expected = [(DISPLAY_NAME_MESSAGE, "warning")] if violates
else []
+ self.assertEqual(expected,
self.actual_diagnostics[self.fixture_paths[f"display_{name}"]])
+
+ def test_parameterized_tests_start_with_a_named_string(self) -> None:
+ for name, (_, violates) in self.name_parameter_cases.items():
+ with self.subTest(name=name):
+ expected = [(NAME_PARAMETER_MESSAGE, "warning")] if violates
else []
+ self.assertEqual(expected,
self.actual_diagnostics[self.fixture_paths[f"parameter_{name}"]])
+ self.assertEqual([],
self.actual_diagnostics[self.fixture_paths["ordinary_helper"]])
+
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/.codex/skills/gen-ut/SKILL.md b/.codex/skills/gen-ut/SKILL.md
index ce2afee0135..677ce89a839 100644
--- a/.codex/skills/gen-ut/SKILL.md
+++ b/.codex/skills/gen-ut/SKILL.md
@@ -44,10 +44,7 @@ Reading either Skill does not expand the user-authorized
scope, file types, Git
### R2: Test form and naming
-- Use JUnit 5 `@Test` for standalone scenarios.
-- Use `@ParameterizedTest(name = "{0}")`, `@MethodSource`, and `Arguments` for
data-driven scenarios. Declare `final String name` first and provide at least
three rows.
-- Do not use `@RepeatedTest`, `Consumer` as scenario transport, `switch`
dispatch inside parameterized tests, or new nested transport types.
-- Follow repository test naming. A single test uniquely covering one public
method is `assert<MethodName>`.
+Apply the Unit Testing Standards in
[CODE_OF_CONDUCT.md](../../../CODE_OF_CONDUCT.md#unit-testing-standards) to
test forms, naming, parameter sources, scenario dispatch, and nested types.
### R3: Change boundary
@@ -74,7 +71,6 @@ Reading either Skill does not expand the user-authorized
scope, file types, Git
- Exercise interface default methods with Mockito `CALLS_REAL_METHODS`.
- Obtain SPI targets through the applicable project loader and keep the
resolved instance as a test-class field by default. Record a concrete reason
before bypassing the loader.
-- Do not add tests for `getType`, `getOrder`, or `getTypeClass` unless
explicitly requested.
- Mock heavy dependencies and collaborator-owned decisions. Allow real simple
values and explicitly scoped integration, contract, or E2E behavior.
### R7: Related tests
@@ -86,11 +82,10 @@ Create a new exact-name test class only when none exists.
For each target public method, record `R8-CANDIDATES` with the method,
candidate count, decision, and evidence. A candidate is high-fit only when all
are true:
-1. target method and branch skeleton are consistent;
+1. target method is the same;
2. differences are mainly input data;
-3. assertion skeleton is consistent or differences are explicitly declared;
-4. at least three scenarios exist;
-5. no `switch` dispatch is required.
+3. the same form of assertion is used;
+4. the group meets the minimum scenario count in `CODE_OF_CONDUCT.md`.
Collaborator-owned differences are not high-fit. Refactor high-fit candidates;
otherwise record the concrete reason.
Use `KEEP` only when a genuinely high-fit refactor would materially reduce
readability or diagnosis.
@@ -138,22 +133,23 @@ Mark `R4=N/A (triggered by R12)` and retain the coverage
command and report path
### R14: Boolean assertions
-Use `assertTrue`/`assertFalse` for literal or constant expectations and
`assertThat(actual, is(expected))` for variable expectations.
-Do not use boolean `assertEquals`, boolean literals inside Hamcrest `is`, or
control flow whose only purpose is selecting `assertTrue` versus `assertFalse`.
+Apply the Boolean assertion standard in `CODE_OF_CONDUCT.md` to literal,
constant, and variable expectations.
### R15: Delivery gates
- `R15-A`: complete the semantic `R8` decision; the scanner cannot infer
high-fit.
-- `R15-B`: confirm any metadata-accessor candidate was explicitly requested;
the scanner only reports likely candidates.
+- `R15-B`: review every test against the test-value rule in
`CODE_OF_CONDUCT.md`; method names do not decide whether a test is permitted.
- `R15-C`: compare the final worktree with `Task scope baseline`; any
candidate-relevant out-of-scope mutation fails. Ignore only reproducible,
untracked or Git-ignored verification outputs.
-- `R15-D`: every parameterized test uses `@MethodSource` with at least three
`Arguments` rows. Inspect external providers manually.
-- `R15-E`: the first parameter is exactly `final String name`.
-- `R15-F`: parameterized bodies contain no `switch`.
-- `R15-G`: parameterized test changes add no nested transport type.
- Inspect each new nested-type candidate's purpose; a confirmed non-transport
type is permitted, a transport type is prohibited, and unresolved purpose keeps
the task at `R10-D`.
-- `R15-H`: boolean assertions obey `R14` without assertion-dispatch control
flow.
-- `R15-I`: parameterized signatures and provider rows contain no `Consumer`;
inspect external providers manually.
+- `R15-D`: verify each parameterized test's argument source and actual case
count against `CODE_OF_CONDUCT.md`; inspect external, unresolved, or dynamic
sources manually.
+- `R15-E`: verify the first parameter against the parameterized-test signature
rule in `CODE_OF_CONDUCT.md`.
+- `R15-F`: inspect each `switch` candidate in a parameterized test against the
grouping rule in `CODE_OF_CONDUCT.md`.
+- `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`.
+- `R15-H`: review Boolean assertions under `R14`; for assertion-dispatch
candidates in parameterized tests, compare the assertion forms against the
grouping rule in `CODE_OF_CONDUCT.md`.
+- `R15-I`: inspect callback candidates used in argument sets against the
grouping rule in `CODE_OF_CONDUCT.md`.
+ Inspect external, unresolved, or dynamic argument sources manually.
- `R15-J`: helpers and providers do not invoke target public methods. Treat
scanner results as partial evidence and inspect unresolved ownership manually.
## Workflow
@@ -165,7 +161,7 @@ Do not use boolean `assertEquals`, boolean literals inside
Hamcrest `is`, or con
5. Decide `R12`; otherwise record the SUT/collaborator ownership boundary and
`R4` branch map.
6. Review scanner candidates against every `R8` condition and record
candidate-level decisions.
7. Check `R9`, then implement the smallest tests allowed by `R2-R7`.
-8. After each coherent edit, use the lightweight precheck for repair feedback
and run invalidated focused tests.
+8. After each coherent edit, run invalidated focused tests and relevant
repository Checkstyle checks.
9. Apply `R13` and inspect semantic `R15-A/B/D/G/I/J` obligations.
10. Complete missing or invalidated final coverage, repository
formatting/checkstyle, and full mechanical checks under the [verification
commands](references/verification.md).
11. Confirm final scope against the baseline and decide `R10` once from
current evidence before delivery.
diff --git a/.codex/skills/gen-ut/references/verification.md
b/.codex/skills/gen-ut/references/verification.md
index 5d300e9b4ea..cc49675cd97 100644
--- a/.codex/skills/gen-ut/references/verification.md
+++ b/.codex/skills/gen-ut/references/verification.md
@@ -65,16 +65,17 @@ python3 .codex/skills/gen-ut/scripts/scan_quality_rules.py
--json \
<ResolvedTestFileSet>
```
+Retain the applicable repository Checkstyle report alongside the scanner
baseline; Checkstyle owns Boolean-literal assertions and parameterized-test
display-name and first-parameter syntax.
Apply `R11` to baseline diagnostics rather than treating every nonzero scanner
exit as a new blocking failure.
+New or task-relevant Checkstyle warnings block completion; only
baseline-proven unchanged, unrelated diagnostics may remain non-blocking under
`R11`.
Read coverage counters and missed branch locations from the native JaCoCo XML
and HTML reports.
## Edit-loop checks
-Run the structural precheck before an expensive build when signatures or
parameterized-test structure changed:
+When signatures or parameterized-test structure change, run the owning
module's repository Checkstyle check through the command wrapper:
```bash
-python3 .codex/skills/gen-ut/scripts/scan_quality_rules.py \
- --precheck-only <ResolvedTestFileSet>
+./mvnw -pl <ResolvedTestModule> checkstyle:check -Pcheck
```
Run focused tests after each coherent edit:
@@ -94,6 +95,11 @@ python3 .codex/skills/gen-ut/scripts/scan_quality_rules.py
--json \
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.
+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.
+These partial signals do not establish a violation.
+Review Boolean assertions with variable expectations from source because
syntax-only checks cannot infer every expression's type.
## Final coverage
diff --git a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
index 6778929658b..2649bb8d912 100644
--- a/.codex/skills/gen-ut/scripts/scan_quality_rules.py
+++ b/.codex/skills/gen-ut/scripts/scan_quality_rules.py
@@ -60,36 +60,18 @@ class FileScanContext:
added_lines: list[str]
-PRECHECK_ORDER = (
- "checkstyle-final-parameters",
- "parameterized-methodsource",
- "parameterized-name-parameter",
-)
-PRECHECK_MESSAGES = {
- "checkstyle-final-parameters": "test method parameters should be final to
avoid Checkstyle FinalParameters failures",
- "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_SPECS = (
- RuleSpec("R8", "@ParameterizedTest must use name = \"{0}\"", "automated"),
- RuleSpec("R14", "forbidden boolean assertion found", "automated"),
+ RuleSpec("R14", "boolean assertions require repository-standard source
review", "manual"),
RuleSpec("R15-A", "parameterization suitability requires semantic
evidence", "manual"),
- RuleSpec("R15-B", "metadata accessor candidates require request-scope
review", "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 @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-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"),
)
-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*,"
-
r"|assertEquals\s*\((?s:[^;]*?),\s*(?:true|false|Boolean\.TRUE|Boolean\.FALSE)\s*\)",
- re.S,
-)
CONSUMER_TOKEN_PATTERN = re.compile(r"\bConsumer\s*(?:<|\b)")
CONSTRUCTOR_CALL_PATTERN = re.compile(r"\bnew\s+(\w+)\s*\(")
METHOD_DECL_PATTERN =
re.compile(r"(?:private|protected|public)?\s*(?:static\s+)?[\w$<>\[\],
?]+\s+(\w+)\s*\([^)]*\)\s*(?:throws [^{]+)?\{", re.S)
@@ -103,15 +85,10 @@ PARAM_METHOD_PATTERN = re.compile(
re.S,
)
TEST_METHOD_DECL_PATTERN = re.compile(
-
r"((?:@Test(?:\s*\([^)]*\))?|@ParameterizedTest(?:\s*\([^)]*\))?)\s*(?:@\w+(?:\s*\([^)]*\))?\s*)*)"
+
r"((?:@Test(?:\s*\([^)]*\))?|@ParameterizedTest(?:\s*\([^)]*\))?|@RepeatedTest(?:\s*\([^)]*\))?)\s*(?:@\w+(?:\s*\([^)]*\))?\s*)*)"
r"void\s+(assert\w+)\s*\([^)]*\)\s*(?:throws [^{]+)?\{",
re.S,
)
-TEST_METHOD_SIGNATURE_PATTERN = re.compile(
-
r"((?:@Test(?:\s*\([^)]*\))?|@ParameterizedTest(?:\s*\([^)]*\))?)\s*(?:@\w+(?:\s*\([^)]*\))?\s*)*)"
- r"void\s+(assert\w+)\s*\(([^)]*)\)\s*(?:throws [^{]+)?",
- re.S,
-)
R15_A_CALL_PATTERN = re.compile(r"\b\w+\.(\w+)\s*\(")
R15_A_IGNORE = {"assertThat", "assertTrue", "assertFalse", "mock", "when",
"verify", "is", "not"}
R15_G_TYPE_DECL_PATTERN = re.compile(
@@ -127,7 +104,6 @@ R15_H_IF_RETURN_PATTERN = re.compile(
r"|if\s*\([^)]*\)\s*\{[\s\S]*?assertFalse\s*\([^;]+\)\s*;[\s\S]*?return\s*;[\s\S]*?\}\s*assertTrue\s*\([^;]+\)\s*;",
re.S,
)
-R15_NAME_PATTERN = re.compile(r'name\s*=\s*"\{0\}"')
R15_SWITCH_PATTERN = re.compile(r"\bswitch\s*\(")
TYPE_DECL_LINE_PATTERN = re.compile(
r"^\s*(?:(?:public|protected|private|static|final|abstract|sealed|non-sealed)\s+)*(class|interface|enum|record)\s+(\w+)\b"
@@ -237,38 +213,6 @@ def extract_block(text: str, brace_index: int) -> str:
return ""
-def split_parameters(params: str) -> list[str]:
- result = []
- current = []
- angle_depth = 0
- paren_depth = 0
- bracket_depth = 0
- for char in params:
- if "," == char and 0 == angle_depth and 0 == paren_depth and 0 ==
bracket_depth:
- part = "".join(current).strip()
- if part:
- result.append(part)
- current = []
- continue
- current.append(char)
- if "<" == char:
- angle_depth += 1
- elif ">" == char and angle_depth > 0:
- angle_depth -= 1
- elif "(" == char:
- paren_depth += 1
- elif ")" == char and paren_depth > 0:
- paren_depth -= 1
- elif "[" == char:
- bracket_depth += 1
- elif "]" == char and bracket_depth > 0:
- bracket_depth -= 1
- tail = "".join(current).strip()
- if tail:
- result.append(tail)
- return result
-
-
def parse_method_sources(method_name: str, annotation_block: str) -> list[str]:
result = []
matches = list(METHOD_SOURCE_PATTERN.finditer(annotation_block))
@@ -588,88 +532,7 @@ def describe_candidate(candidate: dict) -> str:
return f'{candidate["path"]}: method={candidate["method"]}
plainTestCount={candidate["plain_test_count"]}
parameterizedPresent={candidate["parameterized_present"]} decision={decision}'
-def check_parameterized_name(path: Path, source: str) -> list[str]:
- source = mask_java_non_code(source, False)
- violations = []
- token = "@ParameterizedTest"
- pos = 0
- while True:
- token_pos = source.find(token, pos)
- if token_pos < 0:
- break
- line = line_number(source, token_pos)
- cursor = token_pos + len(token)
- while cursor < len(source) and source[cursor].isspace():
- cursor += 1
- if cursor >= len(source) or "(" != source[cursor]:
- violations.append(f"{path}:{line}")
- pos = token_pos + len(token)
- continue
- depth = 1
- end = cursor + 1
- while end < len(source) and depth:
- if "(" == source[end]:
- depth += 1
- elif ")" == source[end]:
- depth -= 1
- end += 1
- if depth or not R15_NAME_PATTERN.search(source[cursor + 1:end - 1]):
- violations.append(f"{path}:{line}")
- pos = end
- return violations
-
-
-def check_r15_d(path: Path, source: str, method_bodies: dict[str, str]) ->
list[str]:
- violations = []
- for match in PARAM_METHOD_PATTERN.finditer(source):
- annotation_block = match.group(1)
- method_name = match.group(2)
- line = line_number(source, match.start())
- providers = parse_method_sources(method_name, annotation_block)
- if not providers:
- violations.append(f"{path}:{line} method={method_name} missing
@MethodSource")
- continue
- total_rows = 0
- for provider in providers:
- body = method_bodies.get(provider)
- if body is None:
- continue
- total_rows +=
len(re.findall(r"\b(?:Arguments\.of|arguments)\s*\(", body))
- if any(provider not in method_bodies for provider in providers):
- continue
- if total_rows < 3:
- violations.append(f"{path}:{line} method={method_name}
argumentsRows={total_rows}")
- return violations
-
-
-def find_unresolved_method_sources(path: Path, source: str, method_bodies:
dict[str, str]) -> list[str]:
- result = []
- for match in PARAM_METHOD_PATTERN.finditer(source):
- method_name = match.group(2)
- providers = parse_method_sources(method_name, match.group(1))
- unresolved = [each for each in providers if each not in method_bodies]
- if unresolved:
- result.append(f"{path}:{line_number(source, match.start())}
method={method_name} unresolvedProviders={','.join(unresolved)}")
- return result
-
-
-def check_r15_e(path: Path, source: str) -> list[str]:
- violations = []
- for match in PARAM_METHOD_PATTERN.finditer(source):
- method_name = match.group(2)
- params = match.group(3).strip()
- line = line_number(source, match.start())
- if not params:
- violations.append(f"{path}:{line} method={method_name}
missingParameters")
- continue
- first_param = params.split(",", 1)[0].strip()
- normalized = re.sub(r"\s+", " ", first_param)
- if "final String name" != normalized:
- violations.append(f"{path}:{line} method={method_name}
firstParam={first_param}")
- return violations
-
-
-def check_r15_f(path: Path, source: str) -> list[str]:
+def find_switch_dispatch_candidates(path: Path, source: str) -> list[str]:
violations = []
for match in PARAM_METHOD_BODY_PATTERN.finditer(source):
method_name = match.group(1)
@@ -701,7 +564,7 @@ def check_r15_g(path: Path, source: str, added_lines:
list[str]) -> list[str]:
return violations
-def check_r15_i(path: Path, source: str, method_bodies: dict[str, str]) ->
list[str]:
+def find_consumer_transport_candidates(path: Path, source: str, method_bodies:
dict[str, str]) -> list[str]:
violations = []
for match in PARAM_METHOD_PATTERN.finditer(source):
annotation_block = match.group(1)
@@ -740,12 +603,7 @@ def check_r15_j(context: FileScanContext) -> list[str]:
return violations
-def check_r14(path: Path, source: str) -> list[str]:
- source = mask_java_non_code(source, True)
- return [f"{path}:{line_number(source, match.start())}" for match in
BOOLEAN_ASSERTION_BAN_PATTERN.finditer(source)]
-
-
-def check_r15_h(path: Path, source: str) -> list[str]:
+def find_boolean_dispatch_candidates(path: Path, source: str) -> list[str]:
source = mask_java_non_code(source, True)
violations = []
for match in TEST_METHOD_DECL_PATTERN.finditer(source):
@@ -759,39 +617,6 @@ def check_r15_h(path: Path, source: str) -> list[str]:
return violations
-def find_metadata_accessor_test_candidates(path: Path, source: str) ->
list[str]:
- source = mask_java_non_code(source, False)
- result = []
- for match in TEST_METHOD_DECL_PATTERN.finditer(source):
- method_name = match.group(2)
- if any(method_name.startswith(f"assert{each}") for each in ("GetType",
"GetOrder", "GetTypeClass")):
- result.append(f"{path}:{line_number(source, match.start())}
method={method_name}")
- return result
-
-
-def checkstyle_preview_final_parameters(path: Path, source: str) -> list[str]:
- violations = []
- for match in TEST_METHOD_SIGNATURE_PATTERN.finditer(source):
- method_name = match.group(2)
- params = match.group(3).strip()
- if not params:
- continue
- line = line_number(source, match.start())
- for each in split_parameters(params):
- normalized = re.sub(r"\s+", " ", each.strip())
- if not normalized.startswith("final "):
- violations.append(f"{path}:{line} method={method_name}
param={each.strip()}")
- return violations
-
-
-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(path, source),
- "parameterized-methodsource": check_r15_d(path, source, method_bodies),
- "parameterized-name-parameter": check_r15_e(path, source),
- }
-
-
def create_file_scan_context(path: Path, scope_baseline: dict | None = None)
-> FileScanContext:
source = path.read_text(encoding="utf-8")
method_blocks = parse_method_blocks(source)
@@ -808,19 +633,6 @@ def create_file_scan_context(path: Path, scope_baseline:
dict | None = None) ->
)
-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(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))
- violations["R15-J"].extend(check_r15_j(context))
- return violations
-
-
def build_rule_result(violations_by_rule: dict[str, list[str]]) -> dict[str,
dict[str, object]]:
return {
each.name: {
@@ -832,54 +644,38 @@ 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,
precheck_only: bool = False) -> dict:
+def find_argument_source_reviews(path: Path, source: str) -> list[str]:
+ result = []
+ for match in PARAM_METHOD_PATTERN.finditer(source):
+ 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")
+ return result
+
+
+def collect_scan_result(paths: list[Path], scope_baseline_path: Path | None)
-> 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 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}
+ 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:
+ violations_by_rule["R15-J"].extend(check_r15_j(context))
violations_by_rule["R15-C"].extend(
check_scope_baseline(scope_baseline_path, paths) if
scope_baseline_path else ["missing scope baseline"]
)
- unresolved_sources = [
- each
- for context in contexts
- for each in find_unresolved_method_sources(context.path,
context.source, context.method_bodies)
- ]
return {
"rules": build_rule_result(violations_by_rule),
- "prechecks": prechecks,
"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": [each for context in contexts for each in
find_metadata_accessor_test_candidates(context.path, context.source)],
- "R15-D": unresolved_sources,
+ "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)],
- "R15-I": unresolved_sources,
+ "R15-H": [each for context in contexts for each in
find_boolean_dispatch_candidates(context.path, context.source)],
+ "R15-I": [each for context in contexts for each in
find_consumer_transport_candidates(context.path, context.source,
context.method_bodies)],
"R15-J": [str(context.path) for context in contexts if not
context.target_public_methods],
},
"java_file_count": len(contexts),
@@ -929,31 +725,12 @@ def print_summary_only(result: dict) -> int:
return 0
-def print_precheck_summary(result: dict) -> int:
- prechecks = result.get("prechecks", {})
- failed = False
- for name in PRECHECK_ORDER:
- details = prechecks.get(name)
- if not details:
- continue
- violations = details["violations"]
- if not violations:
- print(f"[precheck:{name}] ok")
- continue
- failed = True
- print(f"[precheck:{name}] {details['message']}")
- for each in violations:
- print(each)
- return 1 if failed else 0
-
-
def main() -> int:
parser = argparse.ArgumentParser(description="Mechanical quality-rule scan
and task-scope guard for gen-ut.")
parser.add_argument("--scope-baseline", help="Structured task scope
baseline created before editing.")
parser.add_argument("--capture-scope-baseline", help="Write a structured
task scope baseline to this path, then exit.")
parser.add_argument("--json", action="store_true", help="Emit JSON output
instead of the default text report.")
parser.add_argument("--summary-only", action="store_true", help="Emit a
compact text summary with candidate information.")
- parser.add_argument("--precheck-only", action="store_true", help="Emit
only lightweight deterministic prechecks for early edit loops.")
parser.add_argument("paths", nargs="*", help="Resolved test file set.")
args = parser.parse_args()
@@ -967,12 +744,10 @@ 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,
precheck_only=args.precheck_only and not args.json)
+ result = collect_scan_result(paths, baseline_path)
if args.json:
print(json.dumps(result, indent=2, sort_keys=True))
return 1 if failed_rule_names(result) else 0
- if args.precheck_only:
- return print_precheck_summary(result)
if args.summary_only:
return print_summary_only(result)
return print_rule_summary(result)
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 e7d3d61060b..cb8e6ba33a9 100644
--- a/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
+++ b/.codex/skills/gen-ut/tests/test_scan_quality_rules.py
@@ -68,9 +68,7 @@ class ScanQualityRulesTest(unittest.TestCase):
}
}
"""
- method_bodies = {name: body for name, (_, body) in
rules.parse_method_blocks(source).items()}
- self.assertEqual([], rules.check_r15_d(Path("TargetTest.java"),
source, method_bodies))
- reviews =
rules.find_unresolved_method_sources(Path("TargetTest.java"), source,
method_bodies)
+ reviews = rules.find_argument_source_reviews(Path("TargetTest.java"),
source)
self.assertEqual(1, len(reviews))
def test_nested_type_declarations_require_manual_review(self):
@@ -85,13 +83,21 @@ class ScanQualityRulesTest(unittest.TestCase):
"module/src/test/java/foo/TargetTest.java: final class
FooGroup {",
),
(
- "scenario-carrier",
+ "data-only-case",
"final FooCase value",
"target.run(value.input())",
"private record FooCase(int input, int expected) {}",
"+ private record FooCase(int input, int expected) {}",
"module/src/test/java/foo/TargetTest.java: private record
FooCase(int input, int expected) {}",
),
+ (
+ "assertion-callback-case",
+ "final FooCase value",
+ "int actual = target.run(value.input());
value.assertion().accept(actual)",
+ "private record FooCase(int input, Consumer<Integer>
assertion) {}",
+ "+ private record FooCase(int input, Consumer<Integer>
assertion) {}",
+ "module/src/test/java/foo/TargetTest.java: private record
FooCase(int input, Consumer<Integer> assertion) {}",
+ ),
)
for name, parameter, invocation, declaration, added_line,
expected_review in fixtures:
with self.subTest(name=name):
@@ -123,160 +129,117 @@ class ScanQualityRulesTest(unittest.TestCase):
with self.subTest(name=name):
self.assertEqual([],
rules.check_r15_g(Path("TargetTest.java"), candidate_source, added_lines))
- def test_ordinary_get_type_is_not_metadata_accessor_violation(self):
- source = """
+ def test_test_value_review_is_independent_of_method_name(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))
+
+ def test_boolean_dispatch_rule_reports_switch_review_candidate(self):
+ path = Path("TargetTest.java")
+ source = '''
class TargetTest {
- @Test void assertRecord() {
- assertThat(record.getType(), is(OperationType.INSERT));
+ @Test void assertRun() {
+ switch (value) {
+ case 1 -> assertTrue(target.run());
+ default -> assertFalse(target.run());
+ }
}
}
- """
- self.assertEqual([],
rules.find_metadata_accessor_test_candidates(Path("TargetTest.java"), source))
-
- def test_boolean_rule_ignores_comments_and_literals(self):
+ '''
+ 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(["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):
+ 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])
+
+ def
test_branch_and_business_callback_candidates_require_review_without_mechanical_rejection(self):
+ path = Path("TargetTest.java")
source = '''
class TargetTest {
- // assertEquals(true, target.run());
- String example = "assertThat(value, is(false))";
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("cases")
+ void assertRun(final String name, final Consumer<String>
callback) {
+ switch (name) { case "foo" -> target.run(callback);
default -> target.close(); }
+ }
}
'''
- self.assertEqual([], rules.check_r14(Path("TargetTest.java"), source))
-
- def test_boolean_rule_detects_code(self):
- source = "class TargetTest { @Test void assertRun() {
assertEquals(true, target.run()); } }"
- self.assertEqual(["TargetTest.java:1"],
rules.check_r14(Path("TargetTest.java"), source))
-
- def test_parameterized_name_rule_ignores_comment(self):
- source = "// @ParameterizedTest\nclass TargetTest {}"
- self.assertEqual([],
rules.check_parameterized_name(Path("TargetTest.java"), source))
-
- def test_boolean_dispatch_rule_detects_switch(self):
+ blocks = rules.parse_method_blocks(source)
+ context = rules.FileScanContext(path=path, source=source,
method_bodies={name: body for name, (_, body) in blocks.items()},
+ method_blocks=blocks, candidates=[],
target_type_name="Target", target_public_methods={"run", "close"},
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([], 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_repeated_test_is_an_entry_but_helper_still_fails_ownership_gate(self):
source = '''
class TargetTest {
- @Test void assertRun() {
- switch (value) {
- case 1 -> assertTrue(target.run());
- default -> assertFalse(target.run());
- }
+ @RepeatedTest(3)
+ void assertRepeatedExecution() {
+ final Target target = new Target();
+ assertTrue(target.run());
+ }
+ private boolean runHelper() {
+ final Target target = new Target();
+ return target.run();
}
}
'''
- self.assertEqual(["TargetTest.java:3 method=assertRun"],
rules.check_r15_h(Path("TargetTest.java"), source))
+ with tempfile.TemporaryDirectory() as temp_dir:
+ test_path = Path(temp_dir) /
"module/src/test/java/sample/TargetTest.java"
+ target_path = Path(temp_dir) /
"module/src/main/java/sample/Target.java"
+ test_path.parent.mkdir(parents=True)
+ target_path.parent.mkdir(parents=True)
+ test_path.write_text(source, encoding="utf-8")
+ target_path.write_text("class Target {\n public boolean run() {
return true; }\n}\n", encoding="utf-8")
+ with patch.object(rules, "check_scope_baseline", return_value=[]):
+ actual = rules.collect_scan_result([test_path], Path(temp_dir)
/ "scope.json")
+ violations = actual["rules"]["R15-J"]["violations"]
+ self.assertEqual(1, len(violations))
+ self.assertIn("method=runHelper
reason=helperInvokesTargetPublicMethod", violations[0])
def test_resolve_target_source_path_for_root_module(self):
source = "class TargetTest {}"
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):
+ def test_json_precedes_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, "argv", ["scan", "--json", "--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(["candidates", "java_file_count", "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())
@@ -287,7 +250,7 @@ class ScanQualityRulesTest(unittest.TestCase):
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, "argv", ["scan", "--capture-scope-baseline",
str(baseline), "--json", "--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,
diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md
index 9e01f2ceaa8..6b439d471bd 100644
--- a/CODE_OF_CONDUCT.md
+++ b/CODE_OF_CONDUCT.md
@@ -87,14 +87,14 @@ The following code of conduct is based on full compliance
with the [Apache Softw
- Unit tests must exercise behavior through public APIs only. Reflection-based
invocation of private members is forbidden. If tests must access fields via
reflection, use `Plugins.getMemberAccessor()` and limit reflection to `Field`
access only.
- Tests that modify static state must restore the original state after each
test.
- Obtain SPI implementations through the project loader by default. If the
class under test implements `TypedSPI` or `DatabaseTypedSPI`, instantiate it
through `TypedSPILoader` or `DatabaseTypedSPILoader`, not with `new`.
+- Do not write separate tests for methods that only return constants, directly
return fields, or simply delegate calls.
- Every unit-test class must directly test a corresponding production class
and be named `<ProductionClassName>Test`, using the exact simple name of the
production class. This class-name rule is mandatory and is independent of
scenario-focused test-method naming.
- When a production method is covered by only one test case, name that test
method `assert<MethodName>` without extra suffixes, and prefer isolating one
public production method per dedicated test method; when practical, keep test
method ordering aligned with the corresponding production methods.
-- For parameterized tests, provide display names via parameters and use
`"{0}"` as the display-name template.
- Keep test names concise and scenario-focused; avoid `ReturnsXXX` and wording
that restates the expected result instead of naming the scenario.
- Assertions must directly express the tested contract. Use `not` or
`containsString` only when the contract requires inequality or substring
matching; do not use them when an exact value or a more specific matcher is
available.
- Default to direct Mockito mocks. Use a private helper only for repeated
local setup and a standalone fixture only for a stable external or packaged
test boundary. Give fixtures the narrowest practical visibility, keep them in
the nearest owning test package or module, and do not create cross-module test
APIs for convenience. Delete or inline thin mock wrappers.
- Data assertion standards should follow:
- - Boolean type assertions should use `assertTrue` and `assertFalse`;
+ - Boolean assertions must use `assertTrue` or `assertFalse`;
- Null value assertions should use `assertNull` and `assertNotNull`;
- Non-boolean, non-null value equality assertions must use
`assertThat(actual, is(expected))`;
- Type assertions must use `assertThat(actual, isA(ExpectedType.class))`;
@@ -111,6 +111,11 @@ The following code of conduct is based on full compliance
with the [Apache Softw
- For deep chained interactions, use Mockito’s `RETURNS_DEEP_STUBS` instead of
layering intermediate mocks.
- Test data should use standardized prefixes (e.g., `foo_`/`bar_`) to clearly
identify their test purpose
- Use `PropertiesBuilder` simplify `Properties` building.
+- Parameterized tests:
+ - For parameterized tests, provide display names via parameters and use
`"{0}"` as the display-name template;
+ - Declare the first parameter of each parameterized test exactly as `final
String name`;
+ - Provide at least three meaningful argument sets for each parameterized
test;
+ - Only combine cases that invoke the same method under test and use the
same form of assertion.
## SQL Parsing Standards
diff --git
a/database/connector/dialect/mysql/src/test/java/org/apache/shardingsphere/database/connector/mysql/metadata/identifier/MySQLIdentifierCasePolicyProviderTest.java
b/database/connector/dialect/mysql/src/test/java/org/apache/shardingsphere/database/connector/mysql/metadata/identifier/MySQLIdentifierCasePolicyProviderTest.java
index 8fe438090e9..3246003cc6e 100644
---
a/database/connector/dialect/mysql/src/test/java/org/apache/shardingsphere/database/connector/mysql/metadata/identifier/MySQLIdentifierCasePolicyProviderTest.java
+++
b/database/connector/dialect/mysql/src/test/java/org/apache/shardingsphere/database/connector/mysql/metadata/identifier/MySQLIdentifierCasePolicyProviderTest.java
@@ -46,6 +46,8 @@ import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
class MySQLIdentifierCasePolicyProviderTest {
@@ -60,20 +62,20 @@ class MySQLIdentifierCasePolicyProviderTest {
IdentifierCasePolicySet actual = provider.provide(context);
assertThat(actual.getPolicy(IdentifierScope.TABLE).getLookupMode(QuoteCharacter.BACK_QUOTE),
is(expectedQuotedLookupMode));
assertThat(actual.getPolicy(IdentifierScope.TABLE).getLookupMode(QuoteCharacter.NONE),
is(expectedUnquotedLookupMode));
- assertThat(actual.getPolicy(IdentifierScope.TABLE).matches("foo",
"FOO", QuoteCharacter.NONE), is(expectedMatch));
+ assertTrue(expectedMatch ==
actual.getPolicy(IdentifierScope.TABLE).matches("foo", "FOO",
QuoteCharacter.NONE));
assertThat(actual.getPolicy(IdentifierScope.COLUMN).normalizeForDefinition("FooColumn",
QuoteCharacter.NONE), is("FooColumn"));
assertThat(actual.getPolicy(IdentifierScope.INDEX).normalizeForDefinition("FooIndex",
QuoteCharacter.NONE), is("FooIndex"));
assertThat(actual.getPolicy(IdentifierScope.CONSTRAINT).normalizeForDefinition("FooConstraint",
QuoteCharacter.NONE), is("FooConstraint"));
-
assertThat(actual.getPolicy(IdentifierScope.COLUMN).matches("foo_column",
"FOO_COLUMN", QuoteCharacter.NONE), is(Boolean.TRUE));
-
assertThat(actual.getPolicy(IdentifierScope.INDEX).matches("foo_index",
"FOO_INDEX", QuoteCharacter.NONE), is(Boolean.TRUE));
-
assertThat(actual.getPolicy(IdentifierScope.CONSTRAINT).matches("foo_constraint",
"FOO_CONSTRAINT", QuoteCharacter.NONE), is(Boolean.TRUE));
+
assertTrue(actual.getPolicy(IdentifierScope.COLUMN).matches("foo_column",
"FOO_COLUMN", QuoteCharacter.NONE));
+
assertTrue(actual.getPolicy(IdentifierScope.INDEX).matches("foo_index",
"FOO_INDEX", QuoteCharacter.NONE));
+
assertTrue(actual.getPolicy(IdentifierScope.CONSTRAINT).matches("foo_constraint",
"FOO_CONSTRAINT", QuoteCharacter.NONE));
}
@Test
void assertProvideWithQuotedTableName() {
IdentifierCasePolicy actual = provider.provide(new
IdentifierCasePolicyProviderContext(DATABASE_TYPE, new FixtureDataSource(true,
1))).getPolicy(IdentifierScope.TABLE);
assertThat(actual.getLookupMode(QuoteCharacter.BACK_QUOTE),
is(LookupMode.NORMALIZED));
- assertThat(actual.matches("t_mask", "T_MASK",
QuoteCharacter.BACK_QUOTE), is(Boolean.TRUE));
+ assertTrue(actual.matches("t_mask", "T_MASK",
QuoteCharacter.BACK_QUOTE));
}
private static Stream<Arguments> provideArguments() {
@@ -96,13 +98,13 @@ class MySQLIdentifierCasePolicyProviderTest {
void assertProvideWithLowerCaseTableNamesZeroUsesScopedPolicies() {
IdentifierCasePolicyProviderContext context = new
IdentifierCasePolicyProviderContext(DATABASE_TYPE, new FixtureDataSource(true,
0));
IdentifierCasePolicySet actual = provider.provide(context);
-
assertThat(actual.getPolicy(IdentifierScope.DATABASE).matches("foo_db",
"FOO_DB", QuoteCharacter.NONE), is(Boolean.FALSE));
-
assertThat(actual.getPolicy(IdentifierScope.SCHEMA).matches("foo_schema",
"FOO_SCHEMA", QuoteCharacter.NONE), is(Boolean.FALSE));
- assertThat(actual.getPolicy(IdentifierScope.TABLE).matches("foo_tbl",
"FOO_TBL", QuoteCharacter.NONE), is(Boolean.FALSE));
- assertThat(actual.getPolicy(IdentifierScope.VIEW).matches("foo_view",
"FOO_VIEW", QuoteCharacter.NONE), is(Boolean.FALSE));
- assertThat(actual.getPolicy(IdentifierScope.COLUMN).matches("foo_col",
"FOO_COL", QuoteCharacter.NONE), is(Boolean.TRUE));
- assertThat(actual.getPolicy(IdentifierScope.INDEX).matches("foo_idx",
"FOO_IDX", QuoteCharacter.NONE), is(Boolean.TRUE));
-
assertThat(actual.getPolicy(IdentifierScope.CONSTRAINT).matches("foo_fk",
"FOO_FK", QuoteCharacter.NONE), is(Boolean.TRUE));
+
assertFalse(actual.getPolicy(IdentifierScope.DATABASE).matches("foo_db",
"FOO_DB", QuoteCharacter.NONE));
+
assertFalse(actual.getPolicy(IdentifierScope.SCHEMA).matches("foo_schema",
"FOO_SCHEMA", QuoteCharacter.NONE));
+ assertFalse(actual.getPolicy(IdentifierScope.TABLE).matches("foo_tbl",
"FOO_TBL", QuoteCharacter.NONE));
+ assertFalse(actual.getPolicy(IdentifierScope.VIEW).matches("foo_view",
"FOO_VIEW", QuoteCharacter.NONE));
+ assertTrue(actual.getPolicy(IdentifierScope.COLUMN).matches("foo_col",
"FOO_COL", QuoteCharacter.NONE));
+ assertTrue(actual.getPolicy(IdentifierScope.INDEX).matches("foo_idx",
"FOO_IDX", QuoteCharacter.NONE));
+
assertTrue(actual.getPolicy(IdentifierScope.CONSTRAINT).matches("foo_fk",
"FOO_FK", QuoteCharacter.NONE));
}
@ParameterizedTest(name = "{0}")
diff --git a/docs/community/content/involved/conduct/code.cn.md
b/docs/community/content/involved/conduct/code.cn.md
index 24fc8329581..7b925d7c77c 100644
--- a/docs/community/content/involved/conduct/code.cn.md
+++ b/docs/community/content/involved/conduct/code.cn.md
@@ -101,14 +101,14 @@ chapter = true
- 单元测试必须通过公共 API 验证行为，禁止通过反射调用私有成员。 若测试必须通过反射访问字段，应使用
`Plugins.getMemberAccessor()`，且反射仅限于 `Field` 访问。
- 测试修改静态状态时，必须在每个测试结束后恢复其原始状态。
- 默认通过项目加载器获取 SPI 实现。 如果被测类实现了 `TypedSPI` 或 `DatabaseTypedSPI`，应通过
`TypedSPILoader` 或 `DatabaseTypedSPILoader` 实例化，禁止使用 `new`。
+ - 不得为只返回固定值、直接返回字段或简单转发调用的方法单独编写测试。
- 每个单元测试类必须直接测试一个对应的生产类，测试类必须使用生产类的准确简单类名并命名为 `<ProductionClassName>Test`。
该测试类命名规则为强制要求，与面向场景的测试方法命名规则相互独立。
- 当某个生产方法只由一个测试用例覆盖时，测试方法命名为
`assert<MethodName>`，无额外后缀；应优先使用独立测试方法覆盖单个公开的生产方法；可行时，测试方法顺序与对应的生产方法保持一致。
- - 参数化测试需通过参数提供显示名，并使用 `"{0}"` 作为展示名模板。
- 测试名称应简洁并聚焦场景；避免使用 `ReturnsXXX`，也不要使用仅复述预期结果而未说明场景的措辞。
- 断言必须直接表达被测契约。 仅当被测契约是不相等或包含指定子串时，才可使用 `not` 或
`containsString`；可断言完整值或使用更具体的 matcher 时，不得使用这两个 matcher。
- 默认直接使用 Mockito mock。 仅对重复的局部准备使用私有辅助方法，仅对稳定的外部测试边界或打包测试边界使用独立测试夹具。
测试夹具应采用实际可行的最小可见性，并放在最近的所属测试包或模块中；不要为了方便而创建跨模块测试 API。 删除或内联内容单薄的 mock 包装器。
- 数据断言规范应遵循：
- - 布尔类型断言应使用 `assertTrue` 和 `assertFalse`；
+ - 布尔断言必须使用 `assertTrue` 或 `assertFalse`；
- 空值断言应使用 `assertNull` 和 `assertNotNull`；
- 非布尔值、非空值的相等断言必须使用 `assertThat(actual, is(expected))`；
- 类型断言必须使用 `assertThat(actual, isA(ExpectedType.class))`；
@@ -125,6 +125,11 @@ chapter = true
- 深度链式交互使用 Mockito 的 `RETURNS_DEEP_STUBS`，不要层层手动 mock。
- 测试数据应使用标准化前缀（如 `foo_`/`bar_`）明确标识其测试用途。
- 使用 `PropertiesBuilder` 简化 `Properties` 构造。
+ - 参数化测试：
+ - 参数化测试需通过参数提供显示名，并使用 `"{0}"` 作为展示名模板；
+ - 参数化测试的首个参数必须声明为 `final String name`；
+ - 每个参数化测试必须提供至少三组有意义的参数数据；
+ - 只合并调用相同被测方法、使用相同断言方式的用例。
## SQL 解析规范
diff --git a/docs/community/content/involved/conduct/code.en.md
b/docs/community/content/involved/conduct/code.en.md
index b57aad7fec3..94536dcf9f8 100644
--- a/docs/community/content/involved/conduct/code.en.md
+++ b/docs/community/content/involved/conduct/code.en.md
@@ -101,14 +101,14 @@ The following code of conduct is based on full compliance
with the [Apache Softw
- Unit tests must exercise behavior through public APIs only. Reflection-based
invocation of private members is forbidden. If tests must access fields via
reflection, use `Plugins.getMemberAccessor()` and limit reflection to `Field`
access only.
- Tests that modify static state must restore the original state after each
test.
- Obtain SPI implementations through the project loader by default. If the
class under test implements `TypedSPI` or `DatabaseTypedSPI`, instantiate it
through `TypedSPILoader` or `DatabaseTypedSPILoader`, not with `new`.
+- Do not write separate tests for methods that only return constants, directly
return fields, or simply delegate calls.
- Every unit-test class must directly test a corresponding production class
and be named `<ProductionClassName>Test`, using the exact simple name of the
production class. This class-name rule is mandatory and is independent of
scenario-focused test-method naming.
- When a production method is covered by only one test case, name that test
method `assert<MethodName>` without extra suffixes, and prefer isolating one
public production method per dedicated test method; when practical, keep test
method ordering aligned with the corresponding production methods.
-- For parameterized tests, provide display names via parameters and use
`"{0}"` as the display-name template.
- Keep test names concise and scenario-focused; avoid `ReturnsXXX` and wording
that restates the expected result instead of naming the scenario.
- Assertions must directly express the tested contract. Use `not` or
`containsString` only when the contract requires inequality or substring
matching; do not use them when an exact value or a more specific matcher is
available.
- Default to direct Mockito mocks. Use a private helper only for repeated
local setup and a standalone fixture only for a stable external or packaged
test boundary. Give fixtures the narrowest practical visibility, keep them in
the nearest owning test package or module, and do not create cross-module test
APIs for convenience. Delete or inline thin mock wrappers.
- Data assertion standards should follow:
- - Boolean type assertions should use `assertTrue` and `assertFalse`;
+ - Boolean assertions must use `assertTrue` or `assertFalse`;
- Null value assertions should use `assertNull` and `assertNotNull`;
- Non-boolean, non-null value equality assertions must use
`assertThat(actual, is(expected))`;
- Type assertions must use `assertThat(actual, isA(ExpectedType.class))`;
@@ -125,6 +125,11 @@ The following code of conduct is based on full compliance
with the [Apache Softw
- For deep chained interactions, use Mockito’s `RETURNS_DEEP_STUBS` instead of
layering intermediate mocks.
- Test data should use standardized prefixes (e.g., `foo_`/`bar_`) to clearly
identify their test purpose
- Use `PropertiesBuilder` simplify `Properties` building.
+- Parameterized tests:
+ - For parameterized tests, provide display names via parameters and use
`"{0}"` as the display-name template;
+ - Declare the first parameter of each parameterized test exactly as `final
String name`;
+ - Provide at least three meaningful argument sets for each parameterized
test;
+ - Only combine cases that invoke the same method under test and use the
same form of assertion.
## SQL Parsing Standards
diff --git a/src/resources/checkstyle.xml b/src/resources/checkstyle.xml
index 0ac86a91a85..35c8ad78bd3 100644
--- a/src/resources/checkstyle.xml
+++ b/src/resources/checkstyle.xml
@@ -125,7 +125,7 @@
<property name="message" value="Consider using special escape
sequence instead of octal value or Unicode escaped value." />
</module>
<module name="MatchXpath">
- <property name="query"
value="//METHOD_CALL[(./IDENT[@text='assertThat'] or
./DOT/IDENT[@text='assertThat']) and
./ELIST/EXPR[last()]/METHOD_CALL[(./IDENT[@text='is'] or
./DOT/IDENT[@text='is']) and ./ELIST/EXPR/*[self::LITERAL_TRUE or
self::LITERAL_FALSE]]]" />
+ <property name="query"
value="//METHOD_CALL[(./IDENT[@text='assertThat'] or
./DOT/IDENT[@text='assertThat']) and
./ELIST/EXPR[last()]/METHOD_CALL[(./IDENT[@text='is'] or
./DOT/IDENT[@text='is']) and ./ELIST/EXPR/*[self::LITERAL_TRUE or
self::LITERAL_FALSE or self::DOT[./IDENT[@text=('TRUE', 'FALSE')] and
(./IDENT[@text='Boolean'] or ./DOT[./IDENT[@text='Boolean'] and
./DOT[./IDENT[@text='java'] and ./IDENT[@text='lang']]])]]]]" />
<message key="matchxpath.match" value="Use assertTrue/assertFalse
instead of assertThat(..., is(true/false))." />
</module>
<module name="MatchXpath">
@@ -133,6 +133,18 @@
value="//METHOD_DEF[./MODIFIERS/ANNOTATION[(./IDENT[@text='Test'] or
./DOT/IDENT[@text='Test'] or ./IDENT[@text='ParameterizedTest'] or
./DOT/IDENT[@text='ParameterizedTest'] or ./IDENT[@text='RepeatedTest'] or
./DOT/IDENT[@text='RepeatedTest'] or ./IDENT[@text='TestFactory'] or
./DOT/IDENT[@text='TestFactory'] or ./IDENT[@text='TestTemplate'] or
./DOT/IDENT[@text='TestTemplate'])] and not(./IDENT[starts-with(@text,
'assert')])]" />
<message key="matchxpath.match" value="Test method names must
start with 'assert'." />
</module>
+ <module name="MatchXpath">
+ <property name="id" value="ParameterizedTestDisplayName" />
+ <property name="severity" value="warning" />
+ <property name="query"
value="//METHOD_DEF/MODIFIERS/ANNOTATION[(./IDENT[@text='ParameterizedTest'] or
./DOT/IDENT[@text='ParameterizedTest']) and
not(./ANNOTATION_MEMBER_VALUE_PAIR[./IDENT[@text='name'] and
./EXPR/STRING_LITERAL[@text='{0}']])]" />
+ <message key="matchxpath.match" value="Parameterized tests must
use name = "{0}"." />
+ </module>
+ <module name="MatchXpath">
+ <property name="id" value="ParameterizedTestNameParameter" />
+ <property name="severity" value="warning" />
+ <property name="query"
value="//METHOD_DEF[./MODIFIERS/ANNOTATION[(./IDENT[@text='ParameterizedTest']
or
./DOT/IDENT[@text='ParameterizedTest'])]]/PARAMETERS[not(./PARAMETER_DEF[1][./MODIFIERS/FINAL
and ./IDENT[@text='name'] and not(./ELLIPSIS) and
./TYPE[not(.//ARRAY_DECLARATOR) and (./IDENT[@text='String'] or
./DOT[./IDENT[@text='String'] and ./DOT[./IDENT[@text='java'] and
./IDENT[@text='lang']]])]])]" />
+ <message key="matchxpath.match" value="Declare final String name
as the first parameter of parameterized tests." />
+ </module>
<module name="MatchXpath">
<property name="query" value="//VARIABLE_DEF[./MODIFIERS/FINAL and
(parent::FOR_INIT or parent::FOR_EACH_CLAUSE)]" />
<message key="matchxpath.match" value="Do not declare for-loop
variables as final." />
