terrymanu commented on PR #39009: URL: https://github.com/apache/shardingsphere/pull/39009#issuecomment-4886164097
### Summary **Review Result: Mergeable** Reason: This is a code-scope-only result for [PR #39009](https://github.com/apache/shardingsphere/pull/39009). The PR is a test-only helper refactor, and the latest-head diff preserves the existing mocked `getCachedConnections()` setup while keeping the same test scenarios. CI was not reviewed by request. ### Evidence - GitHub reports one changed file, and the local triple-dot diff matches it: `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/transaction/ProxyBackendTransactionManagerTest.java`. - The old helper returned a `Multimap` that each caller immediately used to stub `databaseConnectionManager.getCachedConnections()`. The new helper performs that same stub internally at `ProxyBackendTransactionManagerTest.java:369`, with equivalent call sites at `ProxyBackendTransactionManagerTest.java:121`, `ProxyBackendTransactionManagerTest.java:322`, `ProxyBackendTransactionManagerTest.java:326`, and `ProxyBackendTransactionManagerTest.java:349`. - The affected savepoint, rollback-to-savepoint, release-savepoint, commit cleanup, and rollback cleanup paths still exercise `ProxyBackendTransactionManager` through the same public methods, and the changed class passes locally. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: PR #39009 latest head `197b3d37b0ddd4f7e20b7be62f2d9c64f733fd5a`, base `master@0471919824c2e5b0c0b9048065b54080ca847dfe`, local merge-base `0471919824c2e5b0c0b9048065b54080ca847dfe`. GitHub `/pulls/39009/files` matched the local triple-dot file list: one modified test file under `proxy/backend/core`. - Not Reviewed Scope: GitHub Actions/check-runs/logs, full repository build, full module Checkstyle/Spotless, and runtime Proxy smoke were not reviewed because this review is code correctness only and the PR is test-only. - Verification: Public PR metadata/files/comments/reviews were accessible; GitHub reported 1 changed file and no public comments or reviews. Ran `git diff --name-status 0471919824c2e5b0c0b9048065b54080ca847dfe...197b3d37b0ddd4f7e20b7be62f2d9c64f733fd5a` with exit code 0 and confirmed the one-file scope. Ran `./mvnw -pl proxy/backend/core -DskipITs -Dspotless.skip=true -Dcheckstyle.skip=true -Dtest=org.apache.shardingsphere.proxy.backend.connector.jdbc.transaction.ProxyBackendTransactionManagerTest -Dsurefire.failIfNoSpecifiedTests=false test` with exit code 0: 19 tests run, 0 failures, 0 errors, 0 skipped, BUILD SUCCESS. - Release Note / User Docs: Not required; the PR only refactors a unit-test helper and does not change user-facing behavior, configuration, compatibility, diagnostics, or documentation. -- 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]
