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

yangjie01 pushed a commit to branch branch-3.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.4 by this push:
     new e056fc8fb4d [SPARK-44074][CORE][SQL][TESTS][3.4] Fix loglevel restore 
behavior of `SparkFunSuite#withLogAppender` and re-enable UT `Logging plan 
changes for execution`
e056fc8fb4d is described below

commit e056fc8fb4d1c1ab8786a0989adc7298cc918dae
Author: yangjie01 <yangji...@baidu.com>
AuthorDate: Fri Sep 29 14:45:59 2023 +0800

    [SPARK-44074][CORE][SQL][TESTS][3.4] Fix loglevel restore behavior of 
`SparkFunSuite#withLogAppender` and re-enable UT `Logging plan changes for 
execution`
    
    ### What changes were proposed in this pull request?
    The main change of this pr is to add a call of 
`SparkFunSuite#wupdateLoggers` after restore loglevel when 'level' of 
`withLogAppender` function is not `None`, and under the premise of this change, 
the UT `Logging plan changes for execution` disabled in 
https://github.com/apache/spark/pull/43141 can be re-enabled.
    
    ### Why are the changes needed?
    - Fix bug of `SparkFunSuite#withLogAppender` when 'level' is not None
    - Re-enable UT `Logging plan changes for execution`
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    - Pass GitHub Actions
    - Manual test
    
    ```
    build/sbt "sql/testOnly org.apache.spark.sql.JoinHintSuite 
org.apache.spark.sql.execution.QueryExecutionSuite"
    ```
    
    **Before**
    
    ```
    [info] - Logging plan changes for execution *** FAILED *** (36 milliseconds)
    [info]   testAppender.loggingEvents.exists(((x$10: 
org.apache.logging.log4j.core.LogEvent) => 
x$10.getMessage().getFormattedMessage().contains(expectedMsg))) was false 
(QueryExecutionSuite.scala:232)
    [info]   org.scalatest.exceptions.TestFailedException:
    [info]   at 
org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
    [info]   at 
org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
    [info]   at 
org.scalatest.Assertions$.newAssertionFailedException(Assertions.scala:1231)
    [info]   at 
org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:1295)
    [info]   at 
org.apache.spark.sql.execution.QueryExecutionSuite.$anonfun$new$34(QueryExecutionSuite.scala:232)
    [info]   at scala.collection.immutable.List.foreach(List.scala:431)
    [info]   at 
org.apache.spark.sql.execution.QueryExecutionSuite.$anonfun$new$31(QueryExecutionSuite.scala:231)
    [info]   at 
scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    [info]   at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
    [info]   at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
    [info]   at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
    [info]   at org.scalatest.Transformer.apply(Transformer.scala:22)
    ...
    
    ```
    
    The failure reason is `withLogAppender(hintAppender, level = 
Some(Level.WARN))` used in `JoinHintSuite`, but `SparkFunSuite#wupdateLoggers` 
doesn't have the correct restore Loglevel.
    
    The test was successful before SPARK-44034 due to there was 
`AdaptiveQueryExecSuite` between `JoinHintSuite` and `QueryExecutionSuite`, and 
`AdaptiveQueryExecSuite` called `withLogAppender(hintAppender, level = 
Some(Level.DEBUG))`, but `AdaptiveQueryExecSuite` move to `slow sql` test group 
after SPARK-44034
    
    **After**
    
    ```
    [info] Run completed in 7 seconds, 485 milliseconds.
    [info] Total number of tests run: 32
    [info] Suites: completed 2, aborted 0
    [info] Tests: succeeded 32, failed 0, canceled 0, ignored 0, pending 0
    [info] All tests passed.
    ```
    
    Closes #43157 from LuciferYang/SPARK-44074-34.
    
    Authored-by: yangjie01 <yangji...@baidu.com>
    Signed-off-by: yangjie01 <yangji...@baidu.com>
---
 core/src/test/scala/org/apache/spark/SparkFunSuite.scala               | 1 +
 .../scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala     | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/core/src/test/scala/org/apache/spark/SparkFunSuite.scala 
b/core/src/test/scala/org/apache/spark/SparkFunSuite.scala
index 27198039fdb..0565e2b2d30 100644
--- a/core/src/test/scala/org/apache/spark/SparkFunSuite.scala
+++ b/core/src/test/scala/org/apache/spark/SparkFunSuite.scala
@@ -277,6 +277,7 @@ abstract class SparkFunSuite
           logger.asInstanceOf[Logger].setLevel(restoreLevels(i))
           logger.asInstanceOf[Logger].get().setLevel(restoreLevels(i))
         }
+        
LogManager.getContext(false).asInstanceOf[LoggerContext].updateLoggers()
       }
     }
   }
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
index 0a22efcb34d..d2a101b2395 100644
--- 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
@@ -220,8 +220,7 @@ class QueryExecutionSuite extends SharedSparkSession {
     assertNoTag(tag5, df.queryExecution.sparkPlan)
   }
 
-  // TODO(SPARK-44074): re-enable this test after SPARK-44074 resolved
-  ignore("Logging plan changes for execution") {
+  test("Logging plan changes for execution") {
     val testAppender = new LogAppender("plan changes")
     withLogAppender(testAppender) {
       withSQLConf(SQLConf.PLAN_CHANGE_LOG_LEVEL.key -> "INFO") {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to