-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Feat: Add an option for fast tests by gating slow tests to extended_tests feature #19237
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
Feat: Add an option for fast tests by gating slow tests to extended_tests feature #19237
Conversation
|
@2010YOUY01 hey , does this seem good ? |
2010YOUY01
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you! We can move forward after CI passes.
On my M4 Pro MacBook Pro, the cargo nextest run finish time goes from 2 min 30s -> 45s.
One potential follow-up to do is to rank the remaining tests running time, and deal with the slow ones, to further improve the fast test option speed.
kosiew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of multiple #[cfg(feature = "extended_tests")], you could have simplified with one line at datafusion/core/tests/fuzz.rs
#[cfg(feature = "extended_tests")] <-----
mod fuzz_cases;
57fd438 to
808822c
Compare
|
@kosiew Thanks I changed it, looks good now ?? |
|
@2010YOUY01 @kosiew Can we have it merged !? |
We typically wait 24 hours between approval and merge (except for trivial PRs like typo fixes) to give others a chance to review. |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great idea -- thank you @Yuvraj-cyborg and @2010YOUY01
|
🎉 |
Which issue does this PR close?
Closes #19228
Rationale for this change
This PR gates all fuzz tests behind the
extended_testsfeature flag, allowing developers to run a fast test suite (ideally under 1 minute) during development while still being able to run the full test suite in CI or when needed.What changes are included in this PR?
Added #[cfg(feature = "extended_tests")] to all fuzz test modules in mod.rs. This includes the following modules:
Are these changes tested?
Without the extended_tests feature:
cargo test --package datafusion --test fuzz -- --listThis shows 0 tests.
With the extended_tests feature:
cargo test --package datafusion --test fuzz --features extended_tests -- --listThis shows all ~100+ fuzz tests.
Are there any user-facing changes?
Yes. Users can now run:
Fast test suite (default):
cargo testor
cargo nextest runFull test suite with fuzz tests:
cargo test --features extended_testsor
cargo nextest run --features extended_tests