zhuqi-lucas opened a new pull request, #22117:
URL: https://github.com/apache/datafusion/pull/22117

   ## Which issue does this PR close?
   
   Prep PR for #21976 (EnsureRequirements). Splits the test reorganization that 
@alamb requested in his review of that PR 
(https://github.com/apache/datafusion/pull/21976#pullrequestreview-4255127096) 
into its own change so the EnsureRequirements diff stays focused on the rule 
change.
   
   ## Rationale for this change
   
   When EnsureRequirements lands and replaces `EnforceDistribution` + 
`EnforceSorting`, we want the diff to show:
   
   1. the existing test files now calling `EnsureRequirements::new()` instead 
of the old rules (i.e. a function-name swap on already-moved files), and
   2. any genuinely new tests for new behavior.
   
   Today the four big test files live in `core/tests/physical_optimizer/`, 
while #21976 introduces a separate `ensure_requirements/new_tests.rs` plus 
inline tests in `ensure_requirements/mod.rs`. There is no clean way to diff 
them against the existing coverage. By moving the test files now, the follow-up 
rebase of #21976 will produce exactly the diff @alamb asked for.
   
   ## What changes are included in this PR?
   
   `git mv` of four integration test files plus the wiring needed to compile 
them in the new crate:
   
   | File | Tests |
   |---|---|
   | `enforce_distribution.rs` → `physical-optimizer/tests/` | 66 |
   | `enforce_sorting.rs` → `physical-optimizer/tests/` | 60 |
   | `enforce_sorting_monotonicity.rs` → `physical-optimizer/tests/` | 64 |
   | `replace_with_order_preserving_variants.rs` → `physical-optimizer/tests/` 
| 46 |
   
   Git detected the four moves as renames with 94–99% content similarity, so 
reviewers can use the rename view to confirm the test bodies didn't change.
   
   ### Mechanical changes
   
   - **`datafusion/physical-optimizer/Cargo.toml`**: adds the dev-dependencies 
the integration tests need (`datafusion`, `datafusion-datasource`, 
`datafusion-datasource-parquet`, `arrow`, `object_store`, etc.). `datafusion` 
appears as a dev-dependency even though `datafusion` itself depends on this 
crate; cargo permits this dev-dep cycle because it only affects test builds.
   
   - **`core/tests/physical_optimizer/mod.rs`**: removes the four moved `mod` 
declarations and adds `dead_code` suppression for `test_utils` (some helpers 
were only used by the moved tests).
   
   - **`physical-optimizer/tests/common/test_utils.rs`**: copy of 
`core/tests/physical_optimizer/test_utils.rs` with `#![allow(dead_code)]`. 
Living in `tests/common/` keeps it from being picked up as its own test binary. 
The two copies will be reconciled when the remaining 
`core/tests/physical_optimizer/` tests also move here.
   
   - **`enforce_sorting.rs`**: `DummyStreamPartition` (previously imported via 
`crate::memory_limit::DummyStreamPartition`) is inlined as a 20-line struct. 
The `memory_limit` directory module would otherwise pull in unrelated content.
   
   - **`enforce_sorting_monotonicity.rs`**: `EnforceSortingTest` (previously 
shared with `enforce_sorting.rs` as `pub(crate)`) is duplicated into this file. 
Each integration test is now its own crate root and cannot reach into a sibling 
integration test's `pub(crate)` items. The duplication is annotated as 
transient — once EnsureRequirements lands the helper goes away with the rule it 
tests.
   
   ## Verification
   
   ```
   cargo build --workspace --tests
   cargo test -p datafusion-physical-optimizer --test enforce_distribution      
          # 66 passed
   cargo test -p datafusion-physical-optimizer --test enforce_sorting           
          # 60 passed
   cargo test -p datafusion-physical-optimizer --test 
enforce_sorting_monotonicity        # 64 passed
   cargo test -p datafusion-physical-optimizer --test 
replace_with_order_preserving_variants  # 46 passed
   cargo test -p datafusion --test core_integration physical_optimizer::        
          # 218 passed (remaining core tests)
   cargo clippy --workspace --tests -- -D warnings
   cargo fmt --all --check
   ```
   
   ## Follow-ups (not in this PR)
   
   - Rebase #21976 on top of this PR; the diff there should now be a clean swap 
from `EnforceDistribution` / `EnforceSorting` to `EnsureRequirements` on the 
four moved files, plus the new rule implementation.
   - Move the remaining `core/tests/physical_optimizer/` integration tests 
alongside their rules in a separate PR; reconcile the duplicated 
`test_utils.rs` then.
   


-- 
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]

Reply via email to