KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams#22521
KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams#22521FrankYang0529 wants to merge 1 commit into
Conversation
FrankYang0529
changed the title
FrankYang0529
force-pushed
the
KAFKA-20553
branch
from
09bb5cb to
a3111a7
Compare
FrankYang0529
changed the title
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
There are many tests that have been moved to testfixtures. Would you mind explaining the reasoning? IIRC, the test files should be in the test folder
These classes are referenced by test suite in integration tests:
Following classes are referenced by above test classes:
|
Would it be better to split |
FrankYang0529
force-pushed
the
KAFKA-20553
branch
from
ce1b741 to
534eca1
Compare
| java { | ||
| srcDirs = [] | ||
| } | ||
| scala { |
There was a problem hiding this comment.
There are only four files left that require Scala: StreamToTableJoinScalaIntegrationTestImplicitSerdes, StreamToTableJoinScalaIntegrationTestBase, StreamToTableJoinTestData, and WordCountTest. They seem to be quite old, and we already have similar tests like WordCountDemoTest. Since we are phasing out Scala, I would prefer to just remove them.
@mjsax WDYT?
There was a problem hiding this comment.
I think it would reduce test coverage, particularly StreamToTableJoinScalaIntegrationTestImplicitSerdes which tests Scala implicits.
But I can frankly not remember when I reviewed a Scala PR the last time, so the risk of reducing test coverage should be very small.
There was a problem hiding this comment.
StreamToTableJoinScalaIntegrationTestImplicitSerdes which tests Scala implicits.
nice point. I'd like to open another PR to handle it first.
- rewrite
StreamToTableJoinScalaIntegrationTestImplicitSerdesusingTopologyTestDriverto avoid using a true kafka cluster - move it to the streams-scala module
- remove
StreamToTableJoinScalaIntegrationTestBase,StreamToTableJoinTestData, andWordCountTest - remove the scala dependency from streams:integration-tests module
|
@FrankYang0529 please fix the conflicts |
Signed-off-by: PoAn Yang <payang@apache.org>
FrankYang0529
force-pushed
the
KAFKA-20553
branch
from
534eca1 to
f8b44ef
Compare
| archivesName = "kafka-streams-integration-tests" | ||
| } | ||
|
|
||
| apply plugin: 'java-test-fixtures' |
There was a problem hiding this comment.
I guess the purpose of adding testfixtures to streams:integration-tests was to let the tools module use EmbeddedKafkaCluster. However, I grepped the usages and it seems they can be replaced by ClusterInstance, which would prevent introducing unnecessary module complexity
3 participants
Gradle test fixtures to the clients module and removes all dependencies
on sourceSets.test.output. We should revisit other usages in this JIRA.
This will simplify the dependency graph and unblock the upgrade to
Gradle 9.5+
This patch is working on
streamsandstreams:integration-tests.Reviewers: Chia-Ping Tsai chia7712@gmail.com, Matthias J. Sax
matthias@confluent.io