terrymanu commented on PR #39342: URL: https://github.com/apache/shardingsphere/pull/39342#issuecomment-5183001960
### Result **Review Result: Mergeable** The PR correctly rejects unsupported `NATIVE` MCP E2E mode while preserving Docker enablement and the empty default-disabled configuration. ### Evidence - `MCPE2ECondition.isDockerEnabled()` now accepts only `DOCKER` and returns disabled for an empty configuration: [MCPE2ECondition.java](https://github.com/apache/shardingsphere/blob/81d0e92a330d1cf63ac46d11305a83738c05fcc7/test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/env/MCPE2ECondition.java#L39-L47). - MCP runnable profiles explicitly configure `DOCKER`: [pom.xml](https://github.com/apache/shardingsphere/blob/81d0e92a330d1cf63ac46d11305a83738c05fcc7/test/e2e/mcp/pom.xml#L143-L177). - The checked-in default remains empty, so tests stay disabled unless explicitly enabled: [e2e-env.properties](https://github.com/apache/shardingsphere/blob/81d0e92a330d1cf63ac46d11305a83738c05fcc7/test/e2e/mcp/src/test/resources/env/e2e-env.properties#L20-L21). - No correctness issue was found in the complete one-file PR scope. `git diff --check` passed. ### Coverage - Reviewed head: `81d0e92a330d1cf63ac46d11305a83738c05fcc7`; base: `cdc67f71f62e3fdf0373cdb5c89e642b7a308ab3`. - GitHub’s authoritative file list and local triple-dot scope both contain one file. - Covered behavior: run-type validation, empty configuration, Docker activation, `@EnabledIf` consumers, Maven profiles, and repository-wide run-type references. - Completed root-cause, blast-radius/contracts, tests/runtime, and convergence passes. - Code-scope result only; CI was not reviewed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
