[WIP][SPARK-42240][INFRA][CONNECT][TESTS] Move ClientE2ETestSuite into a separate module and add new GA task to test shaded jvm client with maven
#39807
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LuciferYang
changed the title
LuciferYang
changed the title
LuciferYang
commented
Jan 30, 2023
| <version>${guava.version}</version> | ||
| <scope>compile</scope> | ||
| </dependency> | ||
| <!-- |
Contributor
Author
There was a problem hiding this comment.
move ClientE2ETestSuite out of client jvm, we can revert this change.
Contributor
Author
There was a problem hiding this comment.
If necessary, I can do this change in a separate pr
LuciferYang
commented
Jan 30, 2023
connector/connect/client/jvm/pom.xml
Outdated
| <artifactSet> | ||
| <includes> | ||
| <include>com.google.guava:*</include> | ||
| <include>io.grpc:*:</include> |
Contributor
Author
There was a problem hiding this comment.
should revert this change after #39789 merged
LuciferYang
commented
Jan 30, 2023
| with: | ||
| distribution: temurin | ||
| java-version: ${{ matrix.java }} | ||
| - name: Build and Test with Maven |
Contributor
Author
There was a problem hiding this comment.
The reason for using maven for build and test is that in the current dependency configuration way, I don't know how SBT fetch assembly connect client jar
Contributor
Author
|
Test first, will update pr description later |
LuciferYang
commented
Jan 30, 2023
| sys.props.getOrElse("spark.test.home", sys.env("SPARK_HOME")) | ||
| } | ||
| private val isDebug = System.getProperty(DEBUG_SC_JVM_CLIENT, "false").toBoolean | ||
| private val isDebug = System.getProperty(DEBUG_SC_JVM_CLIENT, "true").toBoolean |
Contributor
Author
There was a problem hiding this comment.
will revert this change after all test passed
This comment was marked as outdated.
This comment was marked as outdated.
LuciferYang
changed the title
ClientE2ETestSuite into a separate module and add new GA task to test shaded jvm client with maven
LuciferYang
commented
Jan 30, 2023
LuciferYang
commented
Jan 30, 2023
...m-e2e-tests/src/test/scala/org/apache/spark/sql/connect/client/util/RemoteSparkSession.scala
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Because the ClientE2ETestSuite requires a shaded server jar, the direct execution of or will still fail, which is no different from without this pr |
Contributor
Author
|
Need wait some days, because |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The main change of this pr as follows:
ClientE2ETestSuiteinto a new separate module, the new module namedconnect-jvm-e2e-tests, and add a new profile namedconnect-client-teststo activate this module. The new profile is added because the current test way of this module is cumbersome and it is not easy to build and test by default.we need the follow steps for the new module test with maven
or
It seems simpler to use sbt for test, but in the current dependency management way, using sbt to fetch the no-shaded connect-client jar cannot achieve the purpose of testing both the shaded client jar and the shaded server jar in the E2E tests.
Add a new GA task named
connect-jvm-e2e-tests, this task use maven to test shadedconnect-client-jvm. Under the current dependency management way, if we can find out how to fetch the assembly jar by sbt, we should change it to a more efficient sbt test mode.Remove
repltest dependency fromconnector/connect/client/jvm/pom.xmldue toClientE2ETestSuitemove into separate module.Why are the changes needed?
We should do E2E test with shaded client jar and shaded server jar. Before this pr,
ClientE2ETestSuiteis inconnect-client-jvmmodule and the test will using shaded server jar and no-shaded client jar. After this pr, the test purpose can be achieved by using maven, but the test way using sbt needs to be further explored.Does this PR introduce any user-facing change?
No
How was this patch tested?
Scala 2.13 using maven
Scala 2.12 using sbt
Scala 2.13 using sbt