villebro opened a new pull request, #2800: URL: https://github.com/apache/datafusion-ballista/pull/2800
# Rationale for this change This is the second in a series of cleanup PRs that remove duplicated test code, each one shrinking the code base, after #2791. This PR only moves fixtures into shared `#[cfg(test)]` modules of `ballista-core`. It does not add, remove or merge any test. Later PRs do the same for the scheduler and the remaining crates' tests. Several execution plan tests in `ballista-core` each defined the same fixtures: the two-by-two shuffle writer input, the one-column schema and partitioned source, the `(k, payload)` batch, the Int32 `a` schema, the same `PartitionLocation` literal, and a per-partition result driver. The range operators repeated five identical helpers between the ordered and unordered variants. # Which issue does this PR close? Not related to an issue. The series follows [a review comment on #2765](https://github.com/apache/datafusion-ballista/pull/2765#discussion_r4487993485) pointing out test fixtures copied between test files. # What changes are included in this PR? - A `#[cfg(test)]` module, `execution_plans/test_util.rs`, with `two_by_two_input`, `drive_partition_results` (generic over the writer), `one_col_schema`, `partitioned_source`, `key_payload_batch`, `int32_schema`/`int32_batch`, `write_ipc_stream`/`ipc_stream_bytes`, `test_location` and `four_partitions`. - `range_repartition_common::test_util` gains `schema_v2_id`, `batch`, `asc`, `empty_input`, `session`, `v_schema`, `cuts` and a generic `drain_concurrent`, replacing the copies in the range repartition, range filter and runtime stats tests. - `MemorySourceConfig::try_new_exec` replaces `DataSourceExec::new(Arc::new(MemorySourceConfig::try_new(..)))` where no sort information is set. - The "Test fixtures" section of the contributor guide (`docs/source/contributors-guide/development.md`) points to these fixtures. - No public API changes: `ballista_core::test_utils` is untouched, and the new helpers are `#[cfg(test)]`. Differences a reviewer may want to check: - `test_location` gives some range shuffle and runtime stats tests a different executor id and host than their hand-written literal. Those tests read local files or ignore the executor, so it is not observable. Literals whose executor id keys a per-address semaphore are left as they were. - `drive_all_partitions` in the shuffle writer tests now flattens `drive_partition_results`, so an `execute` error surfaces as `Execution(..)` and a panicking task panics the test. The one caller that checks errors only asserts `is_err()`. # Testing - `cargo test -p ballista-core --no-fail-fast`: all pass. - `cargo test -p ballista-core -- --list` is identical before and after (515 tests). - `cargo fmt --all -- --check` and `cargo clippy -p ballista-core --all-targets --all-features -- -D warnings`. # Are there any user-facing changes? No. Test code only. -- 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]
