[#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules #4628

mchades · 2024-08-22T11:04:00Z

What changes were proposed in this pull request?

move tests from module integration-test to other corresponding module
add CI for TrinoIT

Why are the changes needed?

Fix: #4633

Does this PR introduce any user-facing change?

no

How was this patch tested?

CI passed

mchades · 2024-08-22T11:53:32Z

@diqiu50 could you plz help to review the Trino part?

.github/workflows/trino-integration-test.yml

diqiu50 · 2024-08-23T04:15:06Z

.github/workflows/trino-integration-test.yml

+            trino-connector/integrate-test/build/*.log
+            trino-connector/integrate-test/build/*.tar
+            distribution/package/logs/gravitino-server.out
+            distribution/package/logs/gravitino-server.log


Is Missing the docker container's logs?

integration-test-common/docker-script/docker-compose.yaml

integration-test-common/docker-script/init/hive/init.sh

jerqi · 2024-08-26T02:57:07Z

The test of the client will depend on server. I don't think it's correct behaviour. An integration test depends on multiple catalogs. We shouldn't remove integration-test module.

mchades · 2024-08-26T03:28:05Z

The test of the client will depend on server. I don't think it's correct behaviour. An integration test depends on multiple catalogs. We shouldn't remove integration-test module.

Each catalog has its own integration test. If the server changes, we can use client integration tests to verify it. Therefore, I removed the integration-test module.

jerqi · 2024-08-26T03:32:39Z

The test of the client will depend on server. I don't think it's correct behaviour. An integration test depends on multiple catalogs. We shouldn't remove integration-test module.

Each catalog has its own integration test. If the server changes, we can use client integration tests to verify it. Therefore, I removed the integration-test module.

If we need a federated catalog test, we should put it into which catalog?

mchades · 2024-08-26T03:41:10Z

The test of the client will depend on server. I don't think it's correct behaviour. An integration test depends on multiple catalogs. We shouldn't remove integration-test module.

Each catalog has its own integration test. If the server changes, we can use client integration tests to verify it. Therefore, I removed the integration-test module.

If we need a federated catalog test, we should put it into which catalog?

It depends on your test purpose, the test can be added to the integration-test of clients/client-java or trino-connector/integration-test or web/integration-test.
For example, the current TagIT, AuditIT, and CatalogIT are in integration test of clients/client-java

jerryshao · 2024-08-27T05:44:30Z

I was thinking that there may be some APIs that only exist on the server side, and don't have client APIs, where do we put that tests?

mchades · 2024-08-27T05:58:29Z

I was thinking that there may be some APIs that only exist on the server side, and don't have client APIs, where do we put that tests?

I think the situation you mentioned can be covered by server's UT, and if necessary, integration/test directory can also be added to the server's test package

mchades · 2024-08-27T06:54:02Z

close and reopen for rerun CI

.github/workflows/backend-integration-test.yml

jerryshao · 2024-09-03T18:27:32Z

integration-test-common/docker-script/docker-compose.yaml

Shall we put these under resource folder?

I just kept it in its original position. Let me give it a try.

After trying this, I would suggest to leave it as it currently is, so that other modules can access it via a fixed path. If it is placed in a resource file, it needs to be accessed via the class path, which adds complexity when calling across modules.

settings.gradle.kts

jerryshao · 2024-09-05T16:33:10Z

.github/workflows/trino-integration-test.yml

+              - clients/filesystem-hadoop3/**
+              - clients/filesystem-hadoop3-runtime/**


The change of gvfs should not trigger the Trino tests.

jerryshao · 2024-09-05T16:33:50Z

.github/workflows/trino-integration-test.yml

+              - server/**
+              - server-common/**
+              - trino-connector/**
+              - docs/open-api/**


Also here. Can you please carefully check this to make sure only the related changes will trigger the tests.

I plan to fix them all in #4827, do you suggest doing it in this PR?

I'm fine doing it in #4827 .

jerryshao · 2024-09-05T16:41:23Z

core/src/test/java/org/apache/gravitino/storage/relational/service/TestFilesetMetaService.java

@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-package org.apache.gravitino.integration.test.store.relational.service;
+package org.apache.gravitino.storage.relational.service;


What is the purpose of changing this package name?

Because this class is actually a UT, not IT, it tests the interface of FilesetMetaService. So I changed the package name

jerryshao · 2024-09-05T16:47:05Z

web/integration-test/build.gradle.kts

@@ -0,0 +1,73 @@
+/*


For the web module, should you also do like trino module, to have two submodules under the web module?

What's your opinion on this?

yeah, it's better to follow the same specifications. I refactored the web module to the same

jerryshao · 2024-09-06T17:11:47Z

build.gradle.kts

@@ -627,7 +625,7 @@ tasks {
  }

  val compileTrinoConnector by registering {
-    dependsOn("trino-connector:copyLibs")
+    dependsOn("trino-connector:trino-connector:copyLibs")


Can you please check if the LICENSE, NOTICE, and DISCLAIMER are correctly copied to the release package for trino-connector, web, and gravitino packages? Since the trino and web package location has been changed, I guess it will be failed to locate the file, can you please check and fix?

Also check the compileDistribution and assembleDistribution, to see if they still work or not?

I have checked them all, they work well

jerryshao · 2024-09-08T17:01:52Z

There're some test errors, can you please fix them?

mchades force-pushed the seperate-trino-ci branch from fe1ac6d to 5635159 Compare August 22, 2024 11:31

mchades changed the title ~~[test] refactor integration-test module~~ [#4633] refactor(integration-test): remove integration-test module Aug 22, 2024

mchades self-assigned this Aug 22, 2024

mchades marked this pull request as ready for review August 22, 2024 11:53

mchades requested review from jerryshao, FANNG1 and yuqi1129 August 22, 2024 11:54

mchades force-pushed the seperate-trino-ci branch from 5635159 to 6a96485 Compare August 23, 2024 03:28

diqiu50 reviewed Aug 23, 2024

View reviewed changes

.github/workflows/trino-integration-test.yml Show resolved Hide resolved

diqiu50 reviewed Aug 23, 2024

View reviewed changes

integration-test-common/docker-script/docker-compose.yaml Outdated Show resolved Hide resolved

diqiu50 reviewed Aug 23, 2024

View reviewed changes

integration-test-common/docker-script/init/hive/init.sh Outdated Show resolved Hide resolved

mchades force-pushed the seperate-trino-ci branch from 6a96485 to 019dc8f Compare August 26, 2024 02:50

mchades force-pushed the seperate-trino-ci branch 6 times, most recently from c5eeb9d to 6b17a13 Compare August 26, 2024 13:03

mchades closed this Aug 27, 2024

mchades reopened this Aug 27, 2024

mchades changed the title ~~[#4633] refactor(integration-test): remove integration-test module~~ [#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules Aug 30, 2024

mchades force-pushed the seperate-trino-ci branch from a68e2ca to becb0d9 Compare August 31, 2024 06:25

mchades mentioned this pull request Aug 31, 2024

[#4811] improvement(CI): optimize CI jobs trigger conditions #4827

Merged

jerryshao reviewed Sep 3, 2024

View reviewed changes

.github/workflows/backend-integration-test.yml Outdated Show resolved Hide resolved

jerryshao reviewed Sep 3, 2024

View reviewed changes

settings.gradle.kts Outdated Show resolved Hide resolved

mchades force-pushed the seperate-trino-ci branch 4 times, most recently from a96fe1c to dbf4bf3 Compare September 5, 2024 07:24

jerryshao reviewed Sep 5, 2024

View reviewed changes

mchades force-pushed the seperate-trino-ci branch from ac4ae9b to 1d5c20b Compare September 6, 2024 08:10

jerryshao reviewed Sep 6, 2024

View reviewed changes

mchades added 7 commits September 8, 2024 14:33

remove integration-test module

48572c2

remove duplicate

56c6aba

reorganize trino connector directory structure

5fce27d

fix build

ce252de

fix trino IT

7c3a1af

fix webIT

6602044

reorganize web module directory structure

0888a0d

mchades force-pushed the seperate-trino-ci branch from 1d5c20b to 0888a0d Compare September 8, 2024 06:59

jerryshao closed this Sep 8, 2024

jerryshao reopened this Sep 8, 2024

fix PG backend IT

c9b9ffc

mchades force-pushed the seperate-trino-ci branch from 1600c3b to c9b9ffc Compare September 9, 2024 04:08

jerryshao approved these changes Sep 9, 2024

View reviewed changes

jerryshao merged commit 6c0ecf5 into apache:main Sep 9, 2024
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules #4628

[#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules #4628

mchades commented Aug 22, 2024 •

edited

Loading

mchades commented Aug 22, 2024

diqiu50 Aug 23, 2024

mchades Aug 26, 2024

jerqi commented Aug 26, 2024

mchades commented Aug 26, 2024

jerqi commented Aug 26, 2024 •

edited

Loading

mchades commented Aug 26, 2024

jerryshao commented Aug 27, 2024

mchades commented Aug 27, 2024

mchades commented Aug 27, 2024

jerryshao Sep 3, 2024

mchades Sep 4, 2024

mchades Sep 4, 2024

jerryshao Sep 5, 2024

jerryshao Sep 5, 2024

mchades Sep 6, 2024

jerryshao Sep 6, 2024

jerryshao Sep 5, 2024

mchades Sep 6, 2024

jerryshao Sep 5, 2024

jerryshao Sep 6, 2024

mchades Sep 6, 2024

jerryshao Sep 6, 2024 •

edited

Loading

jerryshao Sep 6, 2024

mchades Sep 8, 2024 •

edited

Loading

jerryshao commented Sep 8, 2024

		- clients/filesystem-hadoop3/**
		- clients/filesystem-hadoop3-runtime/**

[#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules #4628

[#4633] refactor(integration-test): move the tests in integration-test module into their corresponding modules #4628

Conversation

mchades commented Aug 22, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

mchades commented Aug 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerqi commented Aug 26, 2024

mchades commented Aug 26, 2024

jerqi commented Aug 26, 2024 • edited Loading

mchades commented Aug 26, 2024

jerryshao commented Aug 27, 2024

mchades commented Aug 27, 2024

mchades commented Aug 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryshao Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mchades Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

jerryshao commented Sep 8, 2024

mchades commented Aug 22, 2024 •

edited

Loading

jerqi commented Aug 26, 2024 •

edited

Loading

jerryshao Sep 6, 2024 •

edited

Loading

mchades Sep 8, 2024 •

edited

Loading