Skip to content

Conversation

@slfan1989
Copy link
Contributor

@slfan1989 slfan1989 commented Sep 16, 2025

Which issue does this PR close?

What changes are included in this PR?

  • Added tests for setting and removing statistics files in UpdateStatisticsAction.
  • Tests cover scenarios for:
  1. Setting a single statistics file.
  2. Ensuring no statistics are set.

Are these changes tested?

Tested locally.

@slfan1989 slfan1989 force-pushed the unit-tests-update-statistics-action branch from 898c637 to 18a4807 Compare September 16, 2025 11:43
…isticsAction.

- Tests cover scenarios for:
1. Setting a single statistics file.
2. Ensuring no statistics are set.
@slfan1989 slfan1989 force-pushed the unit-tests-update-statistics-action branch from 18a4807 to b4ea24a Compare September 16, 2025 11:47
@slfan1989
Copy link
Contributor Author

slfan1989 commented Sep 16, 2025

@Xuanwo @liurenjie1024 Could you please help review this PR? Thank you very much!

}

#[test]
fn test_set_single_statistics() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between this and test_update_statistics? Looks like both covert the set_statistics path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your question! In fact, test_no_statistics_set, test_set_single_statistics, and test_update_statistics test different scenarios.

  • test_no_statistics_set: Verifies that statistics_to_set is empty when no statistics are set or removed, ensuring the system handles this correctly when no operations are performed.

  • test_set_single_statistics: Verifies the setting of a single statistics file, ensuring that the set_statistics operation works correctly.

  • test_update_statistics: Tests the setting and removal of multiple statistics files, verifying the correct behavior of multiple set_statistics and remove_statistics operations combined.

Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @slfan1989 for adding this pr!

@liurenjie1024 liurenjie1024 merged commit f22c0f9 into apache:main Sep 25, 2025
16 checks passed
@slfan1989
Copy link
Contributor Author

Thanks @slfan1989 for adding this pr!

@liurenjie1024 @ZENOTME Thank you very much for helping to review the code!

DerGut pushed a commit to DerGut/iceberg-rust that referenced this pull request Oct 6, 2025
## Which issue does this PR close?

- Closes apache#1676.

## What changes are included in this PR?

- Added tests for setting and removing statistics files in
UpdateStatisticsAction.
- Tests cover scenarios for:
1. Setting a single statistics file.
2. Ensuring no statistics are set.

## Are these changes tested?

Tested locally.
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.

Add unit tests for UpdateStatisticsAction functionality.

3 participants