Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-18292][SQL] LogicalPlanToSQLSuite should not use resource dependent path for golden file generation #15789

Closed
wants to merge 2 commits into from
Closed

[SPARK-18292][SQL] LogicalPlanToSQLSuite should not use resource dependent path for golden file generation #15789

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
725cd45
[SPARK-18292][SQL] LogicalPlanToSQLSuite should not use resource depe…
dongjoon-hyun Nov 6, 2016
bd7258d
Fix
dongjoon-hyun Nov 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion sql/hive/src/test/scala/org/apache/spark/sql/catalyst/LogicalPlanToSQLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,15 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {

// Used for generating new query answer files by saving
private val regenerateGoldenFiles: Boolean = System.getenv("SPARK_GENERATE_GOLDEN_FILES") == "1"
private val goldenSQLPath = getTestResourcePath("sqlgen")
private val goldenSQLPath = {
// If regenerateGoldenFiles is true, we must be running this in SBT and we use hard-coded
// relative path. Otherwise, we use classloader's getResource to find the location.
if (regenerateGoldenFiles) {
java.nio.file.Paths.get("src", "test", "resources", "sqlgen").toFile.getCanonicalPath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I see. This seems OK. I think it's OK to just do new File("src/test/resources/sqlgen") here instead, since that's what other tests do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for review, @srowen .
For this one, there is a request in #15546 to follow the way SQLQueryTestSuite.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to confirm that you already tested it in your private branch, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that. But, to make it sure, I'll check it again right now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I tested this in my private branch in a separate folder. When I changed the query for range, the output file is changed like the following.

SPARK-18292-SQLGEN:SPARK-18292$ pwd
/Users/dhyun/SPARK-18292-SQLGEN
SPARK-18292-SQLGEN:SPARK-18292$ git branch
* SPARK-18292
  master
SPARK-18292-SQLGEN:SPARK-18292$ pwd
/Users/dhyun/SPARK-18292-SQLGEN
SPARK-18292-SQLGEN:SPARK-18292$ git status
On branch SPARK-18292
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   sql/hive/src/test/resources/sqlgen/range.sql
    modified:   sql/hive/src/test/scala/org/apache/spark/sql/catalyst/LogicalPlanToSQLSuite.scala

} else {
getTestResourcePath("sqlgen")
}
}

protected override def beforeAll(): Unit = {
super.beforeAll()
Expand Down