Skip to content

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

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

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 1 commit
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
42 changes: 42 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
<fasterxml.jackson.version>2.4.4</fasterxml.jackson.version>
<snappy.version>1.1.1.6</snappy.version>
<netlib.java.version>1.1.2</netlib.java.version>
<maven-jar-plugin.version>2.6</maven-jar-plugin.version>
Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Nit, I don't think this needs to be factored out into a property as I don't think it would be overridden. It just occurs one place too. The plugin versions are just written once in the parent.

Copy link
Contributor Author

@steveloughran steveloughran Mar 23, 2015

Choose a reason for hiding this comment

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

Lifted it from the hadoop code. the parent one is @ v 2.4, so it comes down to whether you are happy with what that parent gives you or not. Easy to alter.


<!--
Dependency scopes that can be overridden by enabling certain profiles. These profiles are
Expand Down Expand Up @@ -1265,6 +1266,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 +1474,46 @@
<groupId>org.scalatest</groupId>
<artifactId>scalatest-maven-plugin</artifactId>
</plugin>
<!-- Build the JARs-->
Copy link
Contributor

@pwendell pwendell Apr 1, 2015

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>
<version>${maven-jar-plugin.version}</version>
<configuration>
Copy link
Member

@srowen srowen Mar 24, 2015

Choose a reason for hiding this comment

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

Sorry, I still wasn't sure what this config was for, what does it do?

<!-- Configuration of the archiver -->
<archive>
Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Is this config needed? I haven't usually needed anything more than the executions below in this kind of incantation.

Copy link
Contributor Author

@steveloughran steveloughran Mar 23, 2015

Choose a reason for hiding this comment

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

primarily to say what you want and the version. if the version control is cut, not needed any more

<manifestEntries>
<mode>development</mode>
<url>${project.url}</url>
</manifestEntries>
<manifest>
<addDefaultSpecificationEntries>true</addDefaultSpecificationEntries>
<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
</manifest>
</archive>
</configuration>
<executions>
<execution>
Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Nit: the <execution>s should be indented 2 more spaces

Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Also not 100% sure but do you need to re-declare the jar goal? I think it's inherited from the default config.

Copy link
Contributor Author

@steveloughran steveloughran Mar 23, 2015

Choose a reason for hiding this comment

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

I believe that as the default config is being overrriden to give it the new goal, the original one needs to be repeated

<id>prepare-jar</id>
<phase>prepare-package</phase>
<goals>
<goal>jar</goal>
</goals>
</execution>
<execution>
<id>prepare-test-jar</id>
<phase>prepare-package</phase>
<goals>
<goal>test-jar</goal>
</goals>
<configuration>
<excludes>
<exclude>log4j.properties</exclude>
</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