dongjoon-hyun commented on a change in pull request #31793:
URL: https://github.com/apache/spark/pull/31793#discussion_r591085071



##########
File path: 
sql/core/src/test/scala/org/apache/spark/sql/execution/adaptive/AdaptiveQueryExecSuite.scala
##########
@@ -869,6 +870,26 @@ class AdaptiveQueryExecSuite
     }
   }
 
+  test("SPARK-34682: CustomShuffleReaderExec operating on canonicalized plan") 
{
+    withSQLConf(SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "true") {
+      val (_, adaptivePlan) = runAdaptiveAndVerifyResult(
+        "SELECT key FROM testData GROUP BY key")
+      val readers = collect(adaptivePlan) {
+        case r: CustomShuffleReaderExec => r
+      }
+      assert(readers.length == 1)
+      val reader = readers.head
+      val c = reader.canonicalized.asInstanceOf[CustomShuffleReaderExec]
+      // we can't just call execute() because that has separate checks for 
canonicalized plans
+      val doExecute = c.getClass.getMethod("doExecute")
+      doExecute.setAccessible(true)
+      val ex = intercept[InvocationTargetException] {
+        doExecute.invoke(c)
+      }
+      assert(ex.getCause.getMessage === "operating on canonicalized plan")

Review comment:
       For the following question, ScalaTest's 
`PrivateMethodTester.invokePrivate` is recommended.
   > The problem is that doExecute is protected, and I can't test by calling 
execute because it has separate checks for canonicalized plans. What would be 
the recommended approach here?
   
   So, you can revise this test case like the following.
   
   ```scala
   -      // we can't just call execute() because that has separate checks for 
canonicalized plans
   -      val doExecute = c.getClass.getMethod("doExecute")
   -      doExecute.setAccessible(true)
   -      val ex = intercept[InvocationTargetException] {
   -        doExecute.invoke(c)
   +      val doExecute = PrivateMethod[Unit](Symbol("doExecute"))
   +      val ex = intercept[IllegalStateException] {
   +        c.invokePrivate(doExecute())
          }
   -      assert(ex.getCause.getMessage === "operating on canonicalized plan")
   +      assert(ex.getMessage === "operating on canonicalized plan")
   ```




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

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