Copilot commented on code in PR #9991: URL: https://github.com/apache/gravitino/pull/9991#discussion_r2801935982
########## gradle/libs.versions.toml: ########## @@ -72,7 +72,7 @@ trino = '435' scala-collection-compat = "2.7.0" scala-java-compat = "1.0.2" sqlite-jdbc = "3.42.0.0" -testcontainers = "1.20.6" +testcontainers = "2.0.3" Review Comment: This upgrade from testcontainers 1.20.6 to 2.0.3 is a major version change that introduces breaking changes. Testcontainers 2.x removed all shaded dependencies, but the codebase has multiple files that depend on the shaded awaitility library via imports like `import static org.testcontainers.shaded.org.awaitility.Awaitility.await;`. These imports exist in at least 9 files: - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/DorisContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/HiveContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/KafkaContainer.java:21 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/MySQLContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/OceanBaseContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/PostgreSQLContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/StarRocksContainer.java:22 - integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TrinoContainer.java:22 - trino-connector/integration-test/src/test/java/org/apache/gravitino/trino/connector/integration/test/TrinoConnectorIT.java:21 - iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/metrics/TestIcebergMetricsManager.java:22 To fix this breaking change, all shaded awaitility imports must be replaced with direct imports from `org.awaitility.Awaitility`, and the awaitility library (already defined in libs.versions.toml at line 94 and 309) must be added as an explicit test dependency to the affected modules' build.gradle files. ```suggestion testcontainers = "1.20.6" ``` ########## gradle/libs.versions.toml: ########## @@ -72,7 +72,7 @@ trino = '435' scala-collection-compat = "2.7.0" scala-java-compat = "1.0.2" sqlite-jdbc = "3.42.0.0" -testcontainers = "1.20.6" +testcontainers = "2.0.3" Review Comment: The PR description is incomplete. It does not explain what changes were proposed, why the changes are needed, what issue this fixes, whether there are user-facing changes, or how the patch was tested. For a version upgrade that introduces breaking changes, the description should include: 1. Why the upgrade is necessary 2. What breaking changes exist in testcontainers 2.x 3. What code changes are required to handle those breaking changes 4. Evidence that all docker tests pass after the upgrade Please complete the PR description template with this information. ########## gradle/libs.versions.toml: ########## @@ -72,7 +72,7 @@ trino = '435' scala-collection-compat = "2.7.0" scala-java-compat = "1.0.2" sqlite-jdbc = "3.42.0.0" -testcontainers = "1.20.6" +testcontainers = "2.0.3" Review Comment: Testcontainers 2.x also removed the shaded ducttape library. The codebase uses `org.rnorth.ducttape.Preconditions` in at least 9 container classes. These will fail to compile after upgrading to testcontainers 2.0.3. These imports should be replaced with Guava's `com.google.common.base.Preconditions` (which is already used throughout the codebase) or standard Java validation patterns. Since the codebase already uses Guava extensively (see imports like `com.google.common.collect.ImmutableSet`), replacing with Guava's Preconditions is the most consistent approach. ```suggestion testcontainers = "1.19.8" ``` -- 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]
