terrymanu commented on PR #40049: URL: https://github.com/apache/shardingsphere/pull/40049#issuecomment-5978996130
### Result **Review Result: Mergeable** Local implementation and verification are complete. The 13 resulting files establish accurate UT/IT execution boundaries and reduce test log noise. No production code, APIs, SPIs, or dependencies were changed. ### Evidence - Classification: Driver and Native use the existing Failsafe setup through their module POMs. DistSQL and Prometheus were renamed to IT classes; the two SQLBuilder classes became UTs. DatabaseTimestampService now separates the mocked failure UT from the H2/Hikari success IT. The default install lifecycle is preserved. The GraalVM workflow only adds test/native/pom.xml to its path filters. - Regression coverage: Each of the 145 methods was discovered once. The bodies of 135 methods remain unchanged; the other 10 only strengthen log assertions or relocate the timestamp success scenario. Hashes for 115 Driver/Native source and resource files remain unchanged. The three isolated DistSQL executor UTs are preserved. - Logging: infra/common/src/test/resources/logback-test.xml:26 only disables duplicate propagation to root; existing levels remain unchanged. The 16 Agent tests retain exception, return-value, and invocation-order assertions. Nine expected JUL records are precisely verified and no longer printed. BaseTransactionTestCase contains only four INFO→DEBUG changes. TransactionBaseE2EIT:148–179 retains scenario, test-class, and exception diagnostics. - Independent defensive-code audit passed: Both AdviceExecutorTest classes remove their handlers, restore the original useParentHandlers state, and replay unconsumed records. Records are consumed only after every assertion succeeds, preserving diagnostics on failure. DatabaseTimestampServiceIT:43 closes only the independent pool created by DataSourcePoolCreator.create(Properties). No shared resource is closed, and no unnecessary SQLException handling remains. - The old ShardingSphereDriverPostgreSQLTest and its YAML remain unchanged. Existing real PostgreSQL ITs have not been proven equivalent to the H2 PostgreSQL MODE schema/table/index/COUNT workflow. H2 dialect and database-type identification remain unchanged. ### Coverage - Code Correctness Review of a standalone local candidate: original task baseline 07048c78058799d16453be0c1be197e60b5c527c plus the task-owned working-tree changes. Current HEAD is ffbc9bc46c69dea54a186b8b9b171b2b44945c9d. All 17 Git paths were reviewed, including the old and new paths for four renames. Unrelated concurrent changes and commits were excluded. - Eight affected modules: jdbc, kernel/time-service/type/database, agent/core, agent/plugins/metrics/type/prometheus, kernel/data-pipeline/dialect/postgresql, test/e2e/driver, test/native, and test/e2e/operation/transaction. - Running `-DskipITs -Dspotless.skip=true clean install` against that module set passed 1,496 UTs. All migrated ITs were skipped, while the seven SQLBuilder UTs ran. Running `-Dspotless.skip=true clean install` passed 1,496 UTs and 104 ITs, with 23 existing skips explicitly recorded. Migrated scenarios were not executed twice. - infra/common passed 1,203 UTs with `-DskipITs install`. After the final changes, all 24 targeted UTs passed. Real MySQL and PostgreSQL JDBC ClassicTransfer transaction tests passed. The Proxy-only TCL scenario was not executed or counted as passing. - Exact-file Spotless and Checkstyle checks returned exit=0, with file selection verified. Spotless covered nine Java files, two POMs, and one workflow; Checkstyle covered nine Java files and one logback XML. Other files were not applicable under the existing configuration. The root command `./mvnw -o -B -ntp apache-rat:check -Pcheck -T1C` returned exit=0. - Native effective POM and test selection were checked. The JVM stage of `-PnativeTestInShardingSphere -DskipNativeTests=true clean verify` discovered 24 candidates: seven passed and 17 retained their existing skip conditions. Duplicate execution was automatically skipped. The environment lacks native-image, so native-image compilation/execution and generateMetadata were not verified as passing. - References to the four old class names were removed. The final scope audit found no production changes, new classification framework, conditional skip annotations, or global logging shutdown. No unresolved review findings remain. -- 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]
