Do not review #4291
Do not review #4291
Conversation
|
build |
|
build |
1 similar comment
|
build |
integration_tests/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.nvidia</groupId> | ||
| <artifactId>rapids-4-spark_${scala.binary.version}</artifactId> | ||
| <artifactId>rapids-4-spark-sql_${scala.binary.version}</artifactId> |
There was a problem hiding this comment.
The integration tests must not depend on the rapids-4-spark-sql artifact. The whole point of having tests outside of the rapids-4-spark-sql project is to test against only the dist artifact jar (i.e.: what users will use), and pulling in this jar is like pulling in the aggregator jars -- it defeats the purpose of separating the tests into a different project and does not test against the artifact that will be shipped (i.e.: is not a true integration test).
|
can we please update the description of the approach taken? |
|
It's a draft PR. I added some comments to the issue. |
|
@gerashegalov @jlowe @tgravescs Dynamically generate a jar for compile, then install it into the local maven repository, the script is in the dist module. The Integration-tests module depends on the dynamical jar. |
There was a problem hiding this comment.
Why are we moving a bunch of unit tests into the integration tests module? That is what is causing all the grief. The original plan posted in #3932 was to move tests that require direct access to internal classes into the sql-plugin project as regular unit tests and the remaining tests that run at the Spark SQL/Dataframe API layer into integration tests. Integration tests must not depend on the sql-plugin jar (or any other internal artifact).
| <artifactId>spark-sql_${scala.binary.version}</artifactId> | ||
| <version>${spark.test.version}</version> | ||
| <scope>compile</scope> |
There was a problem hiding this comment.
The integration tests are still depending on the spark-sql artifact and it will appear in the classpath when the tests are run.
There was a problem hiding this comment.
I agree with must not depend on the rapids-4-spark-sql artifact, but we must depend on spark-sql to run the Scala test cases. When running the Python test cases the spark-sql artifact will appear in the classpath because of command "$SPARK_HOME/bin/pyspark". But for the Scala test cases is a different path, the class path is specified by "mvn test".
I'll move some unit tests to the rapids-4-spark-sql module, but generally the unit tests can also run in the integration test provided the case can be compiled.
Signed-off-by: Chong Gao <res_life@163.com>
res-life
force-pushed
the
fix-dependency-agg
branch
from
9071d9b
to
c426826
Compare
|
build |
|
build |
|
The #3932 is invalid, so close it. |
This fixes #3932
Signed-off-by: Chong Gao res_life@163.com