Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Oct 3, 2025

Which issue does this PR close?

Rationale for this change

Let's keep datafusion tests efficient

Every test binary requires additional build time and space. Thus we should have fewer test binaries.

For example the apply_schema_adapter_tests test binary is 71MB on disk after building

andrewlamb@Andrews-MacBook-Pro-3:~/Software/datafusion2$ cargo test --test apply_schema_adapter_tests
   Compiling darling_core v0.21.3
...
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

andrewlamb@Andrews-MacBook-Pro-3:~/Software/datafusion2$ du -h target/debug/deps/apply_schema_adapter_tests-2091667b2c1cdecb
 71M	target/debug/deps/apply_schema_adapter_tests-2091667b2c1cdecb

But contains only a single test. Let's combine that with the existing schema adapter tests to make testing faster and more efficient

What changes are included in this PR?

  1. Consolidate schema adapter tests

Are these changes tested?

By CI

I also verified the test sill runs like this:

cargo test --test parquet_config -- schema_adapter
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.18s
     Running tests/parquet_config.rs (target/debug/deps/parquet_config-8b16110c71abdb9a)

running 3 tests
test parquet::schema_adapter::test_apply_schema_adapter_without_factory ... ok
test parquet::schema_adapter::test_apply_schema_adapter_with_factory ... ok
test parquet::schema_adapter::test_custom_schema_adapter_and_custom_expression_adapter ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 181 filtered out; finished in 0.01s

Are there any user-facing changes?

@github-actions github-actions bot added core Core DataFusion crate datasource Changes to the datasource crate labels Oct 3, 2025
];
assert_batches_eq!(expected, &batches);
}

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 just moved these tests, I did not change the logic

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 file implicitly generates a new test binary, so let's not do that for only a few tets

@alamb alamb marked this pull request as ready for review October 3, 2025 17:20
@alamb alamb requested review from adriangb and kosiew October 3, 2025 17:20
@adriangb
Copy link
Contributor

adriangb commented Oct 3, 2025

Thank you @alamb!

@adriangb adriangb added this pull request to the merge queue Oct 3, 2025
Merged via the queue into apache:main with commit daeb659 Oct 3, 2025
28 checks passed
@alamb alamb deleted the alamb/fewer_test_binaries branch October 3, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants