Skip to content

Commit ff724d2

Browse files
xkrogencloud-fan
authored andcommitted
[SPARK-33214][TEST][HIVE] Stop HiveExternalCatalogVersionsSuite from using a hard-coded location to store localized Spark binaries
### What changes were proposed in this pull request? This PR changes `HiveExternalCatalogVersionsSuite` to, by default, use a standard temporary directory to store the Spark binaries that it localizes. It additionally adds a new System property, `spark.test.cache-dir`, which can be used to define a static location into which the Spark binary will be localized to allow for sharing between test executions. If the System property is used, the downloaded binaries won't be deleted after the test runs. ### Why are the changes needed? In SPARK-22356 (PR apache#19579), the `sparkTestingDir` used by `HiveExternalCatalogVersionsSuite` became hard-coded to enable re-use of the downloaded Spark tarball between test executions: ``` // For local test, you can set `sparkTestingDir` to a static value like `/tmp/test-spark`, to // avoid downloading Spark of different versions in each run. private val sparkTestingDir = new File("/tmp/test-spark") ``` However this doesn't work, since it gets deleted every time: ``` override def afterAll(): Unit = { try { Utils.deleteRecursively(wareHousePath) Utils.deleteRecursively(tmpDataDir) Utils.deleteRecursively(sparkTestingDir) } finally { super.afterAll() } } ``` It's bad that we're hard-coding to a `/tmp` directory, as in some cases this is not the proper place to store temporary files. We're not currently making any good use of it. ### Does this PR introduce _any_ user-facing change? Developer-facing changes only, as this is in a test. ### How was this patch tested? The test continues to execute as expected. Closes apache#30122 from xkrogen/xkrogen-SPARK-33214-hiveexternalversioncatalogsuite-fix. Authored-by: Erik Krogen <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent 0ad35ba commit ff724d2

File tree

1 file changed

+18
-6
lines changed

1 file changed

+18
-6
lines changed

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,33 @@ import org.apache.spark.util.Utils
4242
* Test HiveExternalCatalog backward compatibility.
4343
*
4444
* Note that, this test suite will automatically download spark binary packages of different
45-
* versions to a local directory `/tmp/spark-test`. If there is already a spark folder with
46-
* expected version under this local directory, e.g. `/tmp/spark-test/spark-2.0.3`, we will skip the
47-
* downloading for this spark version.
45+
* versions to a local directory. If the `spark.test.cache-dir` system property is defined, this
46+
* directory will be used. If there is already a spark folder with expected version under this
47+
* local directory, e.g. `/{cache-dir}/spark-2.0.3`, downloading for this spark version will be
48+
* skipped. If the system property is not present, a temporary directory will be used and cleaned
49+
* up after the test.
4850
*/
4951
@SlowHiveTest
5052
@ExtendedHiveTest
5153
class HiveExternalCatalogVersionsSuite extends SparkSubmitTestUtils {
54+
import HiveExternalCatalogVersionsSuite._
5255
private val isTestAtLeastJava9 = SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)
5356
private val wareHousePath = Utils.createTempDir(namePrefix = "warehouse")
5457
private val tmpDataDir = Utils.createTempDir(namePrefix = "test-data")
55-
// For local test, you can set `sparkTestingDir` to a static value like `/tmp/test-spark`, to
58+
// For local test, you can set `spark.test.cache-dir` to a static value like `/tmp/test-spark`, to
5659
// avoid downloading Spark of different versions in each run.
57-
private val sparkTestingDir = new File("/tmp/test-spark")
60+
private val sparkTestingDir = Option(System.getProperty(SPARK_TEST_CACHE_DIR_SYSTEM_PROPERTY))
61+
.map(new File(_)).getOrElse(Utils.createTempDir(namePrefix = "test-spark"))
5862
private val unusedJar = TestUtils.createJarWithClasses(Seq.empty)
5963

6064
override def afterAll(): Unit = {
6165
try {
6266
Utils.deleteRecursively(wareHousePath)
6367
Utils.deleteRecursively(tmpDataDir)
64-
Utils.deleteRecursively(sparkTestingDir)
68+
// Only delete sparkTestingDir if it wasn't defined to a static location by the system prop
69+
if (Option(System.getProperty(SPARK_TEST_CACHE_DIR_SYSTEM_PROPERTY)).isEmpty) {
70+
Utils.deleteRecursively(sparkTestingDir)
71+
}
6572
} finally {
6673
super.afterAll()
6774
}
@@ -307,3 +314,8 @@ object PROCESS_TABLES extends QueryTest with SQLTestUtils {
307314
}
308315
}
309316
}
317+
318+
object HiveExternalCatalogVersionsSuite {
319+
private val SPARK_TEST_CACHE_DIR_SYSTEM_PROPERTY = "spark.test.cache-dir"
320+
}
321+

0 commit comments

Comments
 (0)