SPARK-2903 [BUILD] [SQL] Spark SQL tests fail to compile due to dependency structure, misplaced test class

[srowen]

Since recently, I find that the SQL modules' test code fails to compile with a simple mvn clean compile test-compile. sql/core fails with a large number of errors, beginning like so:

[error] /Users/srowen/Documents/spark/sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala:23: not found: type PlanTest
[error] class QueryTest extends PlanTest {
[error]                         ^
[error] /Users/srowen/Documents/spark/sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala:28: package org.apache.spark.sql.test is not a value
[error]   test("SPARK-1669: cacheTable should be idempotent") {
...

I think a couple files and dependencies are in the wrong place, and it is resolved with the following:

• Move org.apache.spark.sql.test.TestSQLContext from under sql/core/src/main/scala, to sql/core/src/test/scala , which seems like where it belongs as test code
• Move org.apache.spark.sql.parquet.ParquetTestData likewise from main to test
• QueryTest in sql/core depends on PlanTest, which appears to be catalyst-specific, instead of FunSuite. It shouldn't, and isn't needed.
• Remove dependency from sql/core tests on sql/catalyst tests, which seems reversed
• Introduce dependency from sql/hive tests on sql/core tests to restore access to the files moved above

What I'm still not clear on is why it has only started failing to compile in the last week or so. But, these seem like the right changes to make. It compiles and tests pass.

[SparkQA]

QA tests have started for PR 1833. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18117/consoleFull

[SparkQA]

QA results for PR 1833:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18117/consoleFull

[yhuai]

@srowen will this PR affect sbt/sbt hive/console and sbt/sbt sql/console?

[srowen]

@yhuai It should not, as it only affects tests. Granted, the PR moves two apparently test-only classes out of main into test, but they were not used by any non-test code (or else this PR would not compile).

I tried both of those commands, and I actually get a crash from the Scala compiler. Ominous, but, it was happening without this change too. I don't know what's up with my Scala. Jenkins thinks it's OK, although I don't think he's using SBT.

Is there a specific way this could affect these commands that I should look out for?

[yhuai]

For those two commands, we import org.apache.spark.sql.test.TestSQLContext._ and org.apache.spark.sql.hive.test.TestHive._, respectively. I was wondering if sql/console and hive/console still work after moving TestSQLContext and TestHive to test.

[srowen]

Ohh, I see that now. Hm. I definitely need to either determine it works fine, or find another solution that doesn't involve moving those classes. Now it looks like there certainly was a purpose to where they are at the moment.

[srowen]

@yhuai OK, forget almost all of the PR. I narrowed it down to extending FunSuite only. That fixes the compile-time problem. It came from commit d2f4f30 Do you think it's OK?

It doesn't affect things when you run mvn package, for sort of complex reasons. So in a way this is merely minor cleanup. If it is safe to weaken the superclass, maybe worthwhile.

[SparkQA]

QA tests have started for PR 1833. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18143/consoleFull

[srowen]

Agh, you know what, there are other problems of this form once the build reaches the external/* modules. Test modules aren't seeing other test modules that they depend on unless test JARs are created. That's really the issue, not this code. I will separately try to see if the build can be coaxed into handling test-test dependency without "mvn package".

[SparkQA]

QA results for PR 1833:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds the following public classes (experimental):
class QueryTest extends FunSuite {

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18143/consoleFull