Skip to content

Commit

Permalink
[SPARK-21524][ML] unit test fix: ValidatorParamsSuiteHelpers generate…
Browse files Browse the repository at this point in the history
…s wrong temp files

## What changes were proposed in this pull request?
jira: https://issues.apache.org/jira/browse/SPARK-21524

ValidatorParamsSuiteHelpers.testFileMove() is generating temp dir in the wrong place and does not delete them.

ValidatorParamsSuiteHelpers.testFileMove() is invoked by TrainValidationSplitSuite and crossValidatorSuite. Currently it uses `tempDir` from `TempDirectory`, which unfortunately is never initialized since the `boforeAll()` of `ValidatorParamsSuiteHelpers` is never invoked.

In my system, it leaves some temp directories in the assembly folder each time I run the TrainValidationSplitSuite and crossValidatorSuite.

## How was this patch tested?
unit test fix

Author: Yuhao Yang <yuhao.yang@intel.com>

Closes #18728 from hhbyyh/tempDirFix.
  • Loading branch information
hhbyyh authored and srowen committed Jul 26, 2017
1 parent 1661263 commit ae4ea5f
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 6 deletions.
Expand Up @@ -222,7 +222,7 @@ class CrossValidatorSuite
.setNumFolds(20)
.setEstimatorParamMaps(paramMaps)

ValidatorParamsSuiteHelpers.testFileMove(cv)
ValidatorParamsSuiteHelpers.testFileMove(cv, tempDir)
}

test("read/write: CrossValidator with complex estimator") {
Expand Down
Expand Up @@ -209,7 +209,7 @@ class TrainValidationSplitSuite
.setEstimatorParamMaps(paramMaps)
.setSeed(42L)

ValidatorParamsSuiteHelpers.testFileMove(tvs)
ValidatorParamsSuiteHelpers.testFileMove(tvs, tempDir)
}

test("read/write: TrainValidationSplitModel") {
Expand Down
Expand Up @@ -20,11 +20,12 @@ package org.apache.spark.ml.tuning
import java.io.File
import java.nio.file.{Files, StandardCopyOption}

import org.apache.spark.SparkFunSuite
import org.scalatest.Assertions

import org.apache.spark.ml.param.{ParamMap, ParamPair, Params}
import org.apache.spark.ml.util.{DefaultReadWriteTest, Identifiable, MLReader, MLWritable}
import org.apache.spark.ml.util.{Identifiable, MLReader, MLWritable}

object ValidatorParamsSuiteHelpers extends SparkFunSuite with DefaultReadWriteTest {
object ValidatorParamsSuiteHelpers extends Assertions {
/**
* Assert sequences of estimatorParamMaps are identical.
* If the values for a parameter are not directly comparable with ===
Expand Down Expand Up @@ -62,7 +63,7 @@ object ValidatorParamsSuiteHelpers extends SparkFunSuite with DefaultReadWriteTe
* the path of the estimator so that if the parent directory changes, loading the
* model still works.
*/
def testFileMove[T <: Params with MLWritable](instance: T): Unit = {
def testFileMove[T <: Params with MLWritable](instance: T, tempDir: File): Unit = {
val uid = instance.uid
val subdirName = Identifiable.randomUID("test")

Expand Down

0 comments on commit ae4ea5f

Please sign in to comment.