terrymanu commented on PR #38992:
URL: https://github.com/apache/shardingsphere/pull/38992#issuecomment-4881643665
### Summary
**Review Result: Mergeable**
Reason: This is code-scope ready for the selected Code Correctness Review.
The PR only adds focused unit coverage for `StatementManager`, the tests
exercise the relevant statement creation, fallback, generated-key, cache
reuse/invalidation, and close paths, and the focused verification passed. CI
not reviewed by request.
### Evidence
- The authoritative GitHub file list matches the local triple-dot scope: one
added file,
`jdbc/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/StatementManagerTest.java`.
- The new tests cover the existing `StatementManager` paths for:
- direct `createStatement(...)` success, `SQLFeatureNotSupportedException`
fallback, and ordinary `SQLException` propagation;
- `ExecutionUnit` prepared-statement creation, generated-key options, and
prepared-statement fallback;
- cached statement reuse and stale cache replacement when the cached
connection or statement is closed;
- `close()` cleanup, including the exception path.
- Focused JaCoCo evidence for
`org.apache.shardingsphere.driver.jdbc.core.statement.StatementManager` from
the PR head:
- CLASS: covered=1 missed=0 ratio=100.00%
- LINE: covered=31 missed=0 ratio=100.00%
- BRANCH: covered=12 missed=0 ratio=100.00%
- No GitHub issue comments, review comments, or previous review rounds were
present for this PR at the time of review.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: `jdbc` test scope; latest PR head
`af3d2f6fa43ec22646df54ff8eb882ec95cd734b`; base `master` at
`513e9b9f01703231e8f0dd19827fc6285d82701d`; local merge-base
`513e9b9f01703231e8f0dd19827fc6285d82701d`; local changed-file list matched
GitHub `/pulls/38992/files`.
- Not Reviewed Scope: GitHub Actions/check-runs/CI logs, because this was
not a Mergeability Review or CI Review; broader JDBC integration behavior
outside the added unit-test scope.
- Verification:
- `./mvnw -pl jdbc -DskipITs -Dspotless.skip=true
-Dtest=StatementManagerTest -DfailIfNoTests=true
-Dsurefire.failIfNoSpecifiedTests=false test` exited 0.
- `./mvnw -pl jdbc -Pcheck checkstyle:check -DskipTests` exited 0.
- `./mvnw -pl jdbc -Pcheck spotless:check -DskipTests` exited 0.
- `./mvnw apache-rat:check -Pcheck -T1C` exited 0.
- `./mvnw -pl jdbc -DskipITs -Dsurefire.useManifestOnlyJar=false
-Dtest=StatementManagerTest -DfailIfNoTests=true
-Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=false
-Djacoco.append=false test jacoco:report` exited 0.
- Release Note / User Docs: Not required; this PR is test-only and does not
change user-facing behavior, configuration, compatibility, diagnostics, or
release artifacts.
--
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]
