This is an automated email from the ASF dual-hosted git repository.
zhangliang 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 5b3c7df25d2 Add more test cases on
DatabaseProxyBackendHandlerFactoryTest (#37981)
5b3c7df25d2 is described below
commit 5b3c7df25d2c5c320bcb3151ceca1362a2467a74
Author: Liang Zhang <[email protected]>
AuthorDate: Sun Feb 8 11:02:16 2026 +0800
Add more test cases on DatabaseProxyBackendHandlerFactoryTest (#37981)
---
.codex/skills/gen-ut/SKILL.md | 40 ++++++++---
AGENTS.md | 3 +-
.../DatabaseProxyBackendHandlerFactoryTest.java | 82 +++++++++++++++++-----
.../variable/SetDistVariableExecutorTest.java | 3 +-
4 files changed, 98 insertions(+), 30 deletions(-)
diff --git a/.codex/skills/gen-ut/SKILL.md b/.codex/skills/gen-ut/SKILL.md
index 83805e26257..4372cc39cf7 100644
--- a/.codex/skills/gen-ut/SKILL.md
+++ b/.codex/skills/gen-ut/SKILL.md
@@ -23,6 +23,7 @@ Handling missing input:
Test class placeholder convention:
- `<ResolvedTestClass>` can be one fully qualified test class or a
comma-separated list.
+- `<ResolvedTestFileSet>` is the concrete test file path list resolved from
`<ResolvedTestClass>` (space-separated in shell commands).
## Mandatory Constraints (Single Source of Rules)
@@ -57,6 +58,21 @@ Test class placeholder convention:
- after test implementation (early fail-fast gate);
- before final delivery (final release gate).
- If any match is found, task state is "not complete" until all violations
are fixed and scan is rerun clean.
+- `R14`: Change scope must be strictly limited to tests resolved from input
target classes.
+ - Allowed edit files: only `<ResolvedTestClass>` mapped source files under
`src/test/java` or `src/test/resources`.
+ - Forbidden: modifying any other test file to fix unrelated build/lint/gate
failures.
+ - Exception: only if the user explicitly approves scope expansion in the
current turn.
+- `R15`: No meaningless test code; each line must have direct necessity for
the scenario.
+ - Do not add redundant mocks/stubs/assertions that do not change branch
selection, object behavior, or verification result.
+ - Prefer Mockito default return values unless explicit stubbing is required
by the scenario.
+ - If a line is only cosmetic and removing it does not affect scenario setup
or assertions, remove it.
+- `R16`: Inline single-use local variables in tests.
+ - If a local variable is used exactly once, inline it at the call site.
+ - Exception: keep the variable only when it is required for additional
stubbing/verification or shared assertions.
+- `R17`: Test-method necessity hard gate; no coverage-equivalent duplicates.
+ - Each test method must add at least one unique value: either an uncovered
branch/path from `R4/R5`, or a distinct externally observable behavior
assertion not already covered.
+ - If removing a test method keeps target-class line/branch coverage
unchanged and does not lose unique behavior verification, remove that method.
+ - For factory/route fallback scenarios, keep only one representative method
per branch outcome unless the user explicitly requests an extra regression
guard.
## Execution Boundary
@@ -64,6 +80,7 @@ Test class placeholder convention:
- Do not edit generated directories (such as `target/`).
- Do not use destructive git operations (such as `git reset --hard`, `git
checkout --`).
- If module name is missing, module-less command templates are allowed; if
module is specified, module-scoped commands are mandatory.
+- If failures come from out-of-scope test files under `R14`, report blockers
and wait for user decision instead of editing those files.
## Workflow
@@ -72,11 +89,12 @@ Test class placeholder convention:
3. Output branch-path inventory according to `R4`.
4. Output branch-to-test mapping according to `R5`.
5. Perform dead-code analysis according to `R8` and record findings.
-6. Implement or extend tests according to `R2/R3/R6/R7/R10/R12/R13`.
-7. Run the first `R13` hard-gate scan (early fail-fast) and fix all hits.
-8. Run verification commands and iterate.
-9. Run the second `R13` hard-gate scan (final release gate) and ensure clean.
-10. Deliver results using the output structure.
+6. Implement or extend tests according to
`R2/R3/R6/R7/R10/R12/R13/R14/R15/R16/R17`.
+7. Run necessity self-check according to `R15/R16/R17` and remove redundant
mocks/stubs/assertions/single-use locals/coverage-equivalent test methods.
+8. Run the first `R13` hard-gate scan (early fail-fast) and fix all hits.
+9. Run verification commands and iterate.
+10. Run the second `R13` hard-gate scan (final release gate) and ensure clean.
+11. Deliver results using the output structure.
## Verification and Commands
@@ -97,9 +115,9 @@ With module input:
./mvnw -pl <module> -am -Pcheck checkstyle:check -DskipTests
```
-4. `R13` hard-gate scan (must be clean, run in step 7 and step 9):
+4. `R13` hard-gate scan (must be clean, run in step 8 and step 10):
```bash
-bash -lc 'if rg -n
"assertThat\\s*\\(.*is\\s*\\(\\s*(true|false)\\s*\\)\\s*\\)|assertEquals\\s*\\(\\s*(true|false)\\s*,"
<module>/src/test/java; then echo "[R13] forbidden boolean assertion found";
exit 1; fi'
+bash -lc 'if rg -n
"assertThat\\s*\\(.*is\\s*\\(\\s*(true|false)\\s*\\)\\s*\\)|assertEquals\\s*\\(\\s*(true|false)\\s*,"
<ResolvedTestFileSet>; then echo "[R13] forbidden boolean assertion found";
exit 1; fi'
```
Without module input:
@@ -119,9 +137,9 @@ Without module input:
./mvnw -Pcheck checkstyle:check -DskipTests
```
-4. `R13` hard-gate scan (must be clean, run in step 7 and step 9):
+4. `R13` hard-gate scan (must be clean, run in step 8 and step 10):
```bash
-bash -lc 'if rg -n
"assertThat\\s*\\(.*is\\s*\\(\\s*(true|false)\\s*\\)\\s*\\)|assertEquals\\s*\\(\\s*(true|false)\\s*,"
. --glob "**/src/test/java/**"; then echo "[R13] forbidden boolean assertion
found"; exit 1; fi'
+bash -lc 'if rg -n
"assertThat\\s*\\(.*is\\s*\\(\\s*(true|false)\\s*\\)\\s*\\)|assertEquals\\s*\\(\\s*(true|false)\\s*,"
<ResolvedTestFileSet>; then echo "[R13] forbidden boolean assertion found";
exit 1; fi'
```
Command execution rules:
@@ -133,7 +151,7 @@ Command execution rules:
Follow this order:
-1. Goal and constraints (mapped to `R1-R13`)
+1. Goal and constraints (mapped to `R1-R17`)
2. Plan and implementation (including branch mapping result)
3. Dead-code and coverage results (according to `R8/R9`)
4. Verification commands and exit codes
@@ -143,7 +161,7 @@ Follow this order:
## Quality Self-Check
- Rule definitions must exist only in "Mandatory Constraints"; other sections
should reference rule IDs only.
-- Final state must satisfy `R9`, and all applicable rules (including `R10`,
`R11`, `R12`, and `R13`) must be met, with complete command and exit-code
records.
+- Final state must satisfy `R9`, and all applicable rules (including `R10`,
`R11`, `R12`, `R13`, `R14`, `R15`, `R16`, and `R17`) must be met, with complete
command and exit-code records.
- `R13` command is mandatory evidence; missing `R13` command record or
non-clean scan means not complete.
## Maintenance Rules
diff --git a/AGENTS.md b/AGENTS.md
index e37047451fa..2543f5d07b6 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -20,6 +20,7 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
- Use clear naming and reasonable abstractions.
- Delete unused code; when changing functionality, remove legacy
compatibility shims.
- Keep variable declarations adjacent to first use; if a value must be
retained, declare it `final` to satisfy Checkstyle
VariableDeclarationUsageDistance.
+ - Single-use local variables must be inlined by default; keep a local
variable only when it is reused (for stubbing/verification/assertions) or
materially improves readability.
- **Complete Implementation**: no MVPs/placeholders/TODOs—deliver fully
runnable solutions.
### Performance Standards
@@ -88,7 +89,7 @@ Dangerous operation detected! Operation type: [specific
action] Scope of impact:
- **Pre-task checklist (do before planning/coding):** re-read AGENTS.md and
`CODE_OF_CONDUCT.md`; restate user goal, constraints, forbidden tools/APIs,
coverage expectations, sandbox/network/approval limits; prefer
`rg`/`./mvnw`/`apply_patch`; avoid destructive commands (`git reset --hard`,
`git checkout --`, bulk deletes) and generated paths like `target/`.
- **Risk gate:** if any action fits the Dangerous Operation Checklist, pause
and use the confirmation template before proceeding.
- **Planning rules:** use Sequential Thinking with 3-10 actionable steps (no
single-step plans) via the plan tool for non-trivial tasks; convert all hard
requirements (SPI usage, mocking rules, coverage/test naming, forbidden APIs)
into a checklist inside the plan and do not code until each item is addressed
or explicitly waived.
-- **Execution discipline:** inspect existing code before edits; keep changes
minimal; default to mocks and SPI loaders; keep variable declarations near
first use and mark retained values `final`; delete dead code and avoid
placeholders/TODOs.
+- **Execution discipline:** inspect existing code before edits; keep changes
minimal; default to mocks and SPI loaders; keep variable declarations near
first use and mark retained values `final`; inline single-use locals by default
unless reuse/readability justifies retention; delete dead code and avoid
placeholders/TODOs.
- **Post-task self-check (before replying):** confirm all instructions were
honored; verify no placeholders/unused code; ensure Checkstyle/Spotless gates
for touched modules are satisfied or explain why not run and what to run; list
commands with exit codes; call out risks and follow-ups.
- **Final response template:** include intent/why, changed files with paths,
rationale per file/section, commands run (with exit codes), verification
status, and remaining risks/next actions (if tests skipped, state reason and
the exact command to run).
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/DatabaseProxyBackendHandlerFactoryTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/DatabaseProxyBackendHandlerFactoryTest.java
index 44d8deb589c..1361735eb6d 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/DatabaseProxyBackendHandlerFactoryTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/DatabaseProxyBackendHandlerFactoryTest.java
@@ -17,24 +17,26 @@
package org.apache.shardingsphere.proxy.backend.handler.data;
-import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
import
org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
-import
org.apache.shardingsphere.infra.binder.context.statement.type.CommonSQLStatementContext;
import org.apache.shardingsphere.infra.hint.HintValueContext;
import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
import
org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.session.connection.ConnectionContext;
import org.apache.shardingsphere.infra.session.query.QueryContext;
-import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.apache.shardingsphere.mode.manager.ContextManager;
import
org.apache.shardingsphere.proxy.backend.connector.ProxyDatabaseConnectionManager;
import
org.apache.shardingsphere.proxy.backend.connector.StandardDatabaseProxyConnector;
import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
import
org.apache.shardingsphere.proxy.backend.handler.data.type.UnicastDatabaseProxyBackendHandler;
+import
org.apache.shardingsphere.proxy.backend.response.header.update.UpdateResponseHeader;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
import
org.apache.shardingsphere.sql.parser.statement.core.statement.SQLStatement;
+import
org.apache.shardingsphere.sql.parser.statement.core.statement.attribute.type.DatabaseSelectRequiredSQLStatementAttribute;
import
org.apache.shardingsphere.sql.parser.statement.core.statement.type.dal.DALStatement;
+import
org.apache.shardingsphere.sql.parser.statement.core.statement.type.dal.SetStatement;
+import
org.apache.shardingsphere.sql.parser.statement.core.statement.type.dml.DoStatement;
import
org.apache.shardingsphere.sql.parser.statement.core.statement.type.dml.SelectStatement;
+import
org.apache.shardingsphere.sql.parser.statement.core.segment.generic.table.TableSegment;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.AutoMockExtension;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.ConstructionMockSettings;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.StaticMockSettings;
@@ -43,10 +45,12 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
+import java.sql.SQLException;
import java.util.Collections;
import java.util.Optional;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.isA;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
@@ -58,8 +62,6 @@ import static org.mockito.Mockito.when;
@MockitoSettings(strictness = Strictness.LENIENT)
class DatabaseProxyBackendHandlerFactoryTest {
- private final DatabaseType databaseType =
TypedSPILoader.getService(DatabaseType.class, "FIXTURE");
-
@Test
void assertNewInstanceReturnedUnicastDatabaseProxyBackendHandlerWithDAL() {
String sql = "DESC tbl";
@@ -71,6 +73,41 @@ class DatabaseProxyBackendHandlerFactoryTest {
assertThat(actual, isA(UnicastDatabaseProxyBackendHandler.class));
}
+ @Test
+ void assertNewInstanceReturnedUpdateResponseHeaderWithSetAndNoDatabase()
throws SQLException {
+ String sql = "SET autocommit=1";
+ SetStatement setStatement = mock(SetStatement.class);
+ SQLStatementContext sqlStatementContext =
mock(SQLStatementContext.class, RETURNS_DEEP_STUBS);
+ when(sqlStatementContext.getSqlStatement()).thenReturn(setStatement);
+ DatabaseProxyBackendHandler actual =
DatabaseProxyBackendHandlerFactory.newInstance(
+ new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock()), mock(ConnectionSession.class), false);
+ UpdateResponseHeader actualResponseHeader = (UpdateResponseHeader)
actual.execute();
+ assertThat(actualResponseHeader, isA(UpdateResponseHeader.class));
+ assertThat(actualResponseHeader.getSqlStatement(), is(setStatement));
+ }
+
+ @Test
+ void
assertNewInstanceReturnedUnicastDatabaseProxyBackendHandlerWithSetAndDatabaseSelected()
{
+ String sql = "SET autocommit=1";
+ SetStatement setStatement = mock(SetStatement.class,
RETURNS_DEEP_STUBS);
+
when(setStatement.getAttributes().findAttribute(DatabaseSelectRequiredSQLStatementAttribute.class)).thenReturn(Optional.empty());
+ SQLStatementContext sqlStatementContext =
mock(SQLStatementContext.class, RETURNS_DEEP_STUBS);
+ when(sqlStatementContext.getSqlStatement()).thenReturn(setStatement);
+ DatabaseProxyBackendHandler actual =
DatabaseProxyBackendHandlerFactory.newInstance(
+ new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock()), mockConnectionSession(), false);
+ assertThat(actual, isA(UnicastDatabaseProxyBackendHandler.class));
+ }
+
+ @Test
+ void assertNewInstanceReturnedUnicastDatabaseProxyBackendHandlerWithDo() {
+ String sql = "DO 1";
+ SQLStatementContext sqlStatementContext =
mock(SQLStatementContext.class, RETURNS_DEEP_STUBS);
+
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DoStatement.class));
+ DatabaseProxyBackendHandler actual =
DatabaseProxyBackendHandlerFactory.newInstance(
+ new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock()), mock(), false);
+ assertThat(actual, isA(UnicastDatabaseProxyBackendHandler.class));
+ }
+
private ConnectionContext mockConnectionContext() {
ConnectionContext result = mock(ConnectionContext.class);
when(result.getCurrentDatabaseName()).thenReturn(Optional.of("foo_db"));
@@ -80,31 +117,42 @@ class DatabaseProxyBackendHandlerFactoryTest {
@Test
void
assertNewInstanceReturnedUnicastDatabaseProxyBackendHandlerWithQueryWithoutFrom()
{
String sql = "SELECT 1";
- SelectStatement selectStatement = new SelectStatement(databaseType);
- selectStatement.buildAttributes();
- SQLStatementContext sqlStatementContext = new
CommonSQLStatementContext(selectStatement);
+ SelectStatement selectStatement = mock(SelectStatement.class);
+ when(selectStatement.getFrom()).thenReturn(Optional.empty());
+ SQLStatementContext sqlStatementContext =
mock(SQLStatementContext.class, RETURNS_DEEP_STUBS);
+
when(sqlStatementContext.getSqlStatement()).thenReturn(selectStatement);
DatabaseProxyBackendHandler actual =
DatabaseProxyBackendHandlerFactory.newInstance(
new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock()), mock(), false);
assertThat(actual, isA(UnicastDatabaseProxyBackendHandler.class));
}
@Test
- void assertNewInstanceReturnedSchemaAssignedDatabaseProxyBackendHandler() {
- String sql = "SELECT 1 FROM user WHERE id = 1";
- SQLStatementContext sqlStatementContext = mockSQLStatementContext();
+ void
assertNewInstanceReturnedStandardDatabaseProxyConnectorWithDALRequiringDatabase()
{
+ DALStatement dalStatement = mock(DALStatement.class,
RETURNS_DEEP_STUBS);
+
when(dalStatement.getAttributes().findAttribute(DatabaseSelectRequiredSQLStatementAttribute.class)).thenReturn(Optional.of(mock(DatabaseSelectRequiredSQLStatementAttribute.class)));
+ assertNewInstanceReturnedStandardDatabaseProxyConnector(dalStatement,
"SHOW PROCESSLIST");
+ }
+
+ @Test
+ void
assertNewInstanceReturnedStandardDatabaseProxyConnectorWithSelectFrom() {
+ SelectStatement selectStatement = mock(SelectStatement.class);
+
when(selectStatement.getFrom()).thenReturn(Optional.of(mock(TableSegment.class)));
+
assertNewInstanceReturnedStandardDatabaseProxyConnector(selectStatement,
"SELECT 1 FROM t_order");
+ }
+
+ private void assertNewInstanceReturnedStandardDatabaseProxyConnector(final
SQLStatement sqlStatement, final String sql) {
+ SQLStatementContext sqlStatementContext =
mockSQLStatementContext(sqlStatement);
ConnectionSession connectionSession = mockConnectionSession();
ContextManager contextManager = mockContextManager();
when(ProxyContext.getInstance().getContextManager()).thenReturn(contextManager);
- DatabaseProxyBackendHandler actual =
- DatabaseProxyBackendHandlerFactory.newInstance(
- new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock(ShardingSphereMetaData.class)),
- connectionSession, false);
+ DatabaseProxyBackendHandler actual =
DatabaseProxyBackendHandlerFactory.newInstance(
+ new QueryContext(sqlStatementContext, sql,
Collections.emptyList(), new HintValueContext(), mockConnectionContext(),
mock(ShardingSphereMetaData.class)), connectionSession, false);
assertThat(actual, isA(StandardDatabaseProxyConnector.class));
}
- private SQLStatementContext mockSQLStatementContext() {
+ private SQLStatementContext mockSQLStatementContext(final SQLStatement
sqlStatement) {
SQLStatementContext result = mock(SQLStatementContext.class,
RETURNS_DEEP_STUBS);
- when(result.getSqlStatement()).thenReturn(mock(SQLStatement.class));
+ when(result.getSqlStatement()).thenReturn(sqlStatement);
when(result.getTablesContext().getSchemaNames()).thenReturn(Collections.emptyList());
when(result.getTablesContext().getDatabaseName()).thenReturn(Optional.empty());
return result;
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/variable/SetDistVariableExecutorTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/variable/SetDistVariableExecutorTest.java
index 00875272eb7..00e982adfae 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/variable/SetDistVariableExecutorTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/variable/SetDistVariableExecutorTest.java
@@ -47,6 +47,7 @@ import java.util.Properties;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
@@ -69,7 +70,7 @@ class SetDistVariableExecutorTest {
SetDistVariableStatement statement = new
SetDistVariableStatement("proxy_meta_data_collector_enabled", "false");
ContextManager contextManager = mockContextManager();
executor.executeUpdate(statement, contextManager);
-
assertThat(contextManager.getMetaDataContexts().getMetaData().getTemporaryProps().getValue(TemporaryConfigurationPropertyKey.PROXY_META_DATA_COLLECTOR_ENABLED),
is(false));
+ assertFalse((Boolean)
contextManager.getMetaDataContexts().getMetaData().getTemporaryProps().getValue(TemporaryConfigurationPropertyKey.PROXY_META_DATA_COLLECTOR_ENABLED));
}
@Test
