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 4531dc5efa5 Fix MCP coverage gate input and close approved coverage
gaps (#40282)
4531dc5efa5 is described below
commit 4531dc5efa5af330ee6b9f078dffa075908c5389
Author: Liang Zhang <[email protected]>
AuthorDate: Tue Oct 20 10:02:41 2026 +0800
Fix MCP coverage gate input and close approved coverage gaps (#40282)
* Fix MCP coverage gate input and close approved coverage gaps
* Remove dedicated MCP coverage check
---
.github/workflows/nightly-check.yml | 2 --
.../mcp/core/tool/handler/ToolDefinitionRegistry.java | 2 --
.../handler/execute/MCPSQLExecutionFacadeTest.java | 19 +++++++++++++------
.../ReadwriteSplittingRuleWorkflowRequestTest.java | 14 ++++++++++++++
.../ReadwriteSplittingStatusWorkflowRequestTest.java | 6 ++++++
.../tool/handler/ShardingPlanningRequestBinder.java | 15 ++++-----------
.../handler/ShardingPlanningRequestBinderTest.java | 6 ++++++
.../support/workflow/model/ClarifiedIntentTest.java | 7 +++++++
8 files changed, 50 insertions(+), 21 deletions(-)
diff --git a/.github/workflows/nightly-check.yml
b/.github/workflows/nightly-check.yml
index a3cfafff770..4796cf0c2a8 100644
--- a/.github/workflows/nightly-check.yml
+++ b/.github/workflows/nightly-check.yml
@@ -192,8 +192,6 @@ jobs:
with:
name: jacoco-report
path: ${{ github.workspace }}/target/mergeReport/jacoco
- - name: Verify MCP Coverage
- run: ./mvnw -f mcp/pom.xml jacoco:check@jacoco-check -Pcoverage-check
-Djacoco.skip=false
-Djacoco.check.class.pattern='org.apache.shardingsphere.mcp.*'
check-and-report-sonarcloud:
if: ${{ !cancelled() && github.repository == 'apache/shardingsphere' &&
needs.check-and-report-jacoco.outputs.report-outcome == 'success' }}
diff --git
a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/ToolDefinitionRegistry.java
b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/ToolDefinitionRegistry.java
index d9b8b652f53..ea8cecb3049 100644
---
a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/ToolDefinitionRegistry.java
+++
b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/ToolDefinitionRegistry.java
@@ -68,8 +68,6 @@ public final class ToolDefinitionRegistry {
private static MCPToolDefinition createToolDefinition(final String
toolName, final MCPToolHandler<?> handler) {
MCPToolDescriptor descriptor =
MCPDescriptorCatalogIndex.getRequiredToolDescriptor(toolName);
- ShardingSpherePreconditions.checkNotNull(descriptor.getAnnotations(),
- () -> new IllegalArgumentException(String.format("Tool `%s`
MCP annotations are required for `%s`.", toolName,
handler.getClass().getName())));
return new MCPToolDefinition(descriptor, handler);
}
diff --git
a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPSQLExecutionFacadeTest.java
b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPSQLExecutionFacadeTest.java
index b56a102e8ec..9a13010b26c 100644
---
a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPSQLExecutionFacadeTest.java
+++
b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/MCPSQLExecutionFacadeTest.java
@@ -948,7 +948,7 @@ class MCPSQLExecutionFacadeTest {
@ParameterizedTest(name = "{0}")
@MethodSource("assertExecuteWithQualifiedCurrentDatabaseCases")
- void assertExecuteWithQualifiedCurrentDatabase(final String name, final
String sql, final SupportedMCPStatement supportedStatement) {
+ void assertExecuteWithQualifiedCurrentDatabase(final String name, final
String sql, final SupportedMCPStatement supportedStatement, final String
expectedStatementType) {
MCPDatabaseCapabilityProvider capabilityProvider =
mock(MCPDatabaseCapabilityProvider.class);
MCPSessionExecutionCoordinator coordinator =
mock(MCPSessionExecutionCoordinator.class);
MCPJdbcStatementExecutor statementExecutor =
mock(MCPJdbcStatementExecutor.class);
@@ -961,8 +961,12 @@ class MCPSQLExecutionFacadeTest {
MCPJdbcTransactionStatementExecutor transactionExecutor =
mock(MCPJdbcTransactionStatementExecutor.class);
MCPSQLExecutionFacade facade = createFacade(capabilityProvider,
coordinator, transactionExecutor, statementExecutor);
SQLExecutionResult actual = facade.execute(request);
- assertThat(actual, is(expectedResult));
- verify(statementExecutor).execute(eq(request), any(), eq(capability));
+ assertThat(actual, sameInstance(expectedResult));
+ ArgumentCaptor<ClassificationResult> actualClassification =
ArgumentCaptor.forClass(ClassificationResult.class);
+ verify(statementExecutor).execute(eq(request),
actualClassification.capture(), eq(capability));
+ assertThat(actualClassification.getValue().getStatementClass(),
is(supportedStatement));
+ assertThat(actualClassification.getValue().getStatementType(),
is(expectedStatementType));
+ assertThat(actualClassification.getValue().getNormalizedSql(),
is(sql));
verifyNoInteractions(transactionExecutor);
}
@@ -1165,9 +1169,12 @@ class MCPSQLExecutionFacadeTest {
private static Stream<Arguments>
assertExecuteWithQualifiedCurrentDatabaseCases() {
return Stream.of(
- Arguments.of("table", "SELECT * FROM logic_db.orders",
SupportedMCPStatement.QUERY),
- Arguments.of("function", "SELECT
logic_db.foo_refresh_orders()", SupportedMCPStatement.QUERY),
- Arguments.of("grant", "GRANT SELECT ON logic_db.orders TO
PUBLIC", SupportedMCPStatement.DCL));
+ Arguments.of("table", "SELECT * FROM logic_db.orders",
SupportedMCPStatement.QUERY, "SELECT"),
+ Arguments.of("function", "SELECT
logic_db.foo_refresh_orders()", SupportedMCPStatement.QUERY, "SELECT"),
+ Arguments.of("grant", "GRANT SELECT ON logic_db.orders TO
PUBLIC", SupportedMCPStatement.DCL, "GRANT"),
+ Arguments.of("create function", "CREATE FUNCTION
logic_db.foo_function() RETURNS INT RETURN 1", SupportedMCPStatement.DDL,
"CREATE"),
+ Arguments.of("create procedure", "CREATE PROCEDURE
logic_db.foo_procedure() SELECT 1", SupportedMCPStatement.DDL, "CREATE"),
+ Arguments.of("create trigger", "CREATE TRIGGER
logic_db.foo_trigger BEFORE INSERT ON logic_db.foo_table FOR EACH ROW SET
@foo_value=1", SupportedMCPStatement.DDL, "CREATE"));
}
private static Stream<Arguments>
assertExecuteExplainWithSyntaxFailureCases() {
diff --git
a/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingRuleWorkflowRequestTest.java
b/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingRuleWorkflowRequestTest.java
index 771bcee4958..5d1c97b0214 100644
---
a/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingRuleWorkflowRequestTest.java
+++
b/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingRuleWorkflowRequestTest.java
@@ -25,6 +25,7 @@ import java.util.Map;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertNull;
class ReadwriteSplittingRuleWorkflowRequestTest {
@@ -35,6 +36,14 @@ class ReadwriteSplittingRuleWorkflowRequestTest {
assertThat(actual.getReadStorageUnits(), is(List.of("read_ds_0",
"read_ds_1")));
}
+ @Test
+ void assertSetReadStorageUnitsWithNullString() {
+ ReadwriteSplittingRuleWorkflowRequest actual = new
ReadwriteSplittingRuleWorkflowRequest();
+ actual.setReadStorageUnits(List.of("foo_read"));
+ actual.setReadStorageUnits((String) null);
+ assertThat(actual.getReadStorageUnits(), is(List.of("foo_read")));
+ }
+
@Test
void assertCopy() {
ReadwriteSplittingRuleWorkflowRequest actual = createRequest().copy();
@@ -59,6 +68,11 @@ class ReadwriteSplittingRuleWorkflowRequestTest {
assertThat(actual.getDatabase(), is("logic_db"));
}
+ @Test
+ void assertMergeWithNullRequests() {
+ assertNull(ReadwriteSplittingRuleWorkflowRequest.merge(null, null));
+ }
+
@Test
void assertSetReadStorageUnitsWithMissingListValues() {
ReadwriteSplittingRuleWorkflowRequest request = new
ReadwriteSplittingRuleWorkflowRequest();
diff --git
a/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingStatusWorkflowRequestTest.java
b/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingStatusWorkflowRequestTest.java
index 784d746af99..2d7d308bc0a 100644
---
a/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingStatusWorkflowRequestTest.java
+++
b/mcp/features/readwrite-splitting/src/test/java/org/apache/shardingsphere/mcp/feature/readwritesplitting/tool/model/ReadwriteSplittingStatusWorkflowRequestTest.java
@@ -22,6 +22,7 @@ import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertNull;
class ReadwriteSplittingStatusWorkflowRequestTest {
@@ -36,6 +37,11 @@ class ReadwriteSplittingStatusWorkflowRequestTest {
assertThat(actual.getStorageUnit(), is("read_ds_0"));
}
+ @Test
+ void assertMergeWithNullRequests() {
+ assertNull(ReadwriteSplittingStatusWorkflowRequest.merge(null, null));
+ }
+
@Test
void assertMergeFromBaseRequest() {
WorkflowRequest previous = new WorkflowRequest();
diff --git
a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinder.java
b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinder.java
index 701e0272d5d..f7174e2964e 100644
---
a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinder.java
+++
b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinder.java
@@ -22,7 +22,6 @@ import
org.apache.shardingsphere.mcp.support.workflow.service.WorkflowPlanningAr
import
org.apache.shardingsphere.mcp.support.workflow.service.WorkflowRequestBinder;
import java.util.Collection;
-import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
@@ -139,22 +138,16 @@ public final class ShardingPlanningRequestBinder {
workflowPlanningArguments.applyStringArgument("component_name",
request::setComponentName);
}
- private void replaceValues(final Collection<String> target, final Object
value) {
- List<String> values = createStringList(value);
+ private void replaceValues(final Collection<String> target, final String
value) {
+ Collection<String> values = createStringList(value);
if (!values.isEmpty()) {
target.clear();
target.addAll(values);
}
}
- private List<String> createStringList(final Object value) {
- if (value instanceof List) {
- return ((List<?>)
value).stream().map(String::valueOf).map(String::trim).filter(each ->
!each.isEmpty()).toList();
- }
- if (null == value) {
- return List.of();
- }
- return
Stream.of(String.valueOf(value).split(",")).map(String::trim).filter(each ->
!each.isEmpty()).toList();
+ private Collection<String> createStringList(final String value) {
+ return Stream.of(value.split(",")).map(String::trim).filter(each ->
!each.isEmpty()).toList();
}
}
diff --git
a/mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinderTest.java
b/mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinderTest.java
index de6ec9eaa6e..ba0f0f80813 100644
---
a/mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinderTest.java
+++
b/mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/handler/ShardingPlanningRequestBinderTest.java
@@ -44,6 +44,12 @@ class ShardingPlanningRequestBinderTest {
assertThat(actual.getAuditorNames(), is(List.of("dml_auditor")));
}
+ @Test
+ void assertBindTableRuleWithAuditorSeparators() {
+ ShardingWorkflowRequest actual = new
ShardingPlanningRequestBinder().bindTableRule(Map.of("auditors", " ,
foo_auditor, ,bar_auditor, "));
+ assertThat(actual.getAuditorNames(), is(List.of("foo_auditor",
"bar_auditor")));
+ }
+
@Test
void assertBindTableReferenceRule() {
ShardingWorkflowRequest actual = new
ShardingPlanningRequestBinder().bindTableReferenceRule(Map.of(
diff --git
a/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/ClarifiedIntentTest.java
b/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/ClarifiedIntentTest.java
index 02c8ceaf86d..a32d24b1e4d 100644
---
a/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/ClarifiedIntentTest.java
+++
b/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/model/ClarifiedIntentTest.java
@@ -30,4 +30,11 @@ class ClarifiedIntentTest {
actual.setOperationType(" create ");
assertThat(actual.getOperationType(), is("create"));
}
+
+ @Test
+ void assertSetOperationTypeWithNull() {
+ ClarifiedIntent actual = new ClarifiedIntent();
+ actual.setOperationType(null);
+ assertThat(actual.getOperationType(), is(""));
+ }
}
