Refactor ClickHouse's nativeTest#35476
Merged
Merged
Conversation
6227767 to
5465f08
Compare
linghengqian
commented
May 22, 2025
Member
Author
linghengqian
left a comment
There was a problem hiding this comment.
- The Error Log is simply unbelievable.
./mvnw -PgenerateMetadata -e -T 1C clean testdoes not have this situation.
aused by: org.apache.maven.plugin.MojoExecutionException: Missing jar-file for org.apache.shardingsphere:shardingsphere-proxy-dialect-postgresql:jar:5.5.3-SNAPSHOT:test. Ensure that native-maven-plugin runs in package phase.
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.processArtifact (AbstractNativeImageMojo.java:302)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.processSupportedArtifacts (AbstractNativeImageMojo.java:285)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.addArtifactToClasspath (AbstractNativeImageMojo.java:314)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.addDependenciesToClasspath (AbstractNativeImageMojo.java:366)
at org.graalvm.buildtools.maven.NativeTestMojo.addDependenciesToClasspath (NativeTestMojo.java:122)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.populateClasspath (AbstractNativeImageMojo.java:423)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.getClasspath (AbstractNativeImageMojo.java:430)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.getBuildArgs (AbstractNativeImageMojo.java:201)
at org.graalvm.buildtools.maven.AbstractNativeImageMojo.buildImage (AbstractNativeImageMojo.java:447)
at org.graalvm.buildtools.maven.NativeTestMojo.execute (NativeTestMojo.java:169)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
at jdk.internal.reflect.DirectMethodHandleAccessor.invoke (DirectMethodHandleAccessor.java:103)
at java.lang.reflect.Method.invoke (Method.java:580)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)There was a problem hiding this comment.
Pull Request Overview
This pull request refactors ClickHouse's nativeTest setup by migrating to a Testcontainers-based JDBC driver and updating dependency versions. Key changes include:
- Replacing the old ClickHouse JDBC driver with org.testcontainers.jdbc.ContainerDatabaseDriver in the YAML configuration.
- Updating the test class (ClickHouseTest) to leverage the new Testcontainers driver and cleaning up container resources with ContainerDatabaseDriver.killContainers().
- Bumping dependency versions for Testcontainers and GraalVM Native Build Tools and updating related documentation.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/native/src/test/resources/test-native/yaml/jdbc/databases/clickhouse.yaml | Updated JDBC configuration to use Testcontainers driver and specified TC_INITSCRIPT. |
| test/native/src/test/resources/test-native/sql/clickhouse-init.sql | Added SQL initialization for ClickHouse with table definitions and truncation commands. |
| test/native/src/test/resources/META-INF/services/org.apache.shardingsphere.infra.database.core.type.DatabaseType | Registered the Testcontainers ClickHouse database type. |
| test/native/src/test/java/org/apache/shardingsphere/test/natived/jdbc/databases/ClickHouseTest.java | Refactored test logic including removal of hardcoded container management and use of ContainerDatabaseDriver.killContainers(). |
| test/native/src/test/java/org/apache/shardingsphere/test/natived/commons/algorithm/testcontainers/TcClickhouseDatabaseType.java | Introduced the dedicated Testcontainers ClickHouse database type implementation. |
| pom.xml and test/native/pom.xml | Updated dependency versions for Testcontainers and native-maven-plugin. |
| docs/* | Updated documentation to reflect the new ClickHouse image version and dependency upgrades. |
Comments suppressed due to low confidence (1)
test/native/src/test/java/org/apache/shardingsphere/test/natived/jdbc/databases/ClickHouseTest.java:52
- Consider verifying that calling ContainerDatabaseDriver.killContainers() in the afterEach hook effectively cleans up all Testcontainers resources and does not interfere with parallel test execution.
ContainerDatabaseDriver.killContainers();
4912b14 to
91e8805
Compare
sandynz
approved these changes
May 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For #29052.
Changes proposed in this pull request:
clickhouse/clickhouse-serveras Docker Image in ClickHouseProvider testcontainers/testcontainers-java#8738 , refactored to simplify nativeTest for clickhouse integration, which obviously helps to remove boilerplate code that is not convenient to understand.org.apache.shardingsphere:shardingsphere-infra-database-hiveno longer needs to be introduced separately.org.graalvm.buildtools:native-maven-plugin:0.10.6does not recognizepom.xmlwithout source code, which affects allorg.apache.shardingsphere:shardingsphere-proxy-dialect-*. See Native Build Tools cannot recognize Maven modules that only havepom.xmlbut no source code graalvm/native-build-tools#727 .Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.