Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test] curvefs/mds: split large test cases into focused smaller ones #2717

Merged

Conversation

ken90242
Copy link
Contributor

@ken90242 ken90242 commented Aug 25, 2023

What problem does this PR solve?

Issue Number: #2505

Problem Summary:
All the test cases in these two files are written into one test suite, and the names of the test cases have no meaningful information. Once the code is modified, it is difficult to change the corresponding unit tests accordingly.

What is changed and how it works?

What's Changed:

  1. fs_manager_test.cpp
test_success_cleanup_after_fail_create_volume_fs_on_inode_error
test_success_cleanup_after_fail_create_volume_fs_on_dentry_error
test_success_create_volume_fs
test_success_create_s3_fs
test_success_monotonically_increasing_fsid_for_multiple_fs
test_fail_create_volume_fs_on_fail_partition_creation
test_fail_create_volume_fs_on_failed_root_node_creation
test_fail_create_s3_fs_on_failed_root_node_creation
test_fail_create_duplicate_s3_fs_with_different_fsname
test_success_get_s3_fsinfo_by_fsname
test_success_get_s3_fsinfo_by_fsid
test_success_get_s3_fsinfo_by_consistent_fsid_fsname
test_success_get_s3_fsinfo_by_inconsistent_fsid_fsname
test_success_mount_s3_fs
test_fail_repeated_mount_s3_fs
test_fail_delete_s3_fs_with_existing_mount_path
test_success_umount_s3_fs_with_mount_path
test_success_delete_s3_fs_without_mount_path
test_success_create_volume_fs_when_volume_fs_exists
test_success_get_volume_fsinfo_by_fsname
test_success_get_volume_fsinfo_by_fsid
test_success_get_volume_fsinfo_by_consistent_fsname_fsid
test_fail_mount_volume_fs_on_space_creation_error
test_success_mount_volume_fs
test_fail_repeated_mount_volume_fs
test_fail_umount_volume_fs_on_failed_uninit_space
test_success_umount_volume_fs_after_uninit
test_fail_umount_volume_fs_non_existent_mount_path
test_success_delete_volume_fs_after_umount_existing_mount_path
test_success_delete_volume_fs_without_mount
test_fail_repeated_delete_volume_fs
test_success_restore_session_after_client_timeout
test_success_background_thread_lifecycle
test_success_background_thread_delete_fs
test_success_refresh_session_with_outdated_partition_txid
test_success_refresh_session_with_up_to_date_partition_txid
test_fail_get_latest_txid_without_fsid
test_success_get_latest_txid_with_fsid
  1. mds_service_test.cpp
test_fail_create_fs_missing_volume_for_volume_fstype
test_fail_create_fs_missing_s3_info_for_s3_fstype
test_fail_retrieve_fsinfo_non_existent_fsid
test_fail_create_fs_existing_volume_for_volume_fstype
test_fail_create_fs_hybrid_fstype
test_fail_get_fsinfo_missing_fsid_and_fsname
test_success_get_fsinfo_fsid_with_volume_fstype
test_success_get_fsinfo_fsid_with_s3_fstype
test_success_get_fsinfo_fsname_with_volume_fstype
test_success_get_fsinfo_fsname_with_s3_fstype
test_fail_get_fsinfo_non_existent_fsname
test_success_get_fsinfo_consistent_fsname_and_fsid
test_fail_get_fsinfo_inconsistent_fsname_and_fsid
test_success_mount_then_umount_on_single_path
test_success_delete_fs_no_mount_paths
test_fail_delete_fs_mount_paths_still_exist
test_fail_repeatedly_mount_fs_on_same_path
test_fail_repeatedly_umount_fs_on_same_path
test_success_refresh_sessions
test_success_delete_fs_after_umounting_all_paths

How it Works:
Those gigantic test cases were now divided into smaller and more focused ones.

Side effects(Breaking backward compatibility? Performance regression?):
None

Outcome

$ ./bazel-bin/curvefs/test/mds/curvefs_mds_test --gtest_filter=FSManagerTest*
image

$ ./bazel-bin/curvefs/test/mds/curvefs_mds_test --gtest_filter=MdsServiceTest*
image

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@ken90242 ken90242 force-pushed the feature/curve-fs-mds-optimize-unit-tests branch 9 times, most recently from 62fc1b9 to ba838ad Compare August 28, 2023 00:21
@ken90242 ken90242 changed the title dratft [test] curvefs/mds: split large test cases into focused smaller ones Aug 28, 2023
@ken90242
Copy link
Contributor Author

cicheck

@ken90242 ken90242 marked this pull request as ready for review August 28, 2023 00:33
@ken90242
Copy link
Contributor Author

ken90242 commented Aug 28, 2023

@wuhongsong
@wu-hanqing
I have finished the refactoring for the test cases.
Kindly take a look at this PR at your convenience.

curvefs/test/mds/fs_manager_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/fs_manager_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/fs_manager_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/fs_manager_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
@ken90242 ken90242 force-pushed the feature/curve-fs-mds-optimize-unit-tests branch from ba838ad to 289cf77 Compare September 6, 2023 09:38
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
curvefs/test/mds/mds_service_test.cpp Outdated Show resolved Hide resolved
@ken90242 ken90242 force-pushed the feature/curve-fs-mds-optimize-unit-tests branch from b00196d to 65666b0 Compare September 7, 2023 03:05
@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

@wu-hanqing
I have completed all the comments

@wu-hanqing
Copy link
Contributor

cicheck

2 similar comments
@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

@wu-hanqing
The ci test has passed. I will merge all the commits and run another ci test once you confirm this.

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

btw, who should I ask to be the second reviewer?

@wu-hanqing
Copy link
Contributor

btw, who should I ask to be the second reviewer?

I don't have any more questions, except squeeze 4 commits into 1.

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

Hi @wuhongsong @Cyber-SiKu

Would you mind reviewing my work? Thanks

@ken90242 ken90242 force-pushed the feature/curve-fs-mds-optimize-unit-tests branch from fe2dbd1 to 21b4a8a Compare September 8, 2023 10:49
@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

cicheck

3 similar comments
@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 8, 2023

cicheck

@wu-hanqing
Copy link
Contributor

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Sep 13, 2023

@SeanHai Could you please review my work when you get a chance? Thank you!

@wu-hanqing wu-hanqing merged commit 604409b into opencurve:master Sep 14, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants