terrymanu commented on PR #39023:
URL: https://github.com/apache/shardingsphere/pull/39023#issuecomment-4893112576
### Summary
**Review Result: Mergeable**
Reason: Code-scope only: the latest PR removes an unused MCP E2E fixture
field and its backing constant without changing the fixture creation path,
runtime database configuration, or remaining fixture API. CI not reviewed by
request.
### Evidence
- GitHub `/pulls/39023/files` and the local `origin/master...HEAD` diff both
show a single modified file:
`test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/runtime/ProxyEncryptWorkflowRuntimeTestSupport.java`.
- The removed `PHYSICAL_DATABASE_NAME` constant and `physicalDatabaseName`
field were only used to initialize the Lombok-generated
`getPhysicalDatabaseName()` accessor. The latest head has no remaining
`physicalDatabaseName` or `getPhysicalDatabaseName` references in
`test/e2e/mcp/src/test/java`.
- The fixture still exposes the values used by production E2E setup:
`getRuntimeDatabases()` is consumed by
`AbstractProductionProxyWorkflowE2ETest`, and `getLogicalDatabaseName()` is
still used by the workflow E2E tests.
- Local verification on the latest PR head passed:
- `./mvnw -pl test/e2e/mcp -DskipITs -Dspotless.skip=true -B -ntp
test-compile` exited 0.
- `./mvnw -pl test/e2e/mcp -DskipITs -Dspotless.skip=true -B -ntp
checkstyle:check` exited 0.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope:
`test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/runtime/ProxyEncryptWorkflowRuntimeTestSupport.java`;
latest PR head `fec352769f151e8ff94a9a47486fb6cc22b19de6`; base `master` SHA
`9d9ae38e5db275360177b9d150ccfabe35c1c6c3`; local merge-base
`9d9ae38e5db275360177b9d150ccfabe35c1c6c3`; local changed-file list matched
GitHub `/pulls/39023/files`.
- Not Reviewed Scope: GitHub Actions/check-runs, Docker-backed E2E runtime
execution, and broader MCP modules outside the single changed fixture file.
- Verification: Reviewed GitHub PR metadata, GitHub changed-file scope,
local three-dot diff, fixture call sites, and residual symbol references; ran
scoped `test-compile` and scoped Checkstyle, both passed.
- Release Note / User Docs: Not required; this is an internal test fixture
cleanup with no user-facing behavior, configuration, compatibility, or
migration impact.
--
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]
