Refactor: Update replace_with_order_preserving_variants tests to use insta snapshots for easier updates
#17962
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d testing Converted all tests in replace_with_order_preserving_variants.rs from macro-based assertions to snapshot-based testing using the insta crate. Key changes: - Replaced assert_optimized_in_all_boundedness_situations! and related macros with ReplaceTest helper struct - Used allow_duplicates! and assert_snapshot! for cleaner test assertions - Match order follows original test parameter ordering: (true, _), (false, false), (false, true) - All snapshots match the original expected plans from the macros - Removed unused imports and macros 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…er_preserving_variants tests Replace boolean test parameters with more descriptive enums: - Add `Boundedness` enum with `Unbounded` and `Bounded` variants - Add `SortPreference` enum with `PreserveOrder` and `MaximizeParallelism` variants - Update `ReplaceTest` struct to use these enums instead of bool fields - Rename methods from `with_source_unbounded`/`with_prefer_existing_sort` to `with_boundedness`/`with_sort_preference` - Update all test functions to use enum parameters instead of bool - Update all match patterns to use enum variants This makes the tests more self-documenting and easier to understand, as the enum variant names clearly indicate what behavior is being tested. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
blaginin
commented
Oct 8, 2025
|
|
||
| allow_duplicates! { | ||
| match (boundedness, sort_pref) { | ||
| (Boundedness::Bounded, SortPreference::MaximizeParallelism) => { |
There was a problem hiding this comment.
i changed it from bools because otherwise it'd be hard to understand what does e.g. true, false mean
Replace all `if boundedness == Boundedness::Unbounded` checks with cleaner match expressions. This improves code readability and makes the pattern matching more explicit and idiomatic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
alamb
approved these changes
Oct 8, 2025
| async fn test_replace_multiple_input_repartition_1( | ||
| #[values(false, true)] source_unbounded: bool, | ||
| #[values(false, true)] prefer_existing_sort: bool, | ||
| #[values(Boundedness::Unbounded, Boundedness::Bounded)] boundedness: Boundedness, |
There was a problem hiding this comment.
this is so much easier to read!
|
|
||
| allow_duplicates! { | ||
| match (boundedness, sort_pref) { | ||
| (Boundedness::Bounded, SortPreference::MaximizeParallelism) => { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
coretests toinsta#15791