This is an automated email from the ASF dual-hosted git repository.

andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-ballista.git


The following commit(s) were added to refs/heads/main by this push:
     new 91705602c test: centralize physical plan snapshots (#2297)
91705602c is described below

commit 91705602c9e7aa1b58587905cb73d49371046b97
Author: Floze <[email protected]>
AuthorDate: Sat Aug 15 19:16:30 2026 +0400

    test: centralize physical plan snapshots (#2297)
---
 ballista/core/src/lib.rs                                 | 16 ++++++++++++++++
 ballista/scheduler/src/planner.rs                        | 15 ++++++---------
 ballista/scheduler/src/state/aqe/mod.rs                  | 12 ------------
 .../scheduler/src/state/aqe/optimizer_rule/chaos_exec.rs |  2 +-
 .../src/state/aqe/optimizer_rule/distributed_exchange.rs |  2 +-
 .../src/state/aqe/optimizer_rule/join_selection.rs       |  2 +-
 ballista/scheduler/src/state/aqe/test/alter_stages.rs    |  2 +-
 ballista/scheduler/src/state/aqe/test/coalesce_rule.rs   |  2 +-
 ballista/scheduler/src/state/aqe/test/join_selection.rs  |  6 +++---
 ballista/scheduler/src/state/aqe/test/plan_to_stages.rs  |  2 +-
 10 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/ballista/core/src/lib.rs b/ballista/core/src/lib.rs
index feb3f0586..244361eee 100644
--- a/ballista/core/src/lib.rs
+++ b/ballista/core/src/lib.rs
@@ -46,6 +46,22 @@ pub fn print_version() {
     println!("Ballista version: {BALLISTA_VERSION}")
 }
 
+/// Asserts an indented physical plan against an inline snapshot.
+///
+/// Keeping plan assertions behind one macro makes plan changes easier to
+/// review and update consistently across Ballista crates.
+#[macro_export]
+macro_rules! assert_plan {
+    ($plan:expr, @ $expected_lines:literal $(,)?) => {{
+        let plan = datafusion::physical_plan::displayable($plan)
+            .indent(true)
+            .to_string();
+        let actual_lines = plan.trim();
+
+        insta::assert_snapshot!(actual_lines, @ $expected_lines);
+    }};
+}
+
 /// Client utilities for connecting to Ballista schedulers.
 pub mod client;
 /// Connection pool for reusing `BallistaClient` instances across requests.
diff --git a/ballista/scheduler/src/planner.rs 
b/ballista/scheduler/src/planner.rs
index 14453294e..bb3c80c60 100644
--- a/ballista/scheduler/src/planner.rs
+++ b/ballista/scheduler/src/planner.rs
@@ -928,9 +928,9 @@ pub(crate) fn create_shuffle_writer_with_config(
 #[cfg(test)]
 mod test {
     use super::{can_stay_inline, holds_a_broadcast};
-    use crate::assert_plan;
     use crate::planner::{DefaultDistributedPlanner, DistributedPlanner};
     use crate::test_utils::{datafusion_test_context, scan_with_file_groups};
+    use ballista_core::assert_plan;
     use ballista_core::error::BallistaError;
     use ballista_core::execution_plans::{SortShuffleWriterExec, 
UnresolvedShuffleExec};
     use ballista_core::serde::BallistaCodec;
@@ -968,10 +968,10 @@ mod test {
 
         let rewritten = make_empty_exec_serde_safe(plan).unwrap();
 
-        assert_eq!(
-            displayable(rewritten.as_ref()).indent(false).to_string(),
-            "RepartitionExec: partitioning=RoundRobinBatch(4), 
input_partitions=1\n  EmptyExec\n"
-        );
+        assert_plan!(rewritten.as_ref(), @r"
+        RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1
+          EmptyExec
+        ");
     }
 
     #[test]
@@ -985,10 +985,7 @@ mod test {
 
         let rewritten = make_empty_exec_serde_safe(plan).unwrap();
 
-        assert_eq!(
-            displayable(rewritten.as_ref()).indent(false).to_string(),
-            "EmptyExec\n"
-        );
+        assert_plan!(rewritten.as_ref(), @"EmptyExec");
     }
 
     /// A plain scan branch restricts away cleanly, so it can stay inline in
diff --git a/ballista/scheduler/src/state/aqe/mod.rs 
b/ballista/scheduler/src/state/aqe/mod.rs
index d06b19d3e..5a1764552 100644
--- a/ballista/scheduler/src/state/aqe/mod.rs
+++ b/ballista/scheduler/src/state/aqe/mod.rs
@@ -1524,15 +1524,3 @@ fn scalar_to_f64(sv: &ScalarValue) -> 
datafusion::common::Result<f64> {
         ),
     }
 }
-
-/// Checks is the plan same as expected string representation
-#[cfg(test)]
-#[macro_export]
-macro_rules! assert_plan {
-    ($PLAN: expr, @ $EXPECTED_LINES: literal $(,)?) => {
-        let plan = 
datafusion::physical_plan::displayable($PLAN).indent(true).to_string();
-        let actual_lines = plan.trim();
-
-        insta::assert_snapshot!(actual_lines, @ $EXPECTED_LINES);
-    };
-}
diff --git a/ballista/scheduler/src/state/aqe/optimizer_rule/chaos_exec.rs 
b/ballista/scheduler/src/state/aqe/optimizer_rule/chaos_exec.rs
index bef85f3fc..f7ca5d8f4 100644
--- a/ballista/scheduler/src/state/aqe/optimizer_rule/chaos_exec.rs
+++ b/ballista/scheduler/src/state/aqe/optimizer_rule/chaos_exec.rs
@@ -109,7 +109,7 @@ impl PhysicalOptimizerRule for ChaosCreatingRule {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::assert_plan;
+    use ballista_core::assert_plan;
     use ballista_core::config::BallistaConfig;
     use ballista_core::execution_plans::ChaosExec;
     use datafusion::arrow::datatypes::{DataType, Field, Schema};
diff --git 
a/ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs 
b/ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs
index 33084d228..822d82f28 100644
--- a/ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs
+++ b/ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs
@@ -319,8 +319,8 @@ fn nearest_exchange_status(plan: &Arc<dyn ExecutionPlan>) 
-> ExchangeStatus {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::assert_plan;
     use crate::state::aqe::execution_plan::{AdaptiveDatafusionExec, 
ExchangeExec};
+    use ballista_core::assert_plan;
     use ballista_core::execution_plans::{
         RuntimeStatsExec, UnorderedRangeRepartitionExec,
     };
diff --git a/ballista/scheduler/src/state/aqe/optimizer_rule/join_selection.rs 
b/ballista/scheduler/src/state/aqe/optimizer_rule/join_selection.rs
index 78b8fa1c1..ce4f79146 100644
--- a/ballista/scheduler/src/state/aqe/optimizer_rule/join_selection.rs
+++ b/ballista/scheduler/src/state/aqe/optimizer_rule/join_selection.rs
@@ -365,7 +365,7 @@ mod tests {
     use super::*;
     use std::sync::Arc;
 
-    use crate::assert_plan;
+    use ballista_core::assert_plan;
     use ballista_core::config::BallistaConfig;
     use datafusion::{
         arrow::{
diff --git a/ballista/scheduler/src/state/aqe/test/alter_stages.rs 
b/ballista/scheduler/src/state/aqe/test/alter_stages.rs
index bb31ab7bc..387f1c58b 100644
--- a/ballista/scheduler/src/state/aqe/test/alter_stages.rs
+++ b/ballista/scheduler/src/state/aqe/test/alter_stages.rs
@@ -15,11 +15,11 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::assert_plan;
 use crate::state::aqe::planner::AdaptivePlanner;
 use crate::state::aqe::test::{
     mock_batch, mock_context, mock_partitions_with_statistics_no_data,
 };
+use ballista_core::assert_plan;
 use ballista_core::serde::scheduler::{
     ExecutorMetadata, ExecutorOperatingSystemSpecification, 
ExecutorSpecification,
     PartitionId, PartitionLocation, PartitionStats,
diff --git a/ballista/scheduler/src/state/aqe/test/coalesce_rule.rs 
b/ballista/scheduler/src/state/aqe/test/coalesce_rule.rs
index c51cff0a1..a3da8bb8b 100644
--- a/ballista/scheduler/src/state/aqe/test/coalesce_rule.rs
+++ b/ballista/scheduler/src/state/aqe/test/coalesce_rule.rs
@@ -24,9 +24,9 @@
 //! `coalesce_target_partition_bytes` so the bin-pack outcome is hand-traceable
 //! against `split_size_list_by_target_size`.
 
-use crate::assert_plan;
 use crate::state::aqe::planner::AdaptivePlanner;
 use crate::state::aqe::test::{mock_batch, mock_schema};
+use ballista_core::assert_plan;
 use ballista_core::extension::SessionConfigExt;
 use ballista_core::serde::scheduler::{
     ExecutorMetadata, ExecutorOperatingSystemSpecification, 
ExecutorSpecification,
diff --git a/ballista/scheduler/src/state/aqe/test/join_selection.rs 
b/ballista/scheduler/src/state/aqe/test/join_selection.rs
index 9bf234778..6243d7010 100644
--- a/ballista/scheduler/src/state/aqe/test/join_selection.rs
+++ b/ballista/scheduler/src/state/aqe/test/join_selection.rs
@@ -15,10 +15,10 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::{
-    assert_plan,
-    state::aqe::{planner::AdaptivePlanner, 
test::mock_partitions_with_statistics},
+use crate::state::aqe::{
+    planner::AdaptivePlanner, test::mock_partitions_with_statistics,
 };
+use ballista_core::assert_plan;
 use ballista_core::extension::SessionConfigExt;
 use datafusion::{
     arrow::{
diff --git a/ballista/scheduler/src/state/aqe/test/plan_to_stages.rs 
b/ballista/scheduler/src/state/aqe/test/plan_to_stages.rs
index 5290ff26d..3a18933e7 100644
--- a/ballista/scheduler/src/state/aqe/test/plan_to_stages.rs
+++ b/ballista/scheduler/src/state/aqe/test/plan_to_stages.rs
@@ -15,7 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::assert_plan;
 use crate::state::aqe::AdaptiveExecutionGraph;
 use crate::state::aqe::execution_plan::ExchangeExec;
 use crate::state::aqe::planner::AdaptivePlanner;
@@ -24,6 +23,7 @@ use crate::state::aqe::test::{
 };
 use crate::state::execution_graph::ExecutionGraph;
 use ballista_core::JobId;
+use ballista_core::assert_plan;
 use ballista_core::execution_plans::SortShuffleWriterExec;
 use ballista_core::serde::protobuf::job_status::Status;
 use datafusion::arrow::datatypes::{DataType, Field, Schema};


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to