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