Skip to content

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Oct 7, 2025

Which issue does this PR close?

Rationale for this change

I am working on the arrow 57 upgrade and I found I had to update the output of
many tests. I broke some of the changes out into their own PR for easier review

This one in particular breaks the 1200+ line test test_window_partial_constant_and_set_monotonicity, including the 63 test cases in into individual tests so they are easier to update.

Without this change, updating the 63 tests requires running the following commands in a loop (63 times):

cargo test --profile=ci --test core_integration -- enforce_sorting # fails on the first diff
cargo insta review # (and then hit a to accept the changes for the first diff)

What changes are included in this PR?

  1. Move the code to a new file
  2. Move testing code into functions rather than a macro
  3. Rewrite all test cases (used copilot and carefully validated the plans did not change)

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Oct 7, 2025
@alamb alamb force-pushed the alamb/split_up_enforce_sorting branch from 8d1f211 to 6b1bdc3 Compare October 10, 2025 19:52
// aal here

#[tokio::test]
async fn test_window_partial_constant_and_set_monotonicity() -> Result<()> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was simply moved to a new module

use insta::assert_snapshot;
use std::sync::{Arc, LazyLock};

// Function definition - Alias of the resulting column - Arguments of the function
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rewrote the harness to be a proper structure rather than macro, and then carefully ported each test
with the help of copilot magic auto complete.

I did not let copilot stick in the expected plan output -- instead I used the pre-existing plan output

@alamb alamb force-pushed the alamb/split_up_enforce_sorting branch from dc624c6 to 8267fb3 Compare October 11, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant