From 94bd480761b9f0cf8dc5b87421a4f63480d8e3a9 Mon Sep 17 00:00:00 2001 From: "yi.wu" Date: Fri, 14 May 2021 22:17:50 +0800 Subject: [PATCH] [SPARK-35206][TESTS][SQL] Extract common used get project path into a function in SparkFunctionSuite ### What changes were proposed in this pull request? Add a common functions `getWorkspaceFilePath` (which prefixed with spark home) to `SparkFunctionSuite`, and applies these the function to where they're extracted from. ### Why are the changes needed? Spark sql has test suites to read resources when running tests. The way of getting the path of resources is commonly used in different suites. We can extract them into a function to ease the code maintenance. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass existing tests. Closes #32315 from Ngone51/extract-common-file-path. Authored-by: yi.wu Signed-off-by: Wenchen Fan --- .../org/apache/spark/SparkFunSuite.scala | 20 +++++++++++++++---- .../spark/sql/catalyst/SQLKeywordSuite.scala | 8 ++++---- .../parser/TableIdentifierParserSuite.scala | 3 +-- .../spark/sql/ExpressionsSchemaSuite.scala | 3 +-- .../apache/spark/sql/PlanStabilitySuite.scala | 2 +- .../apache/spark/sql/SQLQueryTestSuite.scala | 3 +-- .../spark/sql/TPCDSQueryTestSuite.scala | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/core/src/test/scala/org/apache/spark/SparkFunSuite.scala b/core/src/test/scala/org/apache/spark/SparkFunSuite.scala index abd2d68bbd302..939e64d8e1767 100644 --- a/core/src/test/scala/org/apache/spark/SparkFunSuite.scala +++ b/core/src/test/scala/org/apache/spark/SparkFunSuite.scala @@ -19,21 +19,22 @@ package org.apache.spark // scalastyle:off import java.io.File +import java.nio.file.Path import java.util.{Locale, TimeZone} -import org.apache.log4j.spi.LoggingEvent - import scala.annotation.tailrec +import scala.collection.mutable.ArrayBuffer + import org.apache.commons.io.FileUtils import org.apache.log4j.{Appender, AppenderSkeleton, Level, Logger} +import org.apache.log4j.spi.LoggingEvent import org.scalatest.{BeforeAndAfter, BeforeAndAfterAll, BeforeAndAfterEach, Failed, Outcome} import org.scalatest.funsuite.AnyFunSuite + import org.apache.spark.internal.Logging import org.apache.spark.internal.config.Tests.IS_TESTING import org.apache.spark.util.{AccumulatorContext, Utils} -import scala.collection.mutable.ArrayBuffer - /** * Base abstract class for all unit tests in Spark for handling common functionality. * @@ -119,6 +120,17 @@ abstract class SparkFunSuite file } + /** + * Get a Path relative to the root project. It is assumed that a spark home is set. + */ + protected final def getWorkspaceFilePath(first: String, more: String*): Path = { + if (!(sys.props.contains("spark.test.home") || sys.env.contains("SPARK_HOME"))) { + fail("spark.test.home or SPARK_HOME is not set.") + } + val sparkHome = sys.props.getOrElse("spark.test.home", sys.env("SPARK_HOME")) + java.nio.file.Paths.get(sparkHome, first +: more: _*) + } + /** * Note: this method doesn't support `BeforeAndAfter`. You must use `BeforeAndAfterEach` to * set up and tear down resources. diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/SQLKeywordSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/SQLKeywordSuite.scala index 082b01173e7b8..45f88628f3ab3 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/SQLKeywordSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/SQLKeywordSuite.scala @@ -27,11 +27,11 @@ import org.apache.spark.SparkFunSuite import org.apache.spark.sql.catalyst.plans.SQLHelper import org.apache.spark.sql.catalyst.util.fileToString -trait SQLKeywordUtils extends SQLHelper { +trait SQLKeywordUtils extends SparkFunSuite with SQLHelper { val sqlSyntaxDefs = { val sqlBasePath = { - java.nio.file.Paths.get(sparkHome, "sql", "catalyst", "src", "main", "antlr4", "org", + getWorkspaceFilePath("sql", "catalyst", "src", "main", "antlr4", "org", "apache", "spark", "sql", "catalyst", "parser", "SqlBase.g4").toFile } fileToString(sqlBasePath).split("\n") @@ -41,7 +41,7 @@ trait SQLKeywordUtils extends SQLHelper { // Spark default mode, and the SQL standard. val keywordsInDoc: Array[Array[String]] = { val docPath = { - java.nio.file.Paths.get(sparkHome, "docs", "sql-ref-ansi-compliance.md").toFile + getWorkspaceFilePath("docs", "sql-ref-ansi-compliance.md").toFile } fileToString(docPath).split("\n") .dropWhile(!_.startsWith("|Keyword|")).drop(2).takeWhile(_.startsWith("|")) @@ -150,7 +150,7 @@ trait SQLKeywordUtils extends SQLHelper { } } -class SQLKeywordSuite extends SparkFunSuite with SQLKeywordUtils { +class SQLKeywordSuite extends SQLKeywordUtils { test("all keywords are documented") { val documentedKeywords = keywordsInDoc.map(_.head).toSet if (allCandidateKeywords != documentedKeywords) { diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala index bad3e0d79dd12..0e1a6df4c3020 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala @@ -16,11 +16,10 @@ */ package org.apache.spark.sql.catalyst.parser -import org.apache.spark.SparkFunSuite import org.apache.spark.sql.catalyst.{SQLKeywordUtils, TableIdentifier} import org.apache.spark.sql.internal.SQLConf -class TableIdentifierParserSuite extends SparkFunSuite with SQLKeywordUtils { +class TableIdentifierParserSuite extends SQLKeywordUtils { import CatalystSqlParser._ // Add "$elem$", "$value$" & "$key$" diff --git a/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala index f3db4d811dd86..f8071e6cda175 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/ExpressionsSchemaSuite.scala @@ -72,8 +72,7 @@ class ExpressionsSchemaSuite extends QueryTest with SharedSparkSession { // We use a path based on Spark home for 2 reasons: // 1. Maven can't get correct resource directory when resources in other jars. // 2. We test subclasses in the hive-thriftserver module. - java.nio.file.Paths.get(sparkHome, - "sql", "core", "src", "test", "resources", "sql-functions").toFile + getWorkspaceFilePath("sql", "core", "src", "test", "resources", "sql-functions").toFile } private val resultFile = new File(baseResourcePath, "sql-expression-schema.md") diff --git a/sql/core/src/test/scala/org/apache/spark/sql/PlanStabilitySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/PlanStabilitySuite.scala index 518597f59690d..0df18723921e8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/PlanStabilitySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/PlanStabilitySuite.scala @@ -84,7 +84,7 @@ trait PlanStabilitySuite extends TPCDSBase with DisableAdaptiveExecutionSuite { protected val baseResourcePath = { // use the same way as `SQLQueryTestSuite` to get the resource path - java.nio.file.Paths.get("src", "test", "resources", "tpcds-plan-stability").toFile + getWorkspaceFilePath("sql", "core", "src", "test", "resources", "tpcds-plan-stability").toFile } private val referenceRegex = "#\\d+".r diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala index b4f92af794891..609e0094f9ee4 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala @@ -131,8 +131,7 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper // We use a path based on Spark home for 2 reasons: // 1. Maven can't get correct resource directory when resources in other jars. // 2. We test subclasses in the hive-thriftserver module. - java.nio.file.Paths.get(sparkHome, - "sql", "core", "src", "test", "resources", "sql-tests").toFile + getWorkspaceFilePath("sql", "core", "src", "test", "resources", "sql-tests").toFile } protected val inputFilePath = new File(baseResourcePath, "inputs").getAbsolutePath diff --git a/sql/core/src/test/scala/org/apache/spark/sql/TPCDSQueryTestSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/TPCDSQueryTestSuite.scala index ddfed87d85af2..952e8968020aa 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/TPCDSQueryTestSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/TPCDSQueryTestSuite.scala @@ -79,7 +79,7 @@ class TPCDSQueryTestSuite extends QueryTest with TPCDSBase with SQLQueryTestHelp protected val baseResourcePath = { // use the same way as `SQLQueryTestSuite` to get the resource path - java.nio.file.Paths.get("src", "test", "resources", "tpcds-query-results") + getWorkspaceFilePath("sql", "core", "src", "test", "resources", "tpcds-query-results") .toFile.getAbsolutePath }