diqiu50 commented on code in PR #9752:
URL: https://github.com/apache/gravitino/pull/9752#discussion_r2710706102
##########
docs/how-to-test.md:
##########
@@ -66,6 +66,7 @@ To deploy the Gravitino server locally to run the integration
tests, follow thes
4. Use the `bash
trino-connector/integration-test/trino-test-tools/trino_test.sh` command to run
all the
Trino test sets in the
`trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets`
directory.
Specify the `--trino_worker_num` parameter to make the Trino test sets run
in a distributed environment.
+ Specify the `--trino_version_list` parameter with a comma-separated list of
target Trino versions for testing.
Review Comment:
I think we designed the tool to test only one version. If we need to test
multiple versions, we can run the tool multiple times.
Are there any practical use cases for doing this?
##########
integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TrinoITContainers.java:
##########
@@ -49,7 +49,7 @@ public class TrinoITContainers implements AutoCloseable {
}
public void launch(int gravitinoServerPort) throws Exception {
- launch(gravitinoServerPort, "hive2", false, 0);
+ launch(gravitinoServerPort, "hive2", false, 0, "435");
}
Review Comment:
Hard-coding `435` is not a good practice. We should use the default value
defined in Docker Compose.
##########
integration-test-common/docker-script/docker-compose.yaml:
##########
@@ -128,7 +128,7 @@ services:
entrypoint: /bin/bash /tmp/trino/init.sh
volumes:
- ./init/trino:/tmp/trino
- -
../../trino-connector/trino-connector/build/libs:/usr/lib/trino/plugin/gravitino
+ -
../../trino-connector/${TRINO_CONNECTOR_MODULE:-trino-connector}/build/libs:/usr/lib/trino/plugin/gravitino
extra_hosts:
Review Comment:
Can we add the parameter to specific the `TRINO_CONNECTOR_MODULE` directory
##########
integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TrinoITContainers.java:
##########
@@ -58,13 +58,32 @@ public void launch(
boolean isTrinoConnectorTest,
int trinoWorkerNum)
throws Exception {
+ launch(gravitinoServerPort, hiveRuntimeVersion, isTrinoConnectorTest,
trinoWorkerNum, "435");
+ }
+
+ public void launch(
+ int gravitinoServerPort,
+ String hiveRuntimeVersion,
+ boolean isTrinoConnectorTest,
+ int trinoWorkerNum,
+ String trinoVersion)
+ throws Exception {
shutdown();
Map<String, String> env = new HashMap<>();
env.put("TRINO_WORKER_NUM", String.valueOf(trinoWorkerNum));
env.put("GRAVITINO_SERVER_PORT", String.valueOf(gravitinoServerPort));
env.put("HIVE_RUNTIME_VERSION", hiveRuntimeVersion);
env.put("TRINO_CONNECTOR_TEST", String.valueOf(isTrinoConnectorTest));
+ if (Strings.isNotEmpty(trinoVersion)) {
+ env.put("TRINO_VERSION", trinoVersion);
+ int version = Integer.parseInt(trinoVersion);
+ if (version >= 435 && version <= 439) {
+ env.put("TRINO_CONNECTOR_MODULE", "trino-connector");
+ } else {
+ env.put("TRINO_CONNECTOR_MODULE", "trino-connector");
+ }
+ }
if (System.getProperty("gravitino.log.path") != null) {
Review Comment:
This check is not appropriate here. I think we should create a script to run
tests for a specific Trino version.
If the test fails, the error should be raised in the Trino connector.
--
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]
