[#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse #6160

diqiu50 · 2025-01-09T03:39:38Z

What changes were proposed in this pull request?

Add integration test framework of gvfs-fuse
Integrate LocalStack into the gvfs-fuse integration test
Add ci pipeline for integration test

Why are the changes needed?

Fix: #6131

Does this PR introduce any user-facing change?

No

How was this patch tested?

IT

FANNG1 · 2025-01-09T13:54:30Z

.github/workflows/gvfs-fuse-build-test.yml

        run: |
           ./gradlew :clients:filesystem-fuse:build -PenableFuse=true

+      - name: Integration test
+        run: |
+          ./gradlew bundles:aws-bundle:build -x :clients:client-python:build compileDistribution -x test -x web -PjdkVersion=${{ matrix.java-version }}


it's hard to use, use make prepare-it-env ?

It's not good to let make call gradle again.
Currently, the integration test module does not automatically build the package, right?

yes, the current IT doesn't automatically build the package, but here you depends on bundles:aws-bundle:build, I wonder whether you could make it auto dependents. and There are separate actions in Gravitino IT, seems better make rust client keep consistent with it.

- name: Package Gravitino if: ${{ inputs.test-mode == 'deploy' }} run: | ./gradlew compileDistribution -x test -PjdkVersion=${{ inputs.java-version }}

FANNG1 · 2025-01-09T13:55:52Z

clients/filesystem-fuse/src/default_raw_filesystem.rs

@@ -334,13 +334,21 @@ impl<T: PathFileSystem> RawFileSystem for DefaultRawFileSystem<T> {
        file.flush().await
    }

-    async fn close_file(&self, _file_id: u64, fh: u64) -> Result<()> {
+    async fn close_file(&self, file_id: u64, fh: u64) -> Result<()> {
+        let file_entry = self.get_file_entry(file_id).await;


why adding this?

Check if the file has been deleted.

If there is no file entry, is it expected not closing the open file?

Yes, closing deleted file may cause bugs.
This fix is not good. Add the todo comments to handle that case.

clients/filesystem-fuse/src/gravitino_client.rs

clients/filesystem-fuse/Makefile

clients/filesystem-fuse/src/gravitino_fileset_filesystem.rs

clients/filesystem-fuse/src/open_dal_filesystem.rs

FANNG1 · 2025-01-10T09:15:42Z

clients/filesystem-fuse/src/open_dal_filesystem.rs

+
+    #[tokio::test]
+    async fn s3_ut_test_s3_stat() {
+        test_enable_with!(RUN_TEST_WITH_S3);


this seems hacks, could you refer other rust projects for more general way to control the tests?

I can’t find a good way to mark the test tags and run them.

try using this?
#[cfg(feature = "ENABLE_S3_TEST")], after using this macro, seems we could remove s3_ut prefix too.

This is generally used to control whether a module participates in compilation, and I haven't seen any projects using it to control tests. People familiar with Rust might find this confusing, right?

It also cannot filter out tests that do not have feature = "ENABLE_S3_TEST" enabled.

There have a lib can mark tags for the testers. but it can not work with #[tokio::test]
https://docs.rs/tagrs/latest/tagrs/

please refer to https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/tests/file_io_s3_test.rs

The storage-s3 feature is used to enable that functionality for all code, not just for testing.

clients/filesystem-fuse/tests/fuse_test.rs

diqiu50 · 2025-01-13T08:43:07Z

@yuqi1129 @mchades @xunliu Please take a review.

.github/workflows/gvfs-fuse-build-test.yml

yuqi1129 · 2025-01-13T09:35:07Z

.github/workflows/gvfs-fuse-build-test.yml

+          ./gradlew build -x :clients:client-python:build -x test -x web -PjdkVersion=${{ matrix.java-version }}
+          ./gradlew compileDistribution -x :clients:client-python:build -x test -x web -PjdkVersion=${{ matrix.java-version }}


I believe move this two line to the Build and test Gvfs-fuse may be more proper

The one above builds gvfs-fuse, and the below builds the Gravitino server needed for testing.

FANNG1 · 2025-01-14T06:50:45Z

LGTM, @yuqi1129 any other comments?

yuqi1129 · 2025-01-14T06:55:12Z

LGTM, @yuqi1129 any other comments?

I have reviewed again and no further comments.

…#6160) ### What changes were proposed in this pull request? Add integration test framework of gvfs-fuse Integrate LocalStack into the gvfs-fuse integration test Add ci pipeline for integration test ### Why are the changes needed? Fix: #6131 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? IT

diqiu50 added 17 commits January 6, 2025 11:44

Add testers for fileset fs

9810afd

Add integration test script

4350b43

Add integration testers script

81bbf63

Fix

6b68650

Fix

922ed0a

Fix

3435810

Fix

4ad62ed

Update test script

7900190

Add integration test

4e34bd2

fix some logs

1f014f6

Test s3

0960c08

Update

8f9bea8

Update

536ff25

refector s3 testcases

52649d7

Fix

00bb21f

Integration with localstack

6c29810

Fix

7a32f7f

diqiu50 changed the title ~~[#6131] feat (gvfs-fuse): Integrate with localstack of gvfs-fuse integration test~~ [#6131] feat (gvfs-fuse): Integrate localstack of gvfs-fuse integration test Jan 9, 2025

diqiu50 changed the title ~~[#6131] feat (gvfs-fuse): Integrate localstack of gvfs-fuse integration test~~ [#6131] feat (gvfs-fuse): Integrate LocalStack into the GVFS-FUSE integration test Jan 9, 2025

diqiu50 changed the title ~~[#6131] feat (gvfs-fuse): Integrate LocalStack into the GVFS-FUSE integration test~~ [#6131] feat (gvfs-fuse): Integrate LocalStack into the gvfs-fuse integration test Jan 9, 2025

diqiu50 self-assigned this Jan 9, 2025

diqiu50 mentioned this pull request Jan 9, 2025

[#6131] feat (gvfs-fuse): Add integration fileset filesystem operation in gvfs fuse #6132

Closed

jerryshao requested a review from FANNG1 January 9, 2025 06:06

diqiu50 added 4 commits January 9, 2025 15:28

Add pipline

eb23de6

Fix

5ed1a21

Add logs

14cfdc9

Fix tty error

078a2e9

diqiu50 changed the title ~~[#6131] feat (gvfs-fuse): Integrate LocalStack into the gvfs-fuse integration test~~ [#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse Jan 9, 2025

diqiu50 requested a review from xunliu January 9, 2025 08:18

Fix ci error

e278ccf

FANNG1 reviewed Jan 9, 2025

View reviewed changes

diqiu50 added 2 commits January 10, 2025 10:08

Fix

629d449

Fix

ed3d1b4

diqiu50 requested a review from FANNG1 January 10, 2025 02:24

jerryshao added the branch-0.8 Automatically cherry-pick commit to branch-0.8 label Jan 10, 2025

diqiu50 added 3 commits January 10, 2025 16:16

Fix

df7b0da

Fix

1907c9e

Fix

1b8a949

FANNG1 reviewed Jan 10, 2025

View reviewed changes

clients/filesystem-fuse/src/gravitino_fileset_filesystem.rs Show resolved Hide resolved

FANNG1 reviewed Jan 10, 2025

View reviewed changes

clients/filesystem-fuse/tests/fuse_test.rs Show resolved Hide resolved

Fix

016aec2

jerryshao requested a review from FANNG1 January 13, 2025 06:51

diqiu50 requested a review from mchades January 13, 2025 08:17

yuqi1129 reviewed Jan 14, 2025

View reviewed changes

diqiu50 force-pushed the gvfs-pr8 branch from 07b38b2 to 016aec2 Compare January 14, 2025 02:20

Update

df64efd

diqiu50 requested a review from yuqi1129 January 14, 2025 06:11

FANNG1 approved these changes Jan 14, 2025

View reviewed changes

FANNG1 merged commit c6476b8 into apache:main Jan 14, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse #6160

[#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse #6160

diqiu50 commented Jan 9, 2025 •

edited

Loading

FANNG1 Jan 9, 2025

diqiu50 Jan 10, 2025

FANNG1 Jan 10, 2025

diqiu50 Jan 10, 2025

FANNG1 Jan 9, 2025

diqiu50 Jan 10, 2025

FANNG1 Jan 10, 2025

diqiu50 Jan 10, 2025

FANNG1 Jan 10, 2025

diqiu50 Jan 10, 2025

FANNG1 Jan 13, 2025 •

edited

Loading

diqiu50 Jan 13, 2025 •

edited

Loading

diqiu50 Jan 13, 2025

FANNG1 Jan 14, 2025

diqiu50 Jan 14, 2025 •

edited

Loading

diqiu50 commented Jan 13, 2025

yuqi1129 Jan 13, 2025

diqiu50 Jan 14, 2025

yuqi1129 Jan 14, 2025

FANNG1 commented Jan 14, 2025

yuqi1129 commented Jan 14, 2025

		./gradlew build -x :clients:client-python:build -x test -x web -PjdkVersion=${{ matrix.java-version }}
		./gradlew compileDistribution -x :clients:client-python:build -x test -x web -PjdkVersion=${{ matrix.java-version }}

[#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse #6160

[#6131] feat (gvfs-fuse): Add integration test framework of gvfs-fuse #6160

Conversation

diqiu50 commented Jan 9, 2025 • 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?

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

FANNG1 Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

diqiu50 Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diqiu50 Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

diqiu50 commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FANNG1 commented Jan 14, 2025

yuqi1129 commented Jan 14, 2025

diqiu50 commented Jan 9, 2025 •

edited

Loading

FANNG1 Jan 13, 2025 •

edited

Loading

diqiu50 Jan 13, 2025 •

edited

Loading

diqiu50 Jan 14, 2025 •

edited

Loading