jayzhan211 opened a new pull request, #26694:
URL: https://github.com/apache/datafusion/pull/26694
## Which issue does this PR close?
- None. Follow-up to the review of #26649.
## Rationale for this change
After #26649, the `extended` CI step picks its tests with name filters
(`fuzz_cases::`, `memory_limit_validation::`, `spill_pool_scenario_fuzz`). If a
test behind the `extended_tests` feature lives somewhere the filters do not
match, or a filtered test is renamed, the filter matches nothing, the step
still passes, and the test silently stops running in CI.
## What changes are included in this PR?
- The tests behind `extended_tests` now live in test targets that require
the feature as a whole:
- `fuzz` (unchanged contents).
- `memory_limit_validation`, which moves out of `core_integration` into
its own target.
Both have `required-features = ["extended_tests"]` in `Cargo.toml`, so the
three `#[cfg(feature = "extended_tests")]` lines in the source are gone.
- The `extended` step is now `cargo test -p datafusion --test fuzz --test
memory_limit_validation --features ...`, with no filters. A new test in either
target runs without touching the step, and renaming a target makes cargo fail.
- The spill pool fuzzer (1000 iterations) moves to its own step,
`spill-pool-fuzz`, run as a second step of the same job. It cannot share the
`extended` step, because a test filter applies to every test binary in the run.
It still selects its test by path.
- Two xtask tests guard what cargo cannot check. xtask is a workspace
member, so they run on every PR:
- `spill_pool_fuzz_test_exists` fails if the path the `spill-pool-fuzz`
step uses no longer names a test.
- `extended_tests_feature_only_gates_whole_targets` fails if any source
file gates code on `extended_tests` with a `cfg`, since such a test would never
run in CI.
- The memory validation runner re-runs each test in a child process by its
path. It now also fails if that path selects no test, which would otherwise
pass. Moving the tests changes those paths, so this checks the move too.
## What is the testing strategy for this PR?
- `cargo xtask ci step test extended` runs the same tests as before: 134 in
`fuzz`, and 21 plus 1 already ignored in `memory_limit_validation`.
- `cargo xtask ci step test spill-pool-fuzz` runs exactly 1 test.
- `cargo test -p datafusion --test fuzz` without the feature fails with
"requires the features: `extended_tests`", and `--tests` without the feature
skips both targets, so the `workspace` step no longer builds them.
- The two new xtask tests fail when the spill pool fuzzer is renamed, when
its file is moved, and when a `#[cfg(feature = "extended_tests")]` is added to
`core_integration.rs`. The memory validation runner fails when its test path
prefix is wrong.
- The `spill-pool-fuzz` step only compiles the `datafusion-physical-plan`
unit test binary, which the `extended` step also built before this PR.
The two targets still run one after the other, as before.
## Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
