wiedld commented on code in PR #15074:
URL: https://github.com/apache/datafusion/pull/15074#discussion_r1985716975


##########
datafusion/core/tests/physical_optimizer/enforce_distribution.rs:
##########
@@ -442,40 +445,30 @@ impl TestConfig {
         self.config.execution.target_partitions = target_partitions;
         self
     }
-}
-
-/// Runs the repartition optimizer and asserts the plan against the expected
-/// Arguments
-/// * `EXPECTED_LINES` - Expected output plan
-/// * `PLAN` - Input plan
-/// * `CONFIG` - [`TestConfig`]
-macro_rules! assert_optimized {
-    ($EXPECTED_LINES: expr, $PLAN: expr, $CONFIG: expr) => {
-        let expected_lines: Vec<&str> = $EXPECTED_LINES.iter().map(|s| 
*s).collect();
 
-        let TestConfig {
-            config,
-            optimizers_to_run,
-        } = $CONFIG;
-
-        // NOTE: These tests verify the joint `EnforceDistribution` + 
`EnforceSorting` cascade
-        //       because they were written prior to the separation of 
`BasicEnforcement` into
-        //       `EnforceSorting` and `EnforceDistribution`.
-        // TODO: Orthogonalize the tests here just to verify 
`EnforceDistribution` and create
-        //       new tests for the cascade.
+    /// Perform a series of runs using the current [`TestConfig`],
+    /// assert the expected plan result,
+    /// and return the result plan (for potentional subsequent runs).
+    fn run(
+        &self,
+        expected_lines: &[&str],
+        plan: Arc<dyn ExecutionPlan>,
+        optimizers_to_run: Vec<Run>,
+    ) -> Result<Arc<dyn ExecutionPlan>> {

Review Comment:
   The reason this returns the plan, is so that we can easily write test cases 
for idempotency.
   
   ```
   let plan_after_first_run = test_config.run(
       expected,
       plan,
       vec![Run::Distribution],
   )?;
   let plan_after_second_run = test_config.run(
       expected,
       plan_after_first_run.clone(),
       vec![Run::Distribution], // exact same run again
   )?;
   assert_eq!(
       get_plan_string(&plan_after_first_run),
       get_plan_string(&plan_after_second_run),
   );
   ```



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