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

SPARK-6433 hive tests to import spark-sql test JAR for QueryTest access #5119

Closed
wants to merge 5 commits into from
Closed

SPARK-6433 hive tests to import spark-sql test JAR for QueryTest access #5119

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
650f442
SPARK-6433 patch 001: test JARs are built; sql/hive pulls in spark-sq…
steveloughran Mar 21, 2015
fdac51b
SPARK-6433 -002; indentation & delegate plugin version to parent
steveloughran Mar 23, 2015
c2b5f89
SPARK-6433 omit "jar" goal from jar plugin
steveloughran Mar 27, 2015
a6dca33
SPARK-6433 : pull configuration section form archive plugin
steveloughran Mar 31, 2015
81ceb01
SPARK-6433 add a clearer comment explaining what the plugin is doing …
steveloughran Apr 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,7 @@
<id>create-source-jar</id>
<goals>
<goal>jar-no-fork</goal>
<goal>test-jar-no-fork</goal>
</goals>
</execution>
</executions>
Expand Down Expand Up @@ -1472,6 +1473,25 @@
<groupId>org.scalatest</groupId>
<artifactId>scalatest-maven-plugin</artifactId>
</plugin>
<!-- Build the JARs-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this say something more useful? Maybe like:

<!-- Build test-jar's for all projects, since some projects depend on tests from others -->

Otherwise it might be hard for someone to understand what is going on here.

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<executions>
<execution>
Copy link
Member

Choose a reason for hiding this comment

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

I think the default jar execution is already active as it's a part of the standard Maven config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maven is mystery too all. This bit of build was lifted from hadoop. Your call as to whether you want the jar execution cut

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it can be mysterious; I feel reasonably sure about how the jar plugin works. I'd rather remove the config if we don't know that it's needed, and I have never need these. I also don't re-specify the default jar execution and it's been fine. We'll know quickly of course whether it doesn't actually produce JARs! but yeah I'd trim this down to what is necessary IMHO.

<id>prepare-test-jar</id>
<phase>prepare-package</phase>
<goals>
<goal>test-jar</goal>
</goals>
<configuration>
<excludes>
<exclude>log4j.properties</exclude>
Copy link
Member

Choose a reason for hiding this comment

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

Just being thorough here at the end -- so the idea here is that the tests shouldn't 'publish' their logging config right? I think that will only affect the log4j.properties in the test dirs, yeah. As long as it doesn't remove the one actually used, in the core source tree, in the final assembly.

</excludes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
14 changes: 14 additions & 0 deletions sql/hive/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,20 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.spark</groupId>
<artifactId>spark-sql_${scala.binary.version}</artifactId>
<type>test-jar</type>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.spark</groupId>
<artifactId>spark-catalyst_${scala.binary.version}</artifactId>
<type>test-jar</type>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
<profiles>
<profile>
Expand Down
140 changes: 0 additions & 140 deletions sql/hive/src/test/scala/org/apache/spark/sql/QueryTest.scala
View file
Open in desktop

This file was deleted.

57 changes: 0 additions & 57 deletions sql/hive/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/CachedTableSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,6 @@ import org.apache.spark.sql.{AnalysisException, DataFrame, QueryTest}
import org.apache.spark.storage.RDDBlockId

class CachedTableSuite extends QueryTest {
/**
* Throws a test failed exception when the number of cached tables differs from the expected
* number.
*/
def assertCached(query: DataFrame, numCachedTables: Int = 1): Unit = {
val planWithCaching = query.queryExecution.withCachedData
val cachedData = planWithCaching collect {
case cached: InMemoryRelation => cached
}

assert(
cachedData.size == numCachedTables,
s"Expected query to contain $numCachedTables, but it actually had ${cachedData.size}\n" +
planWithCaching)
}

def rddIdOf(tableName: String): Int = {
val executedPlan = table(tableName).queryExecution.executedPlan
Expand Down
28 changes: 0 additions & 28 deletions streaming/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,34 +97,6 @@
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
<testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
<plugins>
<!--
This plugin forces the generation of jar containing streaming test classes,
so that the tests classes of external modules can use them. The two execution profiles
are necessary - first one for 'mvn package', second one for 'mvn test-compile'. Ideally,
'mvn compile' should not compile test classes and therefore should not need this.
However, an open Maven bug (http://jira.codehaus.org/browse/MNG-3559)
causes the compilation to fail if streaming test-jar is not generated. Hence, the
second execution profile for 'mvn test-compile'.
-->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>test-jar</goal>
</goals>
</execution>
<execution>
<id>test-jar-on-test-compile</id>
<phase>test-compile</phase>
<goals>
<goal>test-jar</goal>
</goals>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
Expand Down